Stretched phone icon when using themes

[ofuangka]

First off, thanks for all the work in such a useful app.

I think there's a bug with the User Menu when themes are applied. The phone and the buttons are stretched wide:

This is with the Materia-dark-compact theme. It also happens with the Arc-Dark theme. It doesn't happen when you choose to Display devices in their own panel menu:

I realize it's asking a lot to have this work across all themes, but who knows, maybe it's a quick fix.

[andyholmes]

I might be possible to fix. This doesn't happen for me on Arc-Dark, however my testing version might be a bit old.

I'm not sure any themers have noticed this extension, but there is a file ~/.local/share/gnome-shell/extensions/gsconnect@andyholmes.github.io/stylesheet.css (also here) which holds the CSS for the Shell UI. It's fairly explanatory and comprehensive. A themer should be able to use those classes to ensure the extension looks right in their theme, or you can try editing it manually to see if it's a CSS problem.

Otherwise Ill try and come back to this later as I'm pretty deep into some other parts of GSConnect right now.

[ofuangka]

Thanks, I played around a little more and now think it's actually related to either the Materia gnome-shell theme or gnome-shell itself. I had switched to Arc for the gtk theme, but the Materia theme was still applied for gnome-shell which was why I thought it was both themes.

This message appeared in my journallog:

gnome-shell[3941]: setup_framebuffers: assertion 'width > 0' failed

Someone probably forgot to carry the one

[andyholmes]

I've marked this as upstream for now, since I think it is, but I'm going to keep this open. I'd still like to address this when I get time and reuse this issue to track popular themes that have problems with GSConnect.

BTW, that last error I've also seen, and I wondered if it was GSConnect causing it, but I've seen it brought up in #gnome-hackers in reference to mutter, so I'm not sure. It's non-critical anyways, though :)

[kppmsd]

With gnome-shell 3.28, everything in 'Mobile Devices' menu is stretched exactly like in the first screenshot, even when using the default shell theme.

I'm not sure if that's related to the original issue, but thought this would be appropriate place to report it..

[andyholmes]

Okay, definitely a bug then. Unfortunately I'm not on 3.28 yet, but I'll have a look into this tomorrow when I get time. Maybe it was just an oversight/istake that didn't manifest in previous Gnome Shell versions that caused me to miss it.

[stark-dev]

Hi! I've just upgraded desktop to 3.28, I can confirm it shows the same problem. I also use Arc-Dark, but it happens even with default theme. Just a strange thing I noticed, I have text alignment problems also with "Media Player Indicator" extension which definitely works on 3.26. I will report it to extension developers, but this could indicate that something has also changed in upstream.

If you need any further info, tell me ;)

Thanks, Mauro

EDIT: an issue for Media Player Indicator has been already opened here

[kppmsd]

This seems to be the commit causing this on 3.28: https://gitlab.gnome.org/GNOME/gnome-shell/commit/fb9db4e171778a63bd85ac6fde15b7acfdede2c2

I confirmed this by reverting the change by adding the following to the end of my shell theme css-file:

  .aggregate-menu .popup-sub-menu .popup-menu-item :first-child:ltr {
    padding-left: 0;
    margin-left: 0; }
  .aggregate-menu .popup-sub-menu .popup-menu-item :first-child:rtl {
    padding-right: 0;
    margin-right: 0; }

(Note that this is definitely not the right way to fix the issue, the margin and padding should be removed from gsconnect menu items specifically)

I suppose some themes similarily add padding and margin, resulting in similar glitch in gsconnect menu.

[andyholmes]

That's a good find. There is stylesheet for GSConnect in the top-level of the extension dir called stylesheet.css. If you wouldn't mind testing, what happens if you add to that file:

.aggregate-menu .popup-sub-menu .gsconnect-device-box :first-child:ltr {
  padding-left: 0;
  margin-left: 0; }
.aggregate-menu .popup-sub-menu .gsconnect-device-box :first-child:rtl {
  padding-right: 0;
  margin-right: 0; }

[stark-dev]

Hi, just tried it with default theme and Arc-Dark, works perfectly with both! Thanks again,

