GNOME Bugzilla – Bug 604943
St.Widget should add_style and remove_style helper methods. Update current style_classes used as "pseudo-states" to use this.
Last modified: 2010-03-24 13:48:11 UTC
Since HTML elements / widgets can have multiple styles by having space-separated CSS class names, add_style and remove_style methods should be added to the St.Widget classes to make multiple styles more accessible. We should also update gnome-shell to use add_style and remove_style for things that require state (think ".app-well-app-running" => ".app-well-app.running") so that some form of style inheritance can be achieved.
Created attachment 156799 [details] [review] st-widget.h: fix spacing
Created attachment 156800 [details] [review] [StWidget] add list-like methods for style_class and pseudo_class Since style_class and pseudo_class are space-separated lists of names, add new methods to add and remove individual names rather than just re-setting the entire name. Update existing code to use the new methods where appropriate. In some cases, this may result in actors having multiple pseudoclasses where previously they only had one at a time, but there don't seem to be any visible differences.
wsa working on this as part of the hover-related changes in bug 610726
Review of attachment 156799 [details] [review]: Looks OK (I tend to only align headers in sections to avoid mass-scale reindentation when adding a function with a long name or parameter, so I'd probably just have fixe up the direction functions rather than redid the whole thing, but whole thing is OK too.)
Review of attachment 156800 [details] [review]: Looks good. Maybe you can add some basic tests into test-theme.c? ::: src/st/st-button.c @@ -142,9 +142,1 @@ - StButtonPrivate *priv = button->priv; - - if (priv->is_checked) ... 6 more ... + st_widget_remove_style_pseudo_class ((StWidget*) button, "active"); Might require CSS file modifications to make sure that things were ordered and interacted properly, but we don't really use any of this stuff in our CSS file. ::: src/st/st-scroll-bar.c @@ +724,3 @@ if (target != bar->priv->handle) { + st_widget_remove_style_pseudo_class ((StWidget*) bar->priv->handle, "hover"); Yikes, this should have been st_button_recheck_hover() or something rather than poking around at the pseudo-class. No worse with your patch, of course. ::: src/st/st-widget.c @@ +1363,3 @@ + while (class_list) + { + while (*class_list == ' ') st-theme.c uses cr_utils_is_white_space() not ' '. As far as I can determine, cr_utils_is_white_space() is identical to g_ascii_isspace() - [ \t\f\r\n] but not \v.
(In reply to comment #5) > if (target != bar->priv->handle) > { > + st_widget_remove_style_pseudo_class ((StWidget*) bar->priv->handle, > "hover"); > > Yikes, this should have been st_button_recheck_hover() or something rather than > poking around at the pseudo-class. No worse with your patch, of course. the hover-tracking patch in 610726 adds st_widget_sync_hover(), we can fix it there.
Created attachment 156928 [details] [review] [StWidget] add list-like methods for style_class and pseudo_class to test-theme.c
I was expecting tests just that tried adding and removing pseudo-classes in various ways and check get_style_class(), wasn't necesarily expecting to test the interaction with the assignment of styles to nodes. :-) Looks fine though.
that would have required writing new assert_* methods :-)
Attachment 156799 [details] pushed as 7c37e94 - st-widget.h: fix spacing Attachment 156928 [details] pushed as 909b5ec - [StWidget] add list-like methods for style_class and pseudo_class