GNOME Bugzilla – Bug 612605
CSS rule style
Last modified: 2018-07-11 17:05:48 UTC
We currently have an inconsistent mix of names and style_classes in our CSS, and inconsistent naming of names and classes. Proposed style guide rules: * CSS names and classes should use a "hyphenated-lowercase-words" style. When appropriate, they should be based on the names of their corresponding JS delegate classes. [Currently most of our rules use hyphenated-lowercase-words, but some use camelCase or PascalCase instead.] * Use name-based rules for actors that are unique within gnome-shell, such as the message tray: this.actor = new St.BoxLayout({ name: 'message-tray' }); [This implies you *must* use name in this case. Do we want that, or should we just say that you *can't* use name if it's not unique, but you can use either name or class for unique actors?] * Use class-based rules for non-unique actors, such as a single notification within the message tray: this.actor = new St.Table({ style_class: 'notification' }); * Avoid names and classes that are too generic ('add', 'remove'); use a consistent prefix on related classes to disambiguate. ('workspace-add', 'workspace-remove') [Currently the workspace remove button has class 'workspace-controls add', but that seems bad IMHO because it could accidentally end up picking up rules for some other "add" button. It should be (according to this rule) 'workspace-controls workspace-add'.] Once we agree on what the rules are, we can fix the existing code to match. Also, not completely related, but I remember Owen saying at one point that you shouldn't do ".style-class-name StWidgetClassName", although it seems to me that this shouldn't be any less efficient than anything else--it always has to look at the entire chain of actors above you to compute your CSS anyway, right? But if I'm wrong, or if there are other efficiency concerns, we should document those too.
Makes sense to me (both the proposal of adding rules to the style guide and the proposed rules themselves). (In reply to comment #0) > * Use name-based rules for actors that are unique within gnome-shell, > such as the message tray: > > this.actor = new St.BoxLayout({ name: 'message-tray' }); > > [This implies you *must* use name in this case. Do we want that, > or should we just say that you *can't* use name if it's not unique, > but you can use either name or class for unique actors?] For me, the important part is to not use name on non-unique actors (StScrollBar/StScrollBar come to mind), no strong opinion on making the use of name for unique elements a requirement - I'd prefer name over style-class for unique actors, but I'm not sure whether it's something that should be "enforced". > Also, not completely related, but I remember Owen saying at one point that you > shouldn't do ".style-class-name StWidgetClassName", although it seems to me > that this shouldn't be any less efficient than anything else--it always has to > look at the entire chain of actors above you to compute your CSS anyway, right? If I understood Owen correctly, in the case of StWidgetClassName the GObject-hierarchy of each actor has to be examined in addition to the clutter chain (because we want StWidgetChildClassName to match as well).
(In reply to comment #1) > > Also, not completely related, but I remember Owen saying at one point that you > > shouldn't do ".style-class-name StWidgetClassName", although it seems to me > > that this shouldn't be any less efficient than anything else--it always has to > > look at the entire chain of actors above you to compute your CSS anyway, right? > > If I understood Owen correctly, in the case of StWidgetClassName the > GObject-hierarchy of each actor has to be examined in addition to the clutter > chain (because we want StWidgetChildClassName to match as well). No, that isn't the problem I was talking about - we do the GObject-hierarchy match efficiently using GObject type check methods ... if you have a rule "StBoxLayout { }" - that basically boilds down to ST_IS_BOX_LAYOUT(). The problem is (as described above) that we can't reject the rule based only the actor itself, we have to check parent actors in the layout hierarchy.
but don't you *always* have to check parent actors in the layout hierarchy?
(In reply to comment #3) > but don't you *always* have to check parent actors in the layout hierarchy? You have to resolve the style for the parent actor before resolving the style for the child actor to deal with inheritance, but that's very different from having to match against the parent actor for individual rules. (There are likely sophisticated approaches to resolving CSS rules that could be much more efficient in some cases - it might be interesting to look at the browser CSS engines if we find CSS matching to be a big bottleneck.)
The style was completely rewritten in 3.16, which cleaned up a lot of the pain points. And while still nobody has spend time on codifying the rules, the designers don't seem overly bothered with it - if they supply us with such guidance, I'm happy to add it in the repo, but an old bugzilla bug is unlikely to make it happen, so closing it now (and thus exclude it from a future gitlab migration).