Mauro

[kppmsd]

That fixes the stretched menu items, and I think everything looks as it should.

Also, I think you can safely simplify the css to:

.aggregate-menu .popup-sub-menu .gsconnect-device-box :first-child {
  padding: 0;
  margin: 0; }

[andyholmes]

I still don't have 3.28 installed myself, so I can't test, but in 3.26 the padding of the first plugin button (usually SMS) is affected by this. I'm not sure if this is a typo by fmuellner or not but:

.aggregate-menu .popup-sub-menu .popup-menu-item :first-child:ltr {
any descendent that is a 'first-child' ---------^

Whereas:

.aggregate-menu .popup-sub-menu .popup-menu-item:first-child:ltr {
only the 'first-child' of .popup-menu-item -----^

Does this affect the first plugin button's padding for either of you?

[stark-dev]

Hi, I can confirm that's not a typo, the space is needed otherwise icons appear stretched again.

Here's how it looks with

.aggregate-menu .popup-sub-menu .gsconnect-device-box :first-child {
  padding: 0;
  margin: 0; }

Thanks,

Mauro

EDIT: may I suggest to add some margin? Personally I think that leaving 0 margin makes icons too packaged. Here's how it looks with 1px margin:

I never used css files, so maybe it could not be a good solution, since it is resolution dependent. If so, just leave 0, I will change it locally ;)

[andyholmes]

Oh see, that's what I mean. I believe this was a typo in the git commit linked above, not the CSS snippets. In the git commit, all descendants that are the first-child of any container are affected by the padding/margin changes.

This is relevant CSS (for GSConnect):

.gsconnect-plugin-bar {
    spacing: 6px;
}

.gsconnect-plugin-button {
    border: none;
    border-radius: 16px;
    padding: 8px;
}

This is the heirarchy:

.gsconnect-device-box [ PopupMenu.PopupBaseMenuItem ]
    StButton.gsconnect-device-button <----- first-child (should be the only one affected)
        StDrawingArea <----- first-child
    StBoxLayout.gsconnect-control-box
        StBoxLayout.gsconnect-title-bar <----- first-child
            StLabel.gsconnect-device-name <----- first-child
            StWidget.gsconnect-title-separator
            StBoxLayout.gsconnect-device-battery
                StLabel <----- first-child
                StIcon
        StBoxLayout.gsconnect-plugin-bar
            StButton.gsconnect-plugin-button <----- SMS is first-child
                StIcon <----- first-child
        StBoxLayout.gsconnect-status-bar
            StLabel <----- first-child

I believe the idea was that the "first" first-child is given extra padding so menu labels line up in the User Menu (aggregate menu), but because it's popup-menu-item :first-child which means "any first-child that is a descendant of .popup-menu-item" instead of popup-menu-item:first-child which means "the immediate first-child of a .popup-menu-item", more elements are affected than should be.

I've pinged fmuellner about this, but he hasn't got back to me yet to confirm.

[isantop]

Having anything other than 0 in the Pop Gnome Shell theme causes the icons to be stretched. I've gone ahead and put that fix into the stylesheet there. (Don't need to make any other changes really because since the extension is well written the rest of the stuff looks correct ootb)

[stark-dev]

Oh! I see, I'm sorry. I completely misunderstood, though I had to modify the gsconnect file :sweat_smile: Thanks a lot for your explanation about stylesheets, now it's definitely more clear how they work! I switched to standard Adwaita theme and I was looking for its gnome-shell.css. I couldn't find it, there is only a /usr/share/gnome-shell/theme/gnome-classic.css. I tried to change it, but nothing changes in the shell appearance. If I understood well, theme config is now stored in a .gresource file, and .css file should not be changed. However, gnome-shell.css can be modified for Arc-Dark theme, and adding

  .aggregate-menu .popup-sub-menu .popup-menu-item :first-child:ltr {
    padding-left: 0;
    margin-left: 0; }
  .aggregate-menu .popup-sub-menu .popup-menu-item :first-child:rtl {
    padding-right: 0;
    margin-right: 0; }

