Fix popup styling

[pnkov]

See #152 (comment). Fixed some miscalculations in sizes of popup items.

• magic offset for thumbnail reliable for several rows, cols, scales and themes variations
• changed getWorkAreaForMonitor to monitors due to popup "jumping" when switching between full screen and normal work spaces (can not test multi-monitor setup until monday)

[mzur]

@pnkov Were you able to test the multi-monitor setup?

@ebeem Do you approve?

[ebeem]

I am still not sure what the problem is actually. getWorkAreaForMonitor is used by GNOME to display the switcher popup, I have never faced this issue before and I can't reproduce it myself. I don't see any jumping in the popup.

Can you please test whether this happens without the extension or not? and please provide more information about the meaning of a fullscreen space, I understood this as a workspace that has a fullscreen application running.

unlike using monitors, getWorkAreaForMonitor will only return the usable area in the monitor, so it will reduce the size of the monitor to the size of the space usable by the shell (monitor size - shell fixed widgets).

I would like to know more first about the problem.

[pnkov]

Sorry for delay. I've tested extension with two monitors with different resolution and found that _monitorIndex was always undefined. So popup size calculates wrong. Now I fixed it by providing index from Popup to PopupList.

Don't know why I had an issue with getWorkAreaForMonitor. For some reason it was changing while switching, but now everything seems good for me. So I've reverted this part.

[ebeem]

@pnkov I think passing monitorIndex is not needed anymore, please let's remove it so we don't change files unnecessarily. We want to make sure that this magic offset is not coded into your theme, we prefer always having all style inherited from the gnome theme (which is the current approach). The styling is not very good in some themes, but in others it's actually pretty good. The way I envision it to be is similar to what's mentioned here. Basically the style is inherited, but the options can be customized and overwritten.

@mzur please give us your thoughts as well, meanwhile I will try to test the changes against multiple themes and see if there are any problems with the themes.

Again, this issue isn't easy to solve as themes don't follow a specific standard, and the properties like padding/margin/borders can be set to any values which can make any style look very unprofessional ins some themes (visit the issue #12 to know more).

[ebeem]

@pnkov Is it possible to also include some screenshots of the miscalculated value? I tried with some themes and I can see some compactness in the popup, the image you showed in the PR seems to be using a customized shell theme. Can you confirm if you face it in other themes as well? like Adwaita (default)?

I started guessing that this is a theme issue similar to #12 which we have been discussing for a while but with no real reliable solution as customizing the styling for some properties might have side-effects on other themes. Also, these styling properties you changed might only be an issue with your theme if the popup looks fine on other themes, meaning you can probably either fix your theme's css if possible or keep these changes to your local repo as it doesn't actually affect others.

Please let me know whether you think this problem is global or only specific to your theme so we can further discuss possible solutions, the proposed solution in #12 seems good to me, we can also always expose classes to be customized in a css file (inherited from the extension and the shell theme).

Check the screenshots below of the popup with and without this PR.

Before changes

After changes

[pnkov]

So I switched to Adwaita theme and done some testing. About monitorIndex. I've tested gnome-40 branch and scale calculates based on zero indexed monitor for both of popups. In general I forgot to mention initially that issue better seen with high rows count.

Here some screenshots:

• gnome-40 branch
• gnome-40 branch with monitorIndex commit
• and this pr branch

[ebeem]

Are you sure that passing the monitor index makes difference? This doesn't make sense because the WorkspaceSwitcherPopupList extends BoxLayout BoxLayout and WorkspaceSwitcherPopupList don't use monitorIndex at all. Maybe your styling made some difference, but I believe this has nothing to do with the monitor index. Can you please check this and explain to me the use of it?

[pnkov]

@ebeem Yes I'm sure. As I mentioned before, my second screenshot is up to date gnome-40 branch and only difference is passed monitor index. It used by get_preferred_child_size here.

So issue reproduces only when using multiple monitors with different resolutions. Because Main.layoutManager.getWorkAreaForMonitor(undefined) returns same work area as Main.layoutManager.getWorkAreaForMonitor(0) and in my case children size calculated based on 2560x1440 monitor for 1920x1080 as you can see on second screenshot.

[ebeem]

@pnkov correct, thanks for the explanation. I will test these latest changes ASAP on the list of themes I have so we can merge it into gnome-40

I appreciate your contribution!

[ebeem]

Tested, looks great! Thanks for your help.

@mzur I tested the configured spacing on many themes and it looks fine, I still think we need to allow customization of styling at some point to ensure the popup will look good on all different themes and setups. However, I think this PR is a good fix for the inherited styling and bad spacing, doesn't seem like it will have any side effects and it will cover all cases.

Please test it and consider merging it into gnome-40

[mzur]

Thanks a lot @pnkov!