Bug fix for issues #424, #278, #163 #841
Conversation
Disclaimer: I am only playing with this for an hour or so. But I do think that I just fixed no less than three issues: lukasoppermann#424 and two issues that appear to be the same (@278 and lukasoppermann#163). But I tested on only one machine.
|
See my comment at issue #424. The fix is almost right, but not quite. |
There was a problem hiding this comment.
It appears that #777 fixes the issue, but this change is not visible in the current vversion.
In particular, it is the lines from 428:
if (dragging.oldDisplay !== undefined) { dragging.style.display = dragging.oldDisplay delete dragging.oldDisplay }
but they do not appear in the demo example.
|
Hey @jneb what is your latest take on this after investigating? Does this still fix the bugs? Also please change it to single quotes to appease the gods of linting. Thanks. |
|
I had a good look at the git blame, and found out that a fix you did for
months ago fixes it. ( It does for me, at least. )
And from my analysis of what I think the code is supposed to do, combined
with what I think it's the cause of the problem, your fix (#777) should
handle all cases.
I checked it against Chrome, Edge and Webview; those are all I have.
My conclusion is that the main problem is that the library code in the demo
directory isn't up to date.
But it didn't really figure out which is the latest "official" version; I
was confused to be honest. I guess this confusion may also cause big
reports from others.
But thanks for a great product!
I use it for a personal planning program that I am developing. It started
just for myself (I am a terrible and messy planner, and I do need help :-)
). The goal is to publish it after making it useful for outsiders.
-Jurjen
Op za 6 nov. 2021 15:22 schreef Lukas Oppermann ***@***.***>:
… Hey @jneb <https://github.com/jneb> what is your latest take on this
after investigating? Does this still fix the bugs?
Also please change it to single quotes to appease the gods of linting.
Thanks.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#841 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACEOFD2FD5RP5PVRZG3HBLDUKU22HANCNFSM5HOFT3KQ>
.
|
|
Hey, @jneb can you please fix the linting error so I can go ahead and merge it? |
|
I'll have a look.
- Jurjen
Op vr 12 nov. 2021 09:04 schreef Lukas Oppermann ***@***.***>:
… Hey, @jneb <https://github.com/jneb> can you please fix the linting error
so I can go ahead and merge it?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#841 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACEOFD2NI6AW73TGQMLYTR3ULTDBBANCNFSM5HOFT3KQ>
.
|
|
Awesome, it's just a tiny change, but it blocks the merge. 😉 |
|
If I have a good look at the version of code this fix is merging with, it shouldn't be necessary to change anything at all! |
|
Yes, I checked it. The problems are caused by the "docs" directory containing an old version of the library, and some people (like me) being confused by that version. |
Disclaimer: I am only playing with this for an hour or so.
But this fix makes complete sense: the effect is that if an item is not dropped, there's no code that makes the dragged object visible again, making it disappear.
All I did was make sure that it is made visible at dragend regardless of the drop.
But I do think that I just fixed no less than three issues: #424 and two issues that appear to be the same (#278 and #163).
But I tested on only one machine.