mzur / gnome-shell-wsmatrix

GNOME shell extension to arrange workspaces in a two-dimensional grid with workspace thumbnails
GNU General Public License v3.0
464 stars 59 forks source link

[WIP] initial gnome 40 support #152

Closed ebeem closed 2 years ago

ebeem commented 3 years ago

Please report bugs/issues or discuss features in the discussion accompanying the alpha release for GNOME 40 support.

Still has many bugs, and only the switcher is implemented (no overview).

bugs/missing features:

minor improvements & extra features:

what's good about this branch:

Currently, this implements the switcher using a modifier key. I think this implementation will help us fix a lot of issues and have more flexibility. I am not going to be able to work on this gnome#40 support for about 3 weeks, I will look at it after that and try to fix current bugs.

Any contributions are welcomed. We will probably not merge this into master, once we finalize it, it's better to create a new branch because this one will probably have a lot of dirty code in it.

fixes #146

mzur commented 3 years ago

I removed all files related to the overview for now (they're now in the gnome-40-overview branch). We can take care of the overview once we have a proper implementation of the popup.

I think a (clean) working implementation for the popup can be merged into master and released as beta version. We can then move on and work on the remaining features (which will be quite complex I assume).

I did the same with the workspace overview (Super+W) -> gnome-40-workspace-overview.

ebeem commented 3 years ago

Thanks @mzur, settings are working great now. I fixed the initial selection of workspace (always goes to 1) and the warning messages

mzur commented 3 years ago

Great! I'll continue to tinker with this branch a bit.

ebeem commented 3 years ago

I modified the option scale and its default value, now it reflects the scale of the switcher rather than the thumbnails themselves, the thumbnails will adjust accordingly (I feel this makes much more sense).

ebeem commented 3 years ago

Maybe I mentioned problems with the style, I tried different themes now. I think there's a little problem with padding, but overall, even the materia theme in #12 is displaying the workspace switcher right. Does it look good in your case @mzur?

Screenshot from 2021-04-13 22-30-06

Screenshot from 2021-04-13 22-29-38

mzur commented 3 years ago

The original scale value of 0.1 set the size of each thumbnail. A new default of 0.6 seems too large. I'll need to dig more into the code. Give me a few days :wink: Currently, the aspect ratio of the thumbnails seems off to me (on my system, if I increase the size so I can recognize something).

ebeem commented 3 years ago

@mzur it's not thumbnail scale anymore, it's switcher workspace popup scale. So if we set 0.6, the switcher as whole will take 0.6 of the screen, and the thumbnails (depending on how many rows/columns) will fill this switcher popup which takes 60% of the screen. What do you think?

mzur commented 3 years ago

Setting the size of the whole switcher is definitely easier than setting the size of each thumbnail. I'll have to take a closer look at that, yet. For now, I just removed code that I want to take care of later. I'll get back to this PR in the next days.

ebeem commented 3 years ago

noticed one issue regarding the scaling, if the workspaces rows != columns the thumbnails will fit the switcher by scaling height and value with a different factor. This will not look well. Maybe we need to scale only the width of the switcher and let the height fit automatically as needed.

ebeem commented 3 years ago

I think for the time being, we need to fix these before releasing. Can you confirm @mzur? I can't see any features missing except for overview

mzur commented 3 years ago

I still haven't finished to dig through all the code. Give me a few more days :wink: No need to hurry with this.

ebeem commented 3 years ago

WmOverride.js will need some more cleanup, it contains some code that's not required anymore as the switcher popup is taking care of most of the switcher behavior now. I already started working a bit in the modifier, it has some issues that I might need some help with, basically we have shift as a modifier to move windows to other workspaces by default. The problem I am facing is when the shift is a modifier and it's pressed, then all the other keys are changed (for example if someone presses a, it will be considered A). I am trying to overcome this right now to make the bindings better. If we can't find a solution to this, we will probably have to complicate the configuration page or create a mapping list between keys and their shift ascii which we don't want to do.

ebeem commented 3 years ago

I also added this line that you might want to remove @mzur

label.style = 'font-size: ' + Math.min(this._childHeight, this._childWidth) / 8 + 'px;';

it's going to scale the font size of the workspace name based on the height/width of the space available, I don't think it's possible to set font size anywhere other than style.

ebeem commented 3 years ago

@mzur Another suggestion to keep our code with the same standards as gnome's. I found earlier a page where these guidelines are listed, but I can't find it now.

Maybe it's wiser to just follow the gnome's coding guidelies for this extension to have a list that we can refer to. I know that gnome

ebeem commented 3 years ago

So far merging this will fix

145, #140, #135, #126, #12, #88 (still has an issue with multiple screen tho).

It's supposed to fix #150 before merge. We can look at #127 here once the overview is implemented. There is a progress with #60 (drag and drop available, but on drop nothing happens right now).

mzur commented 3 years ago

Didn't you say you can't work on this for 3 weeks? I was hoping to catch up on the changes in that time :smile: I still need a few days or probably weeks to properly review this or suggest changes. But feel free to continue the work and a big thanks for what you have already done :+1: I also have no objections against adopting GNOME's coding style.

andersjohansson commented 3 years ago

Thank you so much for working on this. I can confirm that the branch in it’s current state has the basic functionality working for me. I have two issues:

  1. Styling. This is what it looks like for me, using arc theme. Skärmbild från 2021-04-21 13-48-28
  2. Animations. In my preferred 3×2 grid. Moving from left/right between workspace 2 and 3 (top middle and top right), and between workspace 4 and 5 (bottom left and bottom middle), shows the wrong animation of the workspace (behind the switcher popup). It is shown as diagonal movement but should be shown as just horizontal. This doesn’t appear to be a problem in a 3×3 grid (as @ebeem seems to be using in the screenshots above). I took a quick look at the code in workspaceAnimation.js but couldn’t quite wrap my head around it or find a good way to debug it. But maybe it is a problem with the arithmetic there?
ebeem commented 3 years ago

@andersjohansson Thanks for reporting! I think I didn't calculate targetColumn & targetRow correctly. I probably switched them, will work on it along with other remaining issues soon.

ebeem commented 3 years ago

@andersjohansson Please check if it works now

wedeluxe commented 3 years ago

Thanks to the great work of you guys I finally dared upgrading my Arch Linux system to GNOME 40. So I'm available for testing as well.

My setup:

Found 2 issues:

  1. I'm using Super+<arrow keys> to switch workspaces. Switching and wraparound work as expected, but the workspace switcher is not displayed. For some weird, unknown reasons (they are not configured in the GNOME Prefs) Ctrl+Alt+Up and Ctrl+Alt+Down are still functional and switch workspaces. When using these two, the switcher is shown.
  2. The workspace switcher on my left monitor is broken (see screenshot). 2021-04-27 021004 Monitor 1 (on the left) is run in portrait orientation. But the switcher and the workspace preview images are shown in landscape orientation. As a result of that, the third column of those previews reaches out to monitor 2 (on the right).
ebeem commented 3 years ago

Thanks for reporting @wedeluxe

Regarding first problem, there is a different approach now to show the workspace switcher, I think we will need to reconsider this more as it uses a mask to detect whether to show switcher popup or not. It's not a good idea to have a mask like <super> (which is your case). Because this is going to basically show the switcher whenever you press <super> and then it will allow you to switch the workspace as you want. Once the switcher popup is shown now, it will steal focus and handle all input. So basically the idea now is to have a mask like <Ctrl><Alt> to show switcher, and then using arrows or keypads to switch spaces while the switcher is shown. Once the mask is released, the switcher will disappear.

Regarding the second problem, I think I will need to take a better look at this, it seems like the switcher and thumbnails are receiving the width/height from your main display/monitor, while it should get it from the current display/monitor. I will look more into that and try to fix it by next week.

dseomn commented 3 years ago

Regarding first problem, there is a different approach now to show the workspace switcher, I think we will need to reconsider this more as it uses a mask to detect whether to show switcher popup or not. It's not a good idea to have a mask like <super> (which is your case). Because this is going to basically show the switcher whenever you press <super> and then it will allow you to switch the workspace as you want. Once the switcher popup is shown now, it will steal focus and handle all input. So basically the idea now is to have a mask like <Ctrl><Alt> to show switcher, and then using arrows or keypads to switch spaces while the switcher is shown. Once the mask is released, the switcher will disappear.

Would it be possible to only show the switcher after the arrow key is pressed? I use <Ctrl><Alt> for a bunch of other stuff in addition to switching workspaces, and I'd really rather not have focus and input handled until I hit the last key in the key combination.

ebeem commented 3 years ago

@dseomn Yes, actually this is the current approach, but because of this

It's not possible currently to change the modifier, the plan is to have it in the preferences later. But you will need to do some tweaks as well to your shortcuts, it must be having some common modifier. For example, you can have <super> and arrow right to switch to the workspace on the right and <Ctrl><alt> and arrow down to switch to the workspace below.

Other than that, once the configuration is implemented, you should be ready to go.

andersjohansson commented 3 years ago

@andersjohansson Please check if it works now

Works fine for me now!

snk0752 commented 3 years ago

popup is working properly on the multi-display setup in fedora34. need the working actor in overview mode. used gnome-40 branch to clone.

MonsieurLanza commented 3 years ago

Hello there, just tried the branch, thanks for the amazing work.

I have a somewhat special issue here, linked to this feature : switcher popup disappear with modifier key release and more interactive (like ctrl+alt) (fixes #145 and fixes #88)

So, if I use the « real » shortcut (here ctrl + alt + direction key) everything works as expected, but as I use a custom keyboard, I configured some shortcuts with 1 key press that just send simultaneous ctrl alt + arrow the time the key is pressed.

Result is I never see the switcher popup, fake « modifier keys » are released way too fast for it to appear, and I get lost in my workspaces. Could we just at least keep the popup the whole time of the switch animation ?

ebeem commented 3 years ago

@MonsieurLanza Let me see what we can do about this, maybe I can apply a custom delay for popup timeout (same as what we used to have before) while still keeping this behavior of releasing the modifier keys.

recolic commented 3 years ago

Hi, I tested this branch on Archlinux gnome 40 just now. It's awesome and working very well, but I noticed a tiny problem: When I use the keyboard shortcut to move window one workspace to the left/right/up/down, the workspace preview(popup) is expected to appear (to tell me where I am). But nothing happens.

This feature was available in the previous version of wsmatrix (gnome 3.38).

ebeem commented 3 years ago

@recolic it should appear, based on your other comment it seems like it's a keybinding issue (conflict) is it the case? if so, it will be customizable, but it's not yet.

recolic commented 3 years ago

@ebeem Hi. This is all my keybindings.. I don't think there's a conflict, and everything is working before the upgrade..

ctrl+alt+arrow = switch to another workspace
shift+alt+arrow = move current window to another workspace
shift+meta+arrow = move current window to another monitor
ebeem commented 3 years ago

This PR implements the popup in a different way, it's not possible to go back to the old implementation of keybindings now because it works in a different way with this PR. now it's similar to switching input for example, you have a modifier key that you press and hold then you can control the popup, you release it and them it will disappear.

The current keybindings are hard-coded (the default gnome's ones), so if you change them, it's expected to not work properly. I will need to fix the keybindings customization part in this PR then you will be able to use your customization.

recolic commented 3 years ago

I tried to read the source code, but failed to understand how does you have a modifier key that you press and hold then you can control the popup, you release it and them it will disappear. working. There's no ctrl or modifier key in _keyPressHandler, so I gave up...

Anyway, thanks for your work!

ebeem commented 3 years ago

Right now, this will work perfectly for default keybindings Ctrl+Alt+Arrow to switch workspaces Ctrl+Alt+Arrow to move windows to workspaces

If this works fine for you (the default keybindings) then it should work once the hard-coded part gets fixed. Can you please confirm?

Make sure you hold the Ctrl+Alt and just click arrows with or without Shift This is the current behavior.

recolic commented 3 years ago

Yeah. I can confirm that it's working. But the following issue still exists:

  1. Any tiny mouse movement would cause an incorrect workspace swtiching. (Releasing ctrl before alt sometimes cause the same error)
  2. I have to release ctrl to make the popup disappear, which prevents me from quick-reading the document while coding...

The following issue is resolved after using new keybinding:

  1. There's no popup while moving window to another workspace with ctrl+shift+arrow. (Using ctrl+alt+shift+allow resolves this issue)
ebeem commented 3 years ago

@recolic glad to hear that, Let me see what I can do about making the release of the modifier key optional, I didn't expect some users to have issues with it so I will try to implement both old approach and new one and have it as an option in the preferences.

Regarding second point, it's already tracked in

thanks for testing and reporting, this is really helpful as it will help us ensure the PR is ready for production before merging.

wedeluxe commented 3 years ago

I would also love to see the old timer-based behavior back as an option. Without doing a deeper research on the topic, I'm assuming there are a lot (corner) cases, that do not work well with the new approach (waiting for release of modifier keys).

As mentioned above, I'm using Super+<arrows> to switch and Super+Shift+<arrows> to move windows.

And by the way, during my daily workflow I used a lot the old feature to open the popup with Super+W. This gives a great overview of where you are and whats around without the need to actually switch workspaces. Any chance of getting this feature back, too?

ebeem commented 3 years ago

fixed

ebeem commented 3 years ago

@wedeluxe your problem with the portrait orientation is not something that I can look at right now since I don't have a similar setup. I think I can do a VM for this, but that won't be an effective solution. I am pretty sure it has something to do with workspaceSwitcherPopup.js in vfunc_allocate or the inherited functions vfunc_get_preferred_height, vfunc_get_preferred_width. Maybe somebody who is interested want to take a look at them.

recolic commented 3 years ago

Hi. I noticed that commit 0d252fdb6798641457e56ed08a2e2dc4c6932862 introduces a new issue:

Press and hold CTRL+ALT
Press arrow (to switch workspace)
[Now popup appears and never disappear]
Press arrow again (to switch to another workspace)
[Now popup disappears and never appear]
Press arrow again (to switch to another workspace)
[Now popup appears and never disappear]
Press arrow again (to switch to another workspace)
[Now popup disappears and never appear]
...

It seems that something went wrong with the timeout implementation...

ebeem commented 3 years ago

Thanks @recolic I think this was fixed in last commit, the first time you show switcher by moving didn't show the popup, but when u move before the popup disappears everything works as expected, can you try now so I can be more confident that we are talking about the same problem? I didn't get the exact scenario you provided.

There was a new problem introduced, I am not sure how this can be fixed because the code seems fine, basically in the log an error message will be shown every time the switcher disappears. Source ID 5458 was not found when attempting to remove it

Seems like a timeout id is being removed twice or something.

recolic commented 3 years ago

I tested the latest commit just now. I seems that the popup up doesn't appears at all...

I'll record an video to better explain the scenario, just a moment..

(it seems that our test results don't match)

recolic commented 3 years ago

Note that I'm using Ctrl+Alt+Arrow to switch workspace.

After commit 0d252fdb6798641457e56ed08a2e2dc4c6932862, the issue is shown in this video https://photos.app.goo.gl/9T9hEBbrXdfM5jMQ7 (popup never disappear, and then never appear, and then never disappear, and then never appear, loop...) After latest commit 37c159818d3bb247854e65407a84bd942ecdc29e, the issue is shown in this video https://photos.app.goo.gl/xL891xjkVou9G4Pr7 (popup doesn't appear at all)

If you have trouble accessing Google Photo, let me know and I can provide a direct http link.

Note that my gnome shell version is 1:40.0+67+g79acae417-1, I'm using archlinux. I tried to reboot my computer and problem still exists.

ebeem commented 3 years ago

It seems like something is wrong with the configs but I can't be sure what. Can you try adjusting your popup timeout from preferences? if you make it 0, you should go back to the old behavior (release to remove the switcher popup). If you have it > 0, then it will be similar to the behavior prior to gnome 40

ebeem commented 3 years ago

I use arrows btw, and it's totally working fine.

ebeem commented 3 years ago

Also remember that the popup timeout is in ms, if you have it a very low number <50, you will probably not be able to easily notice it.

recolic commented 3 years ago

Also remember that the popup timeout is in ms, if you have it a very low number <50, you will probably not be able to easily notice it.

You're correct, it works fine now. But I noticed a very strange behavior. I'm creating a chart and will show it soon

recolic commented 3 years ago

I carefully tested different value for popup timeout, and noticed that, if timeout < 153ms, the popup would not appear at all. (looks like some race condition) if timeout = 0, the popup would always appear and never disappear.

however this is not a important issue. just set timeout to more than 300ms and it's working excellently.

Screenshot from 2021-05-10 15-16-32

Screenshot from 2021-05-10 15-14-07

edit: it should be probability instead of possibility

ebeem commented 3 years ago

Thanks, there's a delay of 150ms to show the switcher itself, I simply added 150ms for the popup timeout, it should be fine now. Can you please confirm if the popup timeout is set to 0? this should make the switcher disappear if you release the keys only, as long as you hold Ctrl it won't disappear.

recolic commented 3 years ago

Thanks, there's a delay of 150ms to show the switcher itself, I simply added 150ms for the popup timeout, it should be fine now. Can you please confirm if the popup timeout is set to 0? this should make the switcher disappear if you release the keys only, as long as you hold Ctrl it won't disappear.

I can confirm that, if the popup timeout is set to 0, the switcher disappear if you release the keys only, as long as you holdCtrlit won't disappear..

ebeem commented 3 years ago

I found the guildelines page of gnome-shell javascript, it was not on gnome-shell's page, it was on gjs's website. https://gjs.guide/guides/gjs/style-guide.html#rule-list

I think we should update readme and include this in a separate PR