Implement Gtk CSS utility with button implementation #397
Conversation
src/gtk/toga_gtk/libs/utils.py
Outdated
| """ | ||
| res = selector + ' {\n' | ||
| for attr, val in decls.items(): | ||
| res += f'\t{attr}: {val};\n' |
There was a problem hiding this comment.
Just realized f-strings are Python3.6+ feature, I should have used .format()
…tests / Check for ValueErrors
| class TestCSSRuleFactoryMethod(unittest.TestCase): | ||
| def setUp(self): | ||
| self.single_line_property = {'color': '#f00'} # Red | ||
| self.multi_line_property = { |
There was a problem hiding this comment.
Just realized dicts don't have any order , causing tests to fail 😢
Edit: For (Python<3.6)
| from gi.repository import Gtk, Gdk | ||
| except ImportError: | ||
| import sys | ||
| # If we're on Linux, Gtk *should* be available. If it isn't, make |
There was a problem hiding this comment.
At column 5: (F811) redefinition of unused 'sys' from line 1
|
|
||
| # Use OrderedDict for Python < 3.6 | ||
| if sys.version_info < (3,6): | ||
| d = OrderedDict() |
There was a problem hiding this comment.
At column 33: (E231) missing whitespace after ','
| css_rule_factory('button', {}) | ||
|
|
||
| @unittest.skipIf(Gtk is None, "Can't run GTK implementation tests on a non-Linux platform") | ||
| class TestGTKApplyCSSMethod(unittest.TestCase): |
There was a problem hiding this comment.
At column 1: (E302) expected 2 blank lines, found 1
src/gtk/toga_gtk/libs/utils.py
Outdated
|
|
||
| import gi | ||
| gi.require_version('Gtk', '3.0') | ||
| from gi.repository import Gtk |
There was a problem hiding this comment.
At column 1: (E402) module level import not at top of file
src/gtk/toga_gtk/libs/utils.py
Outdated
| import gi | ||
| gi.require_version('Gtk', '3.0') | ||
| from gi.repository import Gtk | ||
| from toga.constants import LEFT, RIGHT, CENTER, JUSTIFY |
There was a problem hiding this comment.
At column 1: (E402) module level import not at top of file
src/gtk/toga_gtk/libs/utils.py
Outdated
|
|
||
| import gi | ||
| gi.require_version('Gtk', '3.0') | ||
| from gi.repository import Gtk |
There was a problem hiding this comment.
At column 1: (E402) module level import not at top of file
src/gtk/toga_gtk/libs/utils.py
Outdated
| import gi | ||
| gi.require_version('Gtk', '3.0') | ||
| from gi.repository import Gtk | ||
| from toga.constants import LEFT, RIGHT, CENTER, JUSTIFY |
There was a problem hiding this comment.
At column 1: (E402) module level import not at top of file
Replace depreceated api
ArchKudo
changed the title
There was a problem hiding this comment.
I haven't done a close-detail inspection of the tests yet; I've focussed on the main API - and I have some broader architectural questions.
In particular, I'm not clear (from the documentation you linked, combined with the usage here) what the lifecycle of a GTK style provider/context is intended to be. The CSS language GTK has implemented includes widget and class specifiers, but the API (especially as used here) seems to be specifically targeted at individual widgets.
So - what's the intended lifecycle here? Are you meant to apply a stylesheet to an entire application? Or is per-widget the intended usage? And if so - is the style applied to the "button" target on each button, or on "button#id-1234"? (i.e., what is the interaction of CSS's element targetting namespace and GTK's per-widget style application API?)
Is the only way to specify styles to load-from-string? Do we really have to pay a text parsing overhead every time we want to change a style attribute on a widget? That seems... wasteful.
Are styles additive? Or does a new style declaration replace an old one?
If they're additive (as would be suggested by the add_provider() API naming - can you modify an existing style context?
I'm not denying that this code works. I just want to make sure we're actually using the GTK API the right way - because the way it's being used right now, it seems like there's going to be a lot of object creation going on as you construct CSSProviders and get StyleContexts. And, if you were to switch (for example) the color of a widget back and forth between two values, the internal style definition of the GTK widget is going to become a long list of internal overrides.
| def set_background_color(self, value): | ||
| self.interface.factory.not_implemented('Button.set_background_color()') | ||
| if value: | ||
| gtk_apply_css(self.native, {"background-color": value}) |
There was a problem hiding this comment.
Is there a reason that this is an external utility method, rather than a method on the base toga-GTK widget?
The recommended way is to have a single application level CSS as we would have while creating a website, so in that sense toga doesn't exactly need it, since, it implements its own styling.. however, styling using python api was limited with GTK3 as, some of the theming API was deprecated in favour of CSS..
It applies to the specific widget only but, it is totally possible to have it the other way around depending on the selector...
Given the underlying API is in C, I can't find any other method either than to load CSS from a file or as a string.. 🤷♀️
Yes they are, and follow the CSS precedence rules, also setting priority to application always applies the most current change..
Oh! Totally agree, it probably didn't cross my mind, one solution I can think of is adding a class based UID to each CSS rule since a quick search on SO says there exists API to remove a css class though, I must admit this requires some more research on my side best not to rush it.. Do let me know if any of my responses aren't clear enough and, thanks a lot for reviewing! |
In which case - are there any performance considerations here? Is setting lots of individual per-widget styles any faster or slower than having a single application-level style?
Again - what are the performance considerations? Is it faster to set:
I'd be interested in seeing a performance comparison using a test-bed application - for example, an app with hundreds of buttons in a window, repeatedly changing a style back and forth on one of the buttons. This doesn't need to be a Toga application (in fact, it's probably better if it isn't - we're looking to verify the specific performance characteristics of GTK, not the Toga overhead).
I'm intrigued by the performance characteristics here, too. Is the internal structure retaining the full style definition and recomputing the "net" style after every addition? Or is it only retaining the "current" state of the style, and when you load a new style, determining if selectors affect the "current" style. Or something in between? The specifics of the implementation don't particularly matter - but if it's the former, I'd expect to see a performance difference between iteration 0-100 and iteration 100000 and 100100.
I can see how using a "unique class" as an identifier could be manipulated to our advantage - however, the use case isn't likely to be "reset style to default value", it's "overwrite existing style with new value". To that end, it's probably worth adding |
|
Hi there! It looks like this PR might be dead, so we're closing it for now. Feel free to re-open it if you'd like to continue. 😄 |
|
@ArchKudo This PR was a really good step towards us getting support for Button and Label font sizes, and I think you were like 90% of the way there. Is this something you are still interested in finishing? We have a sprint going on this weekend. Otherwise we could have someone create a new PR from where you left off. |
May 2019 update
set_enabledfrom toga_gtkButtonclass as it is already implemented inWidget(See PR Implement GTK button.enabled #703)References for reviewers:
https://developer.gnome.org/gtk3/stable/chap-css-overview.html
https://developer.gnome.org/gtk3/stable/chap-css-properties.html
About
Most styling functions for Gtk have been depreciated since Gtk+3.16(Example), and
arecan be now modified using Gtk CSSThis PR is thus an attempt to add styling to Gtk widgets using CSS
What this should eventually include / Updates:
Use CSS instead of Pango for changing fontsset_font,set_color, ...Current Roadblocks
A certain test to set Gtk property is failing on ci, my guess is due to different version of pygobject being installed on ci, Could try again after PR Recommend to use pip to install pygobject #374 gets mergedCan only changefont_familyandfont_sizethrough interface: See gist for detailsAccording to what I understood from chatting on Gtk+ irc,background_colorcannot be unittested, since it is no more a simpleGDK.RGBAtype but can be anything including image, video, cairo object, etc.PR Checklist: