lxqt / pcmanfm-qt

File manager and desktop icon manager (Qt port of PCManFM and libfm)
https://lxqt-project.org
GNU General Public License v2.0
426 stars 112 forks source link

Improvement to Desktop icon positioning #1969

Closed isf63 closed 2 weeks ago

isf63 commented 3 weeks ago

If this is valid, given the time frame it should be for LXQt 2.2.0 obviously.

Expected Behavior
Current Behavior
Possible Solution

If the DnD'd items are sticky and the non-sticky items don't get in the way, when all of the destination slots are empty do not move any other icons.

Steps to Reproduce (for bugs)
  1. Align 7 sticky icons as so: image1

  2. Select the top 3 icons and DnD them one slot over: image2

  3. As depicted in the image, one of the bottom-row icons erroneously shifts down.

Context
System Information
tsujan commented 3 weeks ago

If this is valid,...

It's definitely valid. Thanks for reporting!

... it should be for LXQt 2.2.0 obviously.

Unless I can magically find out why it's so before tomorrow. But magics never happen ;)

tsujan commented 3 weeks ago

Magic happened ;) I found out why it's so in this example. It's very logical but counterintuitive.

After the release is finished (a really boring job), I'll try to fix this (an exciting job). It needs making changes to the current logic, and it should be done with care...

EDIT: Made the patch. Will make a PR after 2.1.0.

tsujan commented 3 weeks ago

The PR: https://github.com/lxqt/pcmanfm-qt/pull/1970

I tested it quickly and will re-read its code. However it should be tested thoroughly, and your tests would be very appreciated. There's no haste though.

isf63 commented 3 weeks ago

I'm testing it - the reproducible bug case above I happened to stumble upon, during normal use it would only happen very rarely.

tsujan commented 3 weeks ago

It was enough to make two rows of items, one above the other, and DND the upper row horizontally by one cell; then some items of the lower row moved downward — as they should, because of the original code. That doesn't happen anymore.

tsujan commented 2 weeks ago

@isf63 Any problem after applying the patch?

Here, I tested it tens of times and encountered no issue.

isf63 commented 2 weeks ago

Seems to work, plus fix the bug case.