Skip to content
This repository has been archived by the owner. It is now read-only.

Widgets misaligned in Gnome 3.28 #448

Closed
IBBoard opened this issue Mar 17, 2018 · 34 comments
Closed

Widgets misaligned in Gnome 3.28 #448

IBBoard opened this issue Mar 17, 2018 · 34 comments

Comments

@IBBoard
Copy link
Contributor

@IBBoard IBBoard commented Mar 17, 2018

I've just upgraded to Gnome 3.28 (openSUSE Tumbleweed) and the widgets are all offset to the right. Everything used to be centralised. I'm guessing that some default CSS changed somewhere.

mediaplayeralignment

@IBBoard
Copy link
Contributor Author

@IBBoard IBBoard commented Mar 17, 2018

I can't work out how to use LookingGlass to inspect the widgets, but I've managed to bodge together some CSS that hopefully shows something useful about the structure. The boxes on the left look like they shouldn't be necessary (but I'm guessing based on webdev and GTK3 work, not Gnome Shell extension knowledge!).

mediaplayeralignmentoutlined

@glaubersm
Copy link

@glaubersm glaubersm commented Mar 19, 2018

Confirmed on Arch Linux.

@JasonLG1979
Copy link
Collaborator

@JasonLG1979 JasonLG1979 commented Mar 20, 2018

It's never been truly centered because of how menus are laid out in Shell widgets.

I have yet to update to GNOME 3.28. When I do I see what I can do to fix this. If I had to guess they changed some style class names.

@IBBoard
Copy link
Contributor Author

@IBBoard IBBoard commented Mar 20, 2018

It used to look close enough that I never noticed it wasn't quite central!

If you can tell me how to debug the UI then I can give it a poke and submit a patch. I managed to get the second screenshot by adding * > * > * > * > … > * { border: 1px solid red}, because unlike GTK3 then I couldn't find a way to do a widget picker and find its details!

(I tried LookingGlass, but could only get the buttons in the top bar, but nothing in the opened menu)

@JasonLG1979
Copy link
Collaborator

@JasonLG1979 JasonLG1979 commented Apr 3, 2018

@IBBoard St Widgets(GNOME Shell UI widgets) kinda suck and They are also basically impossible debug and very inflexible most specifically the menus. Because of that most of the extension uses custom widgets that can be found here:

https://github.com/JasonLG1979/gnome-shell-extensions-mediaplayer/blob/master/src/widget.js

To fix the issue I'll basically need to go though and make sure all the stock style class names are still valid and double check all the alignment params by hand.

This extension is getting a little bloated and long in the tooth. It would be nice to get rid of as much custom widgets/hacky workaround code as possible.

@JasonLG1979
Copy link
Collaborator

@JasonLG1979 JasonLG1979 commented Apr 3, 2018

I've started a new MPRIS extension in the effort to maybe start fresh:

https://github.com/JasonLG1979/gnome-shell-extensions-mpris-indicator-button

The end goal of the new extension is to eventually more or less duplicate the Unity SoundMenu (minus the sound device controls). And to be as simple as possible with no config options.

screenshot from 2018-04-03 11-43-03

@IBBoard
Copy link
Contributor Author

@IBBoard IBBoard commented Apr 3, 2018

Does the MPRIS system include ratings? I looked into MPRIS once when getting Quod Libet to work with some MPRIS tools, but I can't remember which bits were and weren't in the standard and which were extensions.

The main thing that I use this extension for is rating tracks (and occasionally looking at the details). I've got keyboard shortcuts for play/pause and skipping, but not for rating!

@JasonLG1979
Copy link
Collaborator

@JasonLG1979 JasonLG1979 commented Apr 3, 2018

The MPRIS spec supports getting ratings from players but not setting ratings. Setting ratings in this extension is a bit of a hack and requires player specific code.

@JasonLG1979
Copy link
Collaborator

@JasonLG1979 JasonLG1979 commented Apr 3, 2018

Some players have dbus interfaces for setting ratings some we just use the command line.

@jfernandz
Copy link

@jfernandz jfernandz commented Apr 4, 2018

@JasonLG1979

So this media player extension will be discontinued?

@JasonLG1979
Copy link
Collaborator

@JasonLG1979 JasonLG1979 commented Apr 4, 2018

No

@andyholmes
Copy link

@andyholmes andyholmes commented Apr 4, 2018

This came up in my extension as well, the offending commit was here:

I submitted an MR that was accepted yesterday, making it easier to address when it lands:

And here's a rough example of how I fixed it (also forward compatible with the MR which should make it in 3.28.1):

/* Reset the margin/padding changes to 0 for the submenu item :first-child's */
.aggregate-menu .popup-sub-menu .custom-submenu-class :first-child {
    padding: 0;
    margin: 0;
}

/* Apply something like this to any :first-child that needs non-0 margin/padding */
.aggregate-menu .popup-sub-menu .custom-childitem-class:first-child {
    padding: 8px;
}

(credits due to the folks on my issue that helped track this down)

@IBBoard
Copy link
Contributor Author

@IBBoard IBBoard commented Apr 4, 2018

I can confirm that it fixes it for me for this extension as well (although it needs improving).

If I add:

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

to .local/share/gnome-shell/extensions/mediaplayer@patapon.info/stylesheet.css and restart Gnome Shell then everything appears pretty much centralised (although the prev/pause/next buttons are too close together)

@IBBoard
Copy link
Contributor Author

@IBBoard IBBoard commented Apr 4, 2018

Okay, it's ugly and so I don't want to make it a PR, but this fixes it for me:

.aggregate-menu .popup-sub-menu > * > * > :first-child,
.aggregate-menu .popup-sub-menu > * > * > :last-child > :first-child,
.aggregate-menu .popup-sub-menu > * > * > :last-child > :first-child > :first-child
{
    margin: 0; padding: 0;
}

I'm sure there are better classes to work with, though! I just can't work out the widget structure and available/required classes from the JavaScript.

mediaplayerextensionfix

@andyholmes
Copy link

@andyholmes andyholmes commented Apr 4, 2018

You'll want to add a class name in between .popup-sub-menu and :first-child, otherwise you will be erasing the padding on all descendants that are the first-child of their container. Possibly #SubMenu, #Gjs_SubMenu or maybe Gjs_SubMenu, I can't remember how subclass CSS names work with St. In the original commit it's a .popup-menu-item, but if you use that class name it will override all .popup-menu-items, not just the extension's.

After that probably you'll want to re-add margin and padding to the buttons and any other elements that need it. I would just re-use the class names from stylesheet.css.

@IBBoard
Copy link
Contributor Author

@IBBoard IBBoard commented Apr 4, 2018

It does appear to break the other submenus at the moment (slightly), but given that I almost never use the "Wired Connection" or user details submenus whereas I look at the media player multiple times a day then I'll live with it as an initial fix!

If I had a decent DOM inspector then I could do better than my second attempt!

@fcastilloec
Copy link

@fcastilloec fcastilloec commented May 7, 2018

@IBBoard Thanks for that workaround!!! I also don't care too much about the submenus either, so it's a great fix. Nonetheless, if you have found a better way to fix this, do let us know, please! Thanks again for the great workaround.

@imkk-000
Copy link

@imkk-000 imkk-000 commented May 8, 2018

@IBBoard Thanks a lot. Your comment fixed me on Ubuntu 18.04.

@Zeka13
Copy link

@Zeka13 Zeka13 commented Jun 4, 2018

plz fix in master

@NellyWhadsDev
Copy link

@NellyWhadsDev NellyWhadsDev commented Jun 8, 2018

Is there a plan to submit this fix into master? Or is the approach non-ideal?

@bpresles
Copy link

@bpresles bpresles commented Jun 10, 2018

@IBBoard In my case the workaround is not centralizing correctly, the elements are too much on the left (instead of being too much on the right before the workaround).

image

@SamadiPour
Copy link
Contributor

@SamadiPour SamadiPour commented Oct 11, 2018

best fix was this one (from k3rnel):

.panel-media-indicator .popup-sub-menu > * > * > :first-child,
.panel-media-indicator .popup-sub-menu > * > * > :last-child > :first-child,
.panel-media-indicator .popup-sub-menu > * > * > :last-child > :first-child > :first-child {
    margin:  0;
    padding: 0;
}

add it to:

~/.local/share/gnome-shell/extensions/mediaplayer@patapon.info/stylesheet.css

here is the result

untitled

@JasonLG1979
Copy link
Collaborator

@JasonLG1979 JasonLG1979 commented Oct 11, 2018

Put in a pull request and I'll merge it.

@fcastilloec
Copy link

@fcastilloec fcastilloec commented Oct 11, 2018

@SamadiPour I tried your fix but it didn't work on Ubuntu 18.04, here's what it looks like with your code:
fix
So far only @IBBoard has worked for me.

@IBBoard
Copy link
Contributor Author

@IBBoard IBBoard commented Oct 11, 2018

That's odd, because @SamadiPour's changes look just like mine, but with a different (more accurate) initial selector!

@SamadiPour
Copy link
Contributor

@SamadiPour SamadiPour commented Oct 11, 2018

@fcastilloec
i'm on 18.04 also. gnome 3.28.
not sure why it didn't work for you
did you reset gnome?
press Alt+F2 write r press Enter

@IBBoard
yeah exactly. changes are same but other submenus won't break. atleast it works fine for me.

@JasonLG1979
Copy link
Collaborator

@JasonLG1979 JasonLG1979 commented Oct 11, 2018

It works if the extension is in indicator mode but not if the extension is in the system menu.

screenshot from 2018-10-11 13-19-03
screenshot from 2018-10-11 13-19-15

@SamadiPour
Copy link
Contributor

@SamadiPour SamadiPour commented Oct 11, 2018

@JasonLG1979
Thanks for noticing
i played with code
and here is the final solution
i can't pull request in master branch so please after approving it, change your code yourself
Thanks.
@fcastilloec please try this one

.panel-media-indicator .popup-sub-menu > * > * > :first-child,
.panel-media-indicator .popup-sub-menu > * > * > :last-child > :first-child,
.panel-media-indicator .popup-sub-menu > * > * > :last-child > :first-child > :first-child {
    margin:  0;
    padding: 0;
}

add it to:

~/.local/share/gnome-shell/extensions/mediaplayer@patapon.info/stylesheet.css

then open this file:

~/.local/share/gnome-shell/extensions/mediaplayer@patapon.info/extension.js

find this line:

menu = Main.panel.statusArea.aggregateMenu.menu;

press enter and add this line after it

 menu.actor.add_style_class_name('panel-media-indicator');

it shoulds look like this:

screenshot

here is the result

untitled

screenshot from 2018-10-11 23-09-02

@fcastilloec
Copy link

@fcastilloec fcastilloec commented Oct 11, 2018

@SamadiPour that did it! If you can't submit a PR, and @JasonLG1979 can't do this fixes. I can fork this repo, do the fixes you described here, and then push the PR. If everybody is ok with that.
All credit goes to you! Thanks!

@JasonLG1979
Copy link
Collaborator

@JasonLG1979 JasonLG1979 commented Oct 11, 2018

@SamadiPour

i can't pull request in master branch so please after approving it, change your code yourself

Stange, I would think you'd want credit for your work. That's why I asked you to put in a PR.

@IBBoard
Copy link
Contributor Author

@IBBoard IBBoard commented Nov 3, 2018

I've put in a tweak to fix the alignment at the pixel level. It wasn't really that noticeable, but the button size difference caught my eye and then I had to screenshot/crop/zoom/use guides in GIMP to make sure it was all just right!

Before (at 300% zoom with 50% guide):
alignment-current

After (at 300% zoom with 50% guide):
alignment-new

(Notice how the line fits the middle of the Transistor logo, the middle star and the pause button)

Difference:
alignment-diff

(Notice how four stars move consistently but the left-hand one shows more difference, and how the prev/next buttons move evenly but the pause button moves more significantly between them)

@IBBoard
Copy link
Contributor Author

@IBBoard IBBoard commented Nov 4, 2018

Sorry - there's another pull request! It's a tweak tweak.

After @JasonLG1979's comments about themes then I tried looking at it again. I think I've fixed everything now and done it in a better, more standardised way after I started to understand how the Gnome Shell Extension widgets worked a bit better.

JasonLG1979 added a commit that referenced this issue Nov 5, 2018
* Limit the CSS rules so that they only affect "Now Playing"

Previous rules affected ALL sub-items, which included playlists!

* Remove class that seems to apply to whole menu

(Debugging CSS showed ".panel-media-indicator" styles applying
to Network Manager!)

* Put padding back on small play buttons
@JasonLG1979
Copy link
Collaborator

@JasonLG1979 JasonLG1979 commented Nov 8, 2018

Thanks. I suck at css and it's always bugged me that no mater what I did things were just so ever slightly off center.

@IBBoard
Copy link
Contributor Author

@IBBoard IBBoard commented Nov 28, 2018

Unless anyone gives any other examples, I think my pull request fixed this.

@IBBoard IBBoard closed this Nov 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet