Skip to content
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

RFC: Toga API for style-related operations #302

Closed
freakboy3742 opened this issue Dec 5, 2017 · 10 comments
Closed

RFC: Toga API for style-related operations #302

freakboy3742 opened this issue Dec 5, 2017 · 10 comments

Comments

@freakboy3742
Copy link
Member

This is a request for comment on a design decision we have to make about the APIs exposed by Toga for style-related operations.

The problem

What API should we expose for style-related changes to a widget? Consider the following operations:

  • Set the font face, font size and text color of this label
  • Set the background color of this widget
  • Hide/show this widget (either "don't display, but retain layout", or "remove from layout entirely")

These are all relatively common things for a widget toolkit to need to do. There will inevitably be an API on platform backends for supporting these operations.

However, what is the API that the Toga interface layer should expose? We use Colosseum to apply styles - however, the style API is deliberately abstracted so that you can pass in any style object you want. In theory, you could define your own style objects implementing a different layout approach (e.g., constraint-based layout?) I'm not sure if anyone will actually ever want to do this - but the option exists.

But should we force all style operations through the style API? Or should we also expose a more traditional functional interface for style operations? i.e., would you rather see:

  • Fonts:
    • label.style.set(font=toga.Font(face='Times New Roman', size=12*pt); or
    • label.font = toga.Font(face='Times New Roman', size=12*pt)
  • Background color:
    • box.style.set(background_color='#abcdef'); or
    • box.background_color = '#abcdef'
  • Show/Hide:
    • box.style.set(display=None, visibility=False); or
    • box.hide() and box.visible = False

If we do provide both interfaces, we're faced with some questions:

  • Which one is the "canonical" definition. If I set the font with CSS, then change the font with a widget API, what happens?

    • The widget is the canonical definition, but the CSS declaration is updated; or
    • The widget is the canonical definition, but the CSS declaration is not updated. CSS is just a way of getting an "initial" style defined; or
    • The CSS declaration is canonical. Widgets have no style state; Style objects are expected to expose an API that matches Toga, so if you call hide() on a widget, it calls hide() on the style object, which sets display=None on the style object, which pushes that down to the widget implementation.
  • In documentation and examples, which API do we favour?

I've been going back and forth in my mind about this; I'm torn between the purity and internal consistency of using CSS for all style operations, and the simple utility of having methods like hide() available. I'm interested in hearing other opinions.

@DariusMontez
Copy link
Contributor

I personally favor the CSS approach because I believe consistency and familiarity make API's easier to learn. Additionally, I think the complexity of the current API can be helped. If the CSS class implemented __setattr__ and __getattr__, It would allow an API with similar look and feel to native HTML5 style:

label.style.font = toga.Font(face='Times New Roman', size=12*pt)
box.style.background_color='#abcdef'
box.style.display = None and box.style.visibility = False

The main purpose would just be to bring back some "simple utility" as well as making those familiar with CSS feel at home :)

Of course, I would ultimately like to see stylesheets made possible, which includes having ids and class names for widgets. Maybe not so much for example apps, but for larger scale applications where many widgets will share the same theme colors etc., and most widgets will have custom padding/margin.

This leads me to a question: should Toga widgets define their own default styles? For example, toga.Button has no default margin, so when one button is placed adjacent to another, they "touch" (at least on GTK). A good portion of the example apps is used avoiding this by adding margin/padding. I would suggest that each widget gives itself default margin and padding, say 10px, so that it "just works" out of the box.

@danyeaw
Copy link
Member

danyeaw commented Dec 5, 2017

I really like that proposal @DariusMontez! One CSS style API, but with a simple and consistent syntax would really be optimal.

@goanpeca
Copy link

goanpeca commented Dec 5, 2017

I really prefer the "Python" way like. Although it is "odd" compared to commonly use toolkits, I think the whole point is to make Toga exploit Python constructs... setters and getters

So 👍 label.font = toga.Font(face='Times New Roman', size=12*pt)
And 👍 box.visible = False

