They handle both focus and highlight, which seems logical.
This fixes most annoying bugs with desktop switching.
Signed-off-by: Mavridis Philippe <mavridisf@gmail.com>
Hi Philippe,
sorry for the late response. I tested the code, it seems to work fine in most cases, using CTRL+movement keys changes the virtual desktop.
Some comments:
it seems there is a small bug. I have 4 virtual desktops in 2x2 configuration. If I show all four desktops, movement works as expected.
If I enable "Layout empty virtual desktops minimized" in the settings and I have windows only on destop 1 and 2 (top row), then moving down from virtual desktop 2 ends up in desktop 3 instead of 4. Viceversa (3 -> 2 when going up) also happens. And Ctrl+J/Down arrow from virtual desktop 4 goes to desktop 1. This does not seem to happen when that option is not enable or when there are windows on all desktop, so no desktop is minimized.
after we change virtual desktop, it would be nice to automatically select a window to start from, without the need to press the movement keys again. This could be addressed on a separate PR is you want.
Thanks for the great work!!
Hi Philippe,
sorry for the late response. I tested the code, it seems to work fine in most cases, using CTRL+movement keys changes the virtual desktop.
Some comments:
1) it seems there is a small bug. I have 4 virtual desktops in 2x2 configuration. If I show all four desktops, movement works as expected.
If I enable "Layout empty virtual desktops minimized" in the settings and I have windows only on destop 1 and 2 (top row), then moving down from virtual desktop 2 ends up in desktop 3 instead of 4. Viceversa (3 -> 2 when going up) also happens. And Ctrl+J/Down arrow from virtual desktop 4 goes to desktop 1. This does not seem to happen when that option is not enable or when there are windows on all desktop, so no desktop is minimized.
2) after we change virtual desktop, it would be nice to automatically select a window to start from, without the need to press the movement keys again. This could be addressed on a separate PR is you want.
Thanks for the great work!!
I have been working on another small bug (which turned out to be a little tricky), which is when a window of a virtual desktop is selected, the virtual desktop highlight/focus is lost and always reverts to the first desktop.
Apropos 1: Confirmed.
Apropos 2: This seems too trivial to create a separate PR.
Thanks for testing the changes.
Hello Michele,
I have been working on another small bug (which turned out to be a little tricky), which is when a window of a virtual desktop is selected, the virtual desktop highlight/focus is lost and always reverts to the first desktop.
Apropos 1: Confirmed.
Apropos 2: This seems too trivial to create a separate PR.
Thanks for testing the changes.
A pointer to the current desktop is stored in a separate variable.
Desktop switching from keyboard is now fixed.
Signed-off-by: Mavridis Philippe <mavridisf@gmail.com>
after we change virtual desktop, it would be nice to automatically select a window to start from, without the need to press the movement keys again. This could be addressed on a separate PR is you want.
> when a window of a virtual desktop is selected, the virtual desktop highlight/focus is lost and always reverts to the first desktop.
Resolved with commit [04447ac149](https://mirror.git.trinitydesktop.org/gitea/TDE/kompose/commit/04447ac1499fae40937b95fcbfd5048083a8b471).
> after we change virtual desktop, it would be nice to automatically select a window to start from, without the need to press the movement keys again. This could be addressed on a separate PR is you want.
Resolved with commit [4e5e32791e](https://mirror.git.trinitydesktop.org/gitea/TDE/kompose/commit/4e5e42791e39b75c1354d3174ffb3edf473d28fd).
@MicheleC The PR is ready to be merged. I've pushed a commit to disable the intimidating option and created a separate issue for point 1 (#10). When it is solved then we can revert that commit and thus restore the option. What do you think?
@MicheleC The PR is ready to be merged. I've pushed a commit to disable the intimidating option and created a separate issue for point 1 (#10). When it is solved then we can revert that commit and thus restore the option. What do you think?
The idea of disabling the checkbox till we fix the bug is good. Nevertheless with the original code in the PR, empty virtual desktops are always minimized.
We need to use the generic layout to avoid doing that.
The idea of disabling the checkbox till we fix the bug is good. Nevertheless with the original code in the PR, empty virtual desktops are always minimized.
We need to use the generic layout to avoid doing that.
@blu256
Could you please test if the proposed fix in the commnet above also work for you? With the original code I always had empty desktops minimized, meaning the bug was always there.
@blu256
Could you please test if the proposed fix in the commnet above also work for you? With the original code I always had empty desktops minimized, meaning the bug was always there.
I had a look through the code, I can't find any usage for this controlHold variable declared in this file. There is another static bool controlHold defined in src/komposetaskcontainerwidget.cpp and used in that file, but this one seems not used at all.
I have rebuilt and tested without this one and the behaviour seems the same.
I had a look through the code, I can't find any usage for this controlHold variable declared in this file. There is another static bool controlHold defined in src/komposetaskcontainerwidget.cpp and used in that file, but this one seems not used at all.
I have rebuilt and tested without this one and the behaviour seems the same.
The first 3 cases could be grouped together, less code and no possibility of different behavior by mistake from different keys.
case TQt::Key_Right:
case TQt::Key_D:
case TQt::Key_L:
direction = DLAYOUT_RIGHT;
break;
Same applies for the other 9 cases (3 groups of 3 cases each)
The first 3 cases could be grouped together, less code and no possibility of different behavior by mistake from different keys.
```
case TQt::Key_Right:
case TQt::Key_D:
case TQt::Key_L:
direction = DLAYOUT_RIGHT;
break;
```
Same applies for the other 9 cases (3 groups of 3 cases each)
MicheleC
Current bugs:
They combine some traits of both {enter,leave}Event and focus{In,Out}Event. Signed-off-by: Mavridis Philippe <mavridisf@gmail.com>d5db6c5c25tocc13befa923 years agoHi Philippe,
sorry for the late response. I tested the code, it seems to work fine in most cases, using CTRL+movement keys changes the virtual desktop.
Some comments:
it seems there is a small bug. I have 4 virtual desktops in 2x2 configuration. If I show all four desktops, movement works as expected.
If I enable "Layout empty virtual desktops minimized" in the settings and I have windows only on destop 1 and 2 (top row), then moving down from virtual desktop 2 ends up in desktop 3 instead of 4. Viceversa (3 -> 2 when going up) also happens. And Ctrl+J/Down arrow from virtual desktop 4 goes to desktop 1. This does not seem to happen when that option is not enable or when there are windows on all desktop, so no desktop is minimized.
after we change virtual desktop, it would be nice to automatically select a window to start from, without the need to press the movement keys again. This could be addressed on a separate PR is you want.
Thanks for the great work!!
Hello Michele,
I have been working on another small bug (which turned out to be a little tricky), which is when a window of a virtual desktop is selected, the virtual desktop highlight/focus is lost and always reverts to the first desktop.
Apropos 1: Confirmed.
Apropos 2: This seems too trivial to create a separate PR.
Thanks for testing the changes.
Resolved with commit 04447ac149.
Resolved with commit 4e5e32791e.
Tested the new version and works well.
Now only point 1. from my previous commit remains to be solved, then we can merge.
Thanks for the good work!
4e5e42791eto420af893f83 years agoWIP: Ctrl+[Movement Keys] changes virtual desktopto Ctrl+[Movement Keys] changes virtual desktop 3 years ago@MicheleC The PR is ready to be merged. I've pushed a commit to disable the intimidating option and created a separate issue for point 1 (#10). When it is solved then we can revert that commit and thus restore the option. What do you think?
The idea of disabling the checkbox till we fix the bug is good. Nevertheless with the original code in the PR, empty virtual desktops are always minimized.
We need to use the generic layout to avoid doing that.
if ( t == TLAYOUT_TASKCONTAINERS &&!KomposeSettings::instance()->getDynamicVirtDeskLayout() )t = TLAYOUT_GENERIC;*/Needs to be something like this, to work correctly.
Done.
@blu256
Could you please test if the proposed fix in the commnet above also work for you? With the original code I always had empty desktops minimized, meaning the bug was always there.
420af893f8to48f570eda83 years ago@MicheleC I tested the proposed changes and it worked. Thanks!
Functionality-wise it is ok! Thanks for the good work.
Code wise there are a couple of things we can improve.
#include <kdebug.h>#include <krootpixmap.h>static bool controlHold = false; // is the control key pressedI had a look through the code, I can't find any usage for this controlHold variable declared in this file. There is another static bool controlHold defined in src/komposetaskcontainerwidget.cpp and used in that file, but this one seems not used at all.
I have rebuilt and tested without this one and the behaviour seems the same.
{if ( e->key() == TQt::Key_Control ){controlHold = true;Line with 'controlHold' is not required (see above)
else if ( e->key() == TQt::Key_Control ){controlHold = false;Line with 'controlHold' is not required (see above)
switch( e->key() ){case TQt::Key_Right:direction = DLAYOUT_RIGHT;The first 3 cases could be grouped together, less code and no possibility of different behavior by mistake from different keys.
Same applies for the other 9 cases (3 groups of 3 cases each)
df8c25b1f2to539e15a5233 years agoLooks good 👍
539e15a523into master 3 years agoMerged and backported. Thanks Philippe!
Reviewers
539e15a523.