antoyo / relm

Idiomatic, GTK+-based, GUI library, inspired by Elm, written in Rust
MIT License
2.41k stars 79 forks source link

refs #25: ability to set the widget style class through the view DSL #273

Closed emmanueltouzery closed 3 years ago

emmanueltouzery commented 3 years ago

I've wanted this feature in relm for a while, in my relm apps i have a bunch of get_style_context().add_class() calls in my init_view implementations.

The new style_class attribute must enable to specify multiple style classes for a widget.. currently I made it that you must repeat the attribute multiple times in the DSL. We could also change it so that the user can type #[style_class=("destructive-action", "linked")]. Let me know if you'd prefer that.

I also had to handle style classes specially in the Attributes struct, because with a Map of attributes, we couldn't list multiple times the style_class attribute. Again other approaches are possible, if you'd rather I do it another way, let me know!

antoyo commented 3 years ago

Nice. Have you got an idea for specifying the CSS file?

Maybe we could have an additional attribute like #[css_file="style.css"] that would expand to:

        use gtk::{CssProviderExt, StyleContextExt};
        let style_context = self.button.get_style_context();
        let style = include_bytes!("style.css");
        let provider = gtk::CssProvider::new();
        provider.load_from_data(style).unwrap();
        style_context.add_provider(&provider, gtk::STYLE_PROVIDER_PRIORITY_APPLICATION);

What do you think?

emmanueltouzery commented 3 years ago

hmm, for loading the style, that's not exactly the code that I use. Your code is tied to a widget, mine is tied to a screen:

        let screen = self.widgets.window.get_screen().unwrap();
        let css = gtk::CssProvider::new();
        css.load_from_data(CSS_DATA)?;
        gtk::StyleContext::add_provider_for_screen(
            &screen,
            &css,
            gtk::STYLE_PROVIDER_PRIORITY_APPLICATION,
        );

Yours is probably better. But in any case we need a widget to work with, you just use it directly and i use it for the screen (in theory i guess it could be bad for multiscreen, but it works for me..).

In general, why not. I wasn't bothering too much with this, because you need this only once in the application. In terms of implementation, so that we don't "inject" too much code through that macro, maybe relm could export a function bind_css_file(widget: &gtk::Widget, css_contents: &str) or something, and then the macro would just generate something like bind_css_file(widget, include_bytes!(css_path). But we could also generate the whole code if you prefer.

If you specify a way to do it, I could implement it.

antoyo commented 3 years ago

That's a good point: let's just merge as is and we'll think about what could be the best way to do this.

antoyo commented 3 years ago

Sounds good! Is it ready to merge?

emmanueltouzery commented 3 years ago

I am happy with it yes, from my point of view it's ready to merge, ONLY i'd be ready to add some documentation somewhere, but I'm not sure where. As you saw I updated an example, that may be enough.

Also I highlighted some choices to make in my initial message, this choice is related to the API, so you might consider whether you're happy with it as-is:

The new style_class attribute must enable to specify multiple style classes for a widget.. currently I made it that you must repeat the attribute multiple times in the DSL. We could also change it so that the user can type #[style_class=("destructive-action", "linked")]. Let me know if you'd prefer that.

antoyo commented 3 years ago

That's fine: we can always change it later if needed.

For the documentation, that would eventually go in this page. If you're willing to try, you could start adding content to it (https://github.com/antoyo/jekyll-relm-blog/blob/master/documentation/reference.adoc), perhaps by doing something similar to the tutorial page. I can help you if needed.

emmanueltouzery commented 3 years ago

ah, short-term i had in mind to expand some documentation to mention this new feature, writing or starting to write the reference from scratch is considerably more work. i do really really like relm though.. maybe at some point i try... but not short-term i think.

antoyo commented 3 years ago

If it would help you that I start the reference so you can just add the documentation for this feature, please tell me.

emmanueltouzery commented 3 years ago

I'm not sure how the reference should look like. There is the tutorial and the crates.io apidoc.

Maybe some sort of description of the #[widget] macro, view!, any supported annotations? So, name, style_class, maybe some other thing?

antoyo commented 3 years ago

Yep, that's the plan.

emmanueltouzery commented 3 years ago

i'm working on something, i'll make a PR on that other repo I think tonight.