While dragging, the user could want to change the modifier, so we should call event->acceptProposedAction() to do update the dragCursor.
FIXED-IN: 20.04.0
meven | |
ngraham | |
davidedmundson | |
elvisangelaccio |
Dolphin |
While dragging, the user could want to change the modifier, so we should call event->acceptProposedAction() to do update the dragCursor.
FIXED-IN: 20.04.0
While dragging files, change modifiers between Ctrl/Shift/Alt... and move the mouse at least 1px to see the cursor changes.
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
This works only once the mouse starts moving after the key modifier has been pressed.
I try like this: start dragging > move the mouse (1) > press and hold modifier keys > move the mouse (at least 1px) (2)
and it works.
You mean it doesn't work without the step (2) ?
Yes, the cursor does not change as the modifier is pressed.
At least despite the cursor does not change in the situation the drop event will run the expected action.
This makes the UI and behavior incoherent though.
So I guess we should add a keypress connection during a drag and disconnect it at the end caring only for modifiers and calling event->acceptProposedAction()
That's a pre-existing issue unrelated to this patch: https://bugs.kde.org/show_bug.cgi?id=388259
It should definitely be fixed somehow, but it's probably not a blocker for this patch. However since it's related, if you can and want to fix it in this patch, that would be lovely!
I've been very impressed with your work lately, @trmdi. Great job!
Nice, it works! Not sure about the code, I'll let @meven or @elvisangelaccio review it.
src/kitemviews/kitemlistcontroller.cpp | ||
---|---|---|
72 | Is this a timer constantly timing out during a drag? |
These ones:
1, When dragging, if you change the modifier key and not move the mouse afterwards -> the cursor does not change, no way to update the cursor except sending a MouseMoveEvent.
2, When you drag stuff from one process into another one the possible actions depends on the modifier key (e.g. ShiftModifier -> possibleActions = MoveAction)
If you change the modifier key without moving the mouse, the possibleActions doesn't change, while you expect the new action -> no way to update this except sending a MouseMoveEvent.
Are these bugs or not? Could this be fixed in QDrag or we have to fix our code?
src/kitemviews/kitemlistcontroller.cpp | ||
---|---|---|
72 | Yes, it's a Qt zero timer. During the drag, it eats all KeyPressEvent. |
src/kitemviews/kitemlistcontroller.cpp | ||
---|---|---|
933 | We have 2 concepts: what action the event is accepted as If this widget ignores the event it shouldn't be changing the drop action |
src/kitemviews/kitemlistcontroller.cpp | ||
---|---|---|
933 |
Could this be possible in this case of Dolphin? |
As far as I can see, bug #388259 has been fixed upstream (either in Qt or in the platform integration plugin).
As for this feature, the problem is it doesn't work on Wayland and I'm afraid that's a going to be a stopper.
With Qt 5.15 I get this debug output when i click that "drag" button:
Qt::CopyAction QFlags<Qt::DropAction>(CopyAction|MoveAction)
Thanks, that means that bug is fixed in Qt 5.15.
This patch now try to change the cursor when the user change the modifier key and not move the mouse.
But I don't understand why it doesn't work in Wayland. Maybe another bug in Qt? If you open Dolphin (without this patch) drag a file, change the modifier key, then release the key, and drop it, it still behaves like you're holding that modifier key.
src/kitemviews/kitemlistcontroller.cpp | ||
---|---|---|
933 |
I think this only ignores the event in this widget, other one can accept it again when the drag enters it. |
Yeah, I think so.
The patch is much smaller now, so I think we can merge it even if it doesn't work on Wayland.
src/kitemviews/kitemlistcontroller.cpp | ||
---|---|---|
933 | That's right, which is why changing the drop action from a widget which ignores the event is a weird thing to do. (Not that it really matters) |
src/kitemviews/kitemlistcontroller.cpp | ||
---|---|---|
933 | But setting the drop action is needed to change the cursor on the DragMove event. |
@ngraham
It seems that everyone accepted this, could you help me to land this to whatever branch you want ?