Opened 13 months ago

Closed 13 months ago

Last modified 13 months ago

#14289 closed bug (fixed)

Drag'n'drop in lists grabs wrong item

Reported by: humdinger Owned by: jscipione
Priority: normal Milestone: Unscheduled
Component: Kits/Interface Kit Version: R1/Development
Keywords: Cc: closequarters
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

This is hrev52080, 32bit

Reproducible in WonderBrush:

  • Create a number of objects
  • Select the 1st object
  • Now drag'n'drop the 3rd object anywhere, for example to the end of the list

Instead of the item ou clicked on, the current selection is moved. Example avi attached.

I suspect hrev52062... ?

Attachments (3)

list-dnd.avi (235.4 KB ) - added by humdinger 13 months ago.
d'n'd items in list
appearance-dnd.avi (811.5 KB ) - added by humdinger 13 months ago.
Appearances d'n'd
locale-dnd.avi (375.6 KB ) - added by humdinger 13 months ago.
Locale prefs

Download all attachments as: .zip

Change History (21)

by humdinger, 13 months ago

Attachment: list-dnd.avi added

d'n'd items in list

comment:1 by waddlesplash, 13 months ago

Cc: closequarters added
Owner: changed from nobody to jscipione
Status: newassigned

comment:2 by jscipione, 13 months ago

I can attest to drag and drop in Appearance preferences colors list code is working correctly. It drags out the clicked on list item and drops onto the dropped item. This is most likely a bug in WonderBrush caused by the change in behavior to ListViews to make them do stuff on MouseUp instead of MouseDown. Wonderbrush may be making some sort of assumption about the item use on MouseDown and MouseUp that we have broken. Still, this may be a bug we should fix in Wonderbrush code rather than going back on our ListView changes.

comment:3 by humdinger, 13 months ago

QuickLaunch also has issues when d'n'd favorites.

Appearances doesn't behave quite correctly either. A dropped colour changes the selected item's colour well, not the one you drop it on. See attached avi.

by humdinger, 13 months ago

Attachment: appearance-dnd.avi added

Appearances d'n'd

comment:4 by jscipione, 13 months ago

hmmmm are you sure you're not on Haiku previous to hrev52090? because that looks a lot like the pre-hrev52090 behavior of setting the currently selected color on drop instead of the post hrev52090 change of selecting the color you dropped on to.

Last edited 13 months ago by jscipione (previous) (diff)

comment:5 by humdinger, 13 months ago

Right. As the ticket says, I'm on hrev52080. Guess I'll have to somehow try to fix QuickLaunch. Wonderbrush being closed-source will have to suffer the change.

comment:6 by jscipione, 13 months ago

Well in that case the Color ListView is behaving as it should.

comment:7 by humdinger, 13 months ago

Locale shows the same behaviour as WonderBrush.

comment:8 by jscipione, 13 months ago

It may be something we're doing in ListView is not quite kosher and needs adjustment. Like we may have to save a piece of data in MouseDown and then use it in MouseUp kind of thing.

comment:9 by closequarters, 13 months ago

So jscipione just did my own testing, and can confirm that Wonderbrush isn't working as one would expect, but that Appearance and Locale seem to be working. This is on the latest nightly hrev52112.

However I'm concerned we keep seeing so many problem reports related to the ListView change, so I'm going to spend a little time looking over the code again, and see if any solutions pop out at me.

comment:10 by jscipione, 13 months ago

Unfortunately it is probably a bug in Wonderbrush perhaps similar to the one in Vision not calling its inherited class method in MouseUp or MouseDown. :(

comment:11 by closequarters, 13 months ago

humdinger, I created a pull request for QuickLaunch to fix the drag and drop in the MainListView. The key was to use the index of the dragged item, not the index of the selected item. See https://github.com/humdingerb/quicklaunch/pull/21

I also spent some time thinking about how we could make sure this works with some of these applications like Wonderbrush that are breaking but so far I haven't come up with a way we can do that without also breaking the multi-select drag and drop that was resolved in #9190

comment:12 by humdinger, 13 months ago

Thanks, David! PR merged. :)
Since I stole much of that code from Locale IIRC, it may be the same issue there (I can still reproduce with hrev52115). Wanna take a look?

Last edited 13 months ago by humdinger (previous) (diff)

comment:13 by closequarters, 13 months ago

humdinger, I tested Locale last night on the current nightly but didn't see any drag and drop issues. (Indeed, Locale is where the multi-select drag and drop fix was tested in the first place). Are you seeing the issue where the "selected" item is dragged instead of the "dragged" item?

comment:14 by humdinger, 13 months ago

I've attached another avi with the Locale issue (hrev52115).

by humdinger, 13 months ago

Attachment: locale-dnd.avi added

Locale prefs

comment:15 by jscipione, 13 months ago

I can definitely see the problem in the Locale Pref code, it is assuming that the dragged item is a selected item and that is not a valid assumption. The bug needs to be fixed in Locale Prefs to use the index of the dragged item instead of the index of the selected item and then to only include the rest of the selected items if the dragged item is selected (which it may not be).

comment:16 by closequarters, 13 months ago

Locale issue, as well as WonderBrush and QuickLaunch issue should be solved by new patch posted in #9190 (https://review.haiku-os.org/#/c/haiku/+/343)

humdinger, the new patch should still work with the PR you merged the other day.

comment:17 by waddlesplash, 13 months ago

Resolution: fixed
Status: assignedclosed

And now merged.

comment:18 by humdinger, 13 months ago

Thanks very much David. I've built an image with your patch (if only I knew waddlesplash would merge so soon... :) ).

I can confirm, it's working nicely now! Even old WonderBrush and Quicklaunch (even before your PR), as well as multiple-items d&d. Nicely done, thanks!

Note: See TracTickets for help on using tickets.