Fix drag'n'drop behaviour in ProjectTreeView plugin
Needs RevisionPublic

Authored by vkorneev on Aug 5 2018, 5:20 PM.

Details

Reviewers
mwolff
Group Reviewers
KDevelop
Summary

Drag'n'drop target folders were switching between open and closed state.
The fix changes this behaviour to only expanding the previously closed folder
items and not changing the state of the expanded ones.

Simply passing the DragMoveEvent to QTreeView::dragMoveEvent only for closed
folder items was not enough, because QTreeView:timerEvent (which is activated
by timer set up in QTreeView::timerEvent) don't track the item which caused
the starting of the timer. So sometimes passing the mouse cursor over the
closed folder and stopping in over the expanded folder made the expanded one
to close again. Current implementation doesn't have such problem.

BUG: 377023

Diff Detail

Repository
R32 KDevelop
Branch
bug377023 (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 1705
Build 1723: arc lint + arc unit
vkorneev created this revision.Aug 5 2018, 5:20 PM
Restricted Application added a project: KDevelop. · View Herald TranscriptAug 5 2018, 5:20 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald Transcript
vkorneev requested review of this revision.Aug 5 2018, 5:20 PM
vkorneev edited the summary of this revision. (Show Details)Aug 5 2018, 5:22 PM
vkorneev edited the summary of this revision. (Show Details)Aug 5 2018, 5:33 PM
vkorneev edited the summary of this revision. (Show Details)Aug 5 2018, 5:35 PM
croick added a subscriber: croick.

I tested your patch. When I'm dragging a file over a folder the project view is flickering. Do you get a similar behaviour?

No, I don't. There where a few little twitches while I was testing it just now, but not the flickering. Here is the video of the test with patch applied.

vkorneev updated this revision to Diff 39161.Aug 6 2018, 1:44 AM

Remove redundant superclass method call

vkorneev added a comment.EditedAug 6 2018, 1:56 AM

I've removed redundant call to superclass method in ProjectTreeView::timerEvent, yet I don't quite think this could have been the reason of the flickering and, as for now, I can't find any other possible reason of the problem. Dear @croick, could you please tell me what OS do you use, so I can try running in VM and check if the flickering persists?

kossebau added a subscriber: kossebau.EditedAug 6 2018, 10:40 PM

Hi @vkorneev thanks for coming up with a patch for that bug report :)

IMHO though that bug is not necessarily a bug. I find myself often accidentally opening a subfolder (e.g. when mouse does not follow mind ;) ) and especially with large subfolders that gets annoying if one cannot close those folders again.

I tested your patch e.g. on kdevelop, and while trying to focus a dragged file on a plugin subdir, accidentally a wrong one was opened, upon which I moved the mouse a bit ouf of the way, just to get more unwanted plugin subfolders opening... without a way to undo that and thus with the actual target far away from the viewport.

So being able to close a folder again in the very same drag movement I would consider rather a feature, like you can open and close a door with your legs while carrying an item with your hands and not having to drop it, just to grab the door handle :)

I would agree though that the timing of the opening and closening while the mouse is over a folder should be improved, that feels a bit out-of-control on subtle accidental mouse movements.

Sp personally not in favour of this very patch as it is. Let's have some others give their opinions as well though, not a kdevelop maintainer myself, only contributor.

Hi @kossebau , you're welcome. Thank you for testing my patch :)

After some thinking on this subject, I have to admit that you're right - the behaviour proposed by this patch is not very convenient, and now I'm not sure whether it makes things better or worse. As an alternative I can propose another behaviour with automatic closing of the folders which were expanded during the drag.

The user starts a drag, the folder under the cursor expands and closes again if the cursor is moved somewhere else, or stays expanded if the cursor is moved to the content of this folder. After the drag the state of the project tree view remains the same as before except for the folder which became the drop target (if some drop action has taken place).

As far as I remember, I've seen such behaviour in some application, but I can't recall where exactly. Long story short, I'll try to implement it and upload it for test. If it fits better - that's good, in other case... well, I'll just move to some next bug :)

croick added a comment.Aug 7 2018, 9:55 PM

could you please tell me what OS do you use, so I can try running in VM and check if the flickering persists?

I'm using an ArchLinux with Qt 5.11.1. But I think it's rather a hardware/driver issue (Intel HD Graphics 630). I have these problems from time to time with other programs as well, but in this case it's quite reproducible.

The user starts a drag, the folder under the cursor expands and closes again if the cursor is moved somewhere else, or stays expanded if the cursor is moved to the content of this folder. After the drag the state of the project tree view remains the same as before except for the folder which became the drop target (if some drop action has taken place).

I would really like this approach and I think most people would consider this an improvement.

vkorneev updated this revision to Diff 39463.Aug 11 2018, 3:49 PM

Implement automatic folder collapse and expand during drag'n'drop

This implementation adds automatic folder expanding and collapsing to
the ProjectTreeView plugin. It also contains an attempt to smooth out the
shift of the items under the cursor during the collapse of the folders,
which, as for now, doesn't work when verticalScrollBar is in the top
position.

I've tried to implement the approach discussed above. Here is the video of the results achieved as for now:

The only problem which really bothers me in the current implementation is the unstable behaviour of the items during the collapse of the other folders: if the folder expanded is situated under the folders which where previously expanded (and therefore should be collapsed automatically), it shifts upwards and slips from under the mouse cursor. I've tried to solve this problem shifting the viewport widget back again, but that doesn't work when the vertical scrollbar value is 0 (can't scroll lower).

So, the critics are welcome, as well as the advice whether I should continue the work on this issue or should better move to something more useful.

mwolff added a subscriber: mwolff.Jan 27 2019, 7:31 PM

sorry for the long delay. personally, I like what I'm seeing in the video, could you cleanup the patch a little please? and does ensureVisible help maybe?

plugins/projectmanagerview/projecttreeview.cpp
57

undo this hunk

177

could you maybe instead just call ensureVisible on the index below the cursor before you collapse the previously opened folders?

plugins/projectmanagerview/projecttreeview.h
80

add m_ prefixes to all members

81

don't abbreviate please

mwolff requested changes to this revision.Jan 27 2019, 7:31 PM
This revision now requires changes to proceed.Jan 27 2019, 7:31 PM