to that file works as expected (it completely removes padding/margin in any entry of the aggregate menu).

I'll try to dive a bit more into gnome stylesheets, hope I'll find something useful.

Thanks again and Happy Easter!

Mauro

EDIT: I forgot one important thing, I tested both the snippet that affects all first children and also the other one on Arc-Dark gnome-shell.css. In the first case (with zero padding on all first child items) icons are shown correctly (although not aligned to the menu), while in the other case are stretched. I'm not sure how settings are overridden, maybe when 0 padding affects only "first" first child, the other items use default gnome shell padding, which I wasn't able to change

[kppmsd]

@stark-dev Indeed, there's no easy way to edit the default shell theme. You'd have to edit the source code and rebuild gnome-shell.

@andyholmes If I understand correctly .popup-menu-item :first-childselects 'the first child of .popup-menu-item', while .popup-menu-item:first-child selects 'the popup-menu-item that is the first child of its parent'.

If you look at the upstream bug report https://bugzilla.gnome.org/show_bug.cgi?id=706191, the first proposed patch by Florian Müllner doesn't have :first-child selector. The latter patch (that got merged) adds them to "Fix focus/hover highlight".

I think the purpose of :first-child is merely to make the selector pick-up whatever is inside of the .popup-menu-item, instead of the sub-menu-item itself.

I tested copying the aforementioned upstream commit at the end of my shell theme (Arc), and changing .popup-menu-item :first-child to .popup-menu-item:first-child, and that messes up the padding of every first item in all sub-menus:

Removing the :first-child selectors completely, results in every sub-menu item having the same padding glitch as the first item on the screenshot.

[andyholmes]

I think the purpose of :first-child is merely to make the selector pick-up whatever is inside of the .popup-menu-item, instead of the sub-menu-item itself.

Ah, right you are. I didn't think of it before, but I copied the gnome-shell.css patch to the top of stylesheet.css and was able to test on 3.26. I think the proper way for Gnome Shell to have done this would have been:

.aggregate-menu .popup-sub-menu .popup-menu-item > :first-child { ... }

For me at least, this has intended effect of fixing the label padding & hover bug reported, and it could be overridden easily with:

.aggregate-menu .popup-sub-menu .gsconnect-device-box > :first-child { ... }

Unfortunately, even if that were changed it wouldn't make it into circulation until at least 3.28.1; this is the simplest I've been able to come up with that solves the problem, without affecting the buttons and keeping compatible with 3.26:

.aggregate-menu .popup-sub-menu .gsconnect-device-box :first-child {
    padding: 0;
    margin: 0;
}

.aggregate-menu .popup-sub-menu .gsconnect-plugin-button:first-child {
    border: none;
    border-radius: 16px;
    padding: 8px;
}

For extra points, it's forwards compatible with the amended CSS I recommended to fmuellner and it fixes the bizarre habit folks seem to have of applying Python's PEP-8 style guide to CSS ;)

[isantop]

The PEP-8 style curly brackets in CSS are bad, but when you put them into SCSS the generated CSS becomes nearly impossible to read. Thanks for contributing to the correct way! :+1:

[uniquePWD]

I see the fix for this was pushed on April 1st, anyone know why it hasn't made its way to extensions.gnome.org?

[stark-dev]

v11, which contains the fix, it's currently waiting for review on Gnome Shell Extensions. This does not depend on the author. I installed last version by cloning the repository and building it locally, but there is also a compiled v11 zip archive, downloadable in the wiki ;)

[uniquePWD]

Thanks @stark-dev, it may be a good idea to reopen this until the update has been reviewed.

[stark-dev]

I'm afraid that reopening it here would mean that the bug is not fixed and this would have a negative impact on the developer, which has closed the bug more than a month ago. I guess the only thing to do is to wait for GNOME guys to review it asap. I know it's annoying to wait, but I'm sure that all the people are making their best to bring new features and fixing bugs. Probably, since there is a simple workaround it's taking more time, or it could be that there is a mandatory review time before it can be published on extension website. This extension is getting more and more popular, I'm sure they will upload it, sooner or later ;)