and 👎 label.style.set(font=toga.Font(face='Times New Roman', size=12*pt)

BUT, following this same line of reasoning

box.background_color = '#abcdef' feels like a "hack" so it perhaps could be more like:

box.background_color = toga.Color('#abcdef')
box.background_color = toga.Color(r=150, b=200, g=190, alpha=200)

Or maybe this is just a convenience/exception
?

Now I guess also someone would like to do something like to have this available:

widget.style_from_css(css)

Now regarding

Which one is the "canonical" definition. If I set the font with CSS, then change the font with a widget API, what happens?

Having used Qt and StyleSheets with Qt AND styling using the direct api I feel I would really like to have:

  • A widget should define defaults based on platform and this should be the initial state, so things look ok by default.
  • The widget or the CSS declaration is updated based on what was called, as in this should probably be a 2 way street, updating one should update the other? maybe we should even provide a way to export a CSS from a widget? This is surely more work, but feels like the "right" thing to do. Not sure what is better to have as a canonical one though.

print(widget.css)

@freakboy3742
Copy link
Member Author

@DariusMontez That's already possible - I was just using the widget.style.set() API in my examples. Under the hood, style.set() is just a proxy for "turn all these kwargs into setattr" calls.

Default widget styles (and platform-specific default widget styles) are both entirely possible, as are style sheets. Default widget styles are a little easier, because we can define them directly on the widgets themselves; stylesheets require an implementation of the CSS selection and cascading module - which is entirely possible, just more work. But that's why I started Colosseum... :-)

However, I agree that having default paddings and margins (just as you do in a browser - H1 has a default font, margin, padding, leading, etc) would be desirable.

@freakboy3742
Copy link
Member Author

@goanpeca To be clear: Is your objection to:

  • label.style.set(font=toga.Font(face='Times New Roman', size=12*pt), or
  • label.style.font = toga.Font(face='Times New Roman', size=12*pt)?

i.e., is it to pushing all the style operations to the style object, or on the set() API specifically? As I indicated to @DariusMontez, the set() API is just a convenience for multiple assignments - the individual attribute assignment approach is also possible.

The Color approach you describe is already possible (or will be, once the Colosseum 0.2 is finalised). Colosseum 0.2 provides an rgb(), rgba() etc definitions, matching those provided by CSS. However, just as CSS does, it also allows '#abcdef' and 'lightgoldenrodyellow' as string definitions.

However, for me, this is probably the biggest reason to stick to widget.style.background_color = 'xxx' (rather than widget.background_color = 'xxx'. It means the style definitions are deliberately kept apart from the widget itself, in the same way that the data definitions are kept apart from the widget itself. This has the (theoretical) side effect that you could assign any other style object you wanted.

So, you could (theoretically) define a "CSS, but disallowing string assignment of colours" style module, and you wouldn't require any changes on Toga itself. You could even mix your CSS styles on one widget with your "no-strings-CSS" styles on a different widget in the same app.

@goanpeca
Copy link

goanpeca commented Dec 5, 2017

@freakboy3742 I guess set() feels weird on python, plus having a set type is also weird, maybe I am just being too picky.

Maybe label.style.update(font=toga.Font(face='Times New Roman', size=12*pt) since we are already more or less agreeing on having defaults anyway, so a widget on startup already defines things?

The Color approach you describe is already possible (or will be, once the Colosseum 0.2 is finalised). Colosseum 0.2 provides an rgb(), rgba() etc definitions, matching those provided by CSS. However, just as CSS does, it also allows '#abcdef' and 'lightgoldenrodyellow' as string definitions.

Yeah, something like what CSS provide, the Color example was Qt messing with my head... :-\

However, for me, this is probably the biggest reason to stick to widget.style.background_color = 'xxx' (rather than widget.background_color = 'xxx'. It means the style definitions are deliberately kept apart from the widget itself, in the same way that the data definitions are kept apart from the widget itself. This has the (theoretical) side effect that you could assign any other style object you wanted.

Yes, it makes sense now

And feels more clean to do

widget.style.load_from_css(css)
print(widget.style.css(css))

So, you could (theoretically) define a "CSS, but disallowing string assignment of colours" style module, and you wouldn't require any changes on Toga itself. You could even mix your CSS styles on one widget with your "no-strings-CSS" styles on a different widget in the same app.

Interesting and powerful but probably not something we should advice :-p, (or pursue)

Are there any use cases in Custom Style classes, I mean, if we just follow CSS, what else could a custom Styler provide, besides disallowing strings XD ?

Ahh... maybe we can have a CSS2Style and CSS3Style or something like that? and could make migrations easier as the standard changes?


All this talk, really makes me want to help on colosseum, I really want to be able to load and dump CSS files! On Qt you have stylesheets and normal Style objects and they do not update themselves but some things can only be changed via that Style object which is horrible and makes a custom app on Qt use both styles and stylesheets... and it just feels wrong... so wrong!

@freakboy3742
Copy link
Member Author

freakboy3742 commented Dec 5, 2017

@goanpeca I hadn't considered the API overlap with the set() object, but I completely agree. Maybe update() would be a better name for that API (and one that has precedent with dict objects).

Any help with Colosseum would be most gratefully accepted :-)

The only use case I can think of for a custom style class is to use a completely different layout scheme - for example, someone who wants to do constraint-based layout for widgets, or have a really simple grid-based layout. I don't think it's a big use case, but if Django has taught me anything, it's that if you don't get your architecture clear to start with, you're gonna be fighting it for life.

@phildini
Copy link
Member

phildini commented Dec 5, 2017

My $0.02: I'm in favor of the widget.style.{property} = {type} form. It feels like the right blend of CSS and Pythonic ways of doing things.

As a bonus, we can make the properties on style discoverable through dir(). Having .update() functions doesn't feel particularly pythonic unless the repr of the style object and all sub-objects is a dict.

All of the above is my opinion, but it seems to be in keeping with the group.

@iocoded
Copy link

iocoded commented May 24, 2019

I support @DariusMontez , it shold be made as simple as possible

label.style.font = toga.Font(face='Times New Roman', size=12*pt)
box.style.background_color='#abcdef'
box.style.display = None and box.style.visibility = False

@freakboy3742
Copy link
Member Author

Closing this on the basis that the "widget.style.{property} = {value}" approach has effectively been implemented in the 0.3 branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants