New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Widgets misaligned in Gnome 3.28 #448

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

Comments

Projects
None yet
@IBBoard
Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link

glaubersm commented Mar 19, 2018

Confirmed on Arch Linux.

@JasonLG1979

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Owner

JasonLG1979 commented Apr 3, 2018

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

@WyRe

This comment has been minimized.

Copy link

WyRe commented Apr 4, 2018

@JasonLG1979

So this media player extension will be discontinued?

@JasonLG1979

This comment has been minimized.

Copy link
Owner

JasonLG1979 commented Apr 4, 2018

No

@andyholmes

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link

imkk-000 commented May 8, 2018

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

@Zeka13

This comment has been minimized.

Copy link

Zeka13 commented Jun 4, 2018

plz fix in master

@NellyWhadsDev

This comment has been minimized.

Copy link

NellyWhadsDev commented Jun 8, 2018

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

@bpresles

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Owner

JasonLG1979 commented Oct 11, 2018

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

@fcastilloec

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

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

Bug #448 alignment tweak tweak (#460)
* 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

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Contributor

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 join this conversation on GitHub. Already have an account? Sign in to comment