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

Theming Reloaded #2312

Merged
merged 48 commits into from
Mar 8, 2024
Merged

Theming Reloaded #2312

merged 48 commits into from
Mar 8, 2024

Conversation

hecrj
Copy link
Member

@hecrj hecrj commented Mar 7, 2024

image

The current theming approach is quite painful and confusing. It was presented a couple of years ago in this RFC and—while at the time it was a clear improvement to the box-all-the-styles approach we had—it introduced a whole new set of moving parts. Way too many moving parts.

A simple use case

Let's say we are a new user and we want to change the style of a button. Quite a simple use case, certainly. We search for "button style" in the API reference (yes, the API is actually somewhat documented!) and we find the Button::style method. Great! But wait, what is... this?!

pub fn style(
    self,
    style: impl Into<<Theme as StyleSheet>::Style>
) -> Button<'a, Message, Theme, Renderer>

How do we even use this? What is style supposed to be exactly? We have a generic Theme which implements a StyleSheet trait which has an associated Style type. Three moving parts! So we need to figure out first the specific Theme type we are dealing with, find its StyleSheet implementation, look at the Style associated type and then learn how to use that. But... I just want to change the color of a button! How hard can that be?!

The current system is terribly complicated; and I have wanted to revisit and redesign it for a while. It seems that time has finally come.

Functions all the way down

What would the ideal style method look like? It should let the user easily customize the appearance of a widget based on its current status without much friction.

The only reason to have StyleSheet traits is to allow users to define different appearances for each particular widget status (like active, hovered, disabled, etc.). However, if we encode the widget status as data we can then leverage a single entrypoint for the whole appearance of a widget! A simple function!

pub fn style(
    self,
    style: fn(&Theme, Status) -> Appearance,
) -> Button<'a, Message, Theme, Renderer>

Thus, widget style is defined using a function pointer that takes the current Theme and some widget Status (if the widget is stateful) and produces the final Appearance of the widget.

There is only one moving part here: the generic Theme type. However, if you are not using a custom theme, this will always be the built-in Theme. And if you are using a custom theme, then you are probably already familiar with the Theme generic type.

We can easily change our button color now:

button("I am red!")
    .style(|_theme, _status| button::Appearance::default().with_background(color!(0xff0000)))

We can even make a style helper for our red buttons:

fn red_button(_theme: &Theme, _status: &button::Status) -> button::Appearance {
    button::Appearance::default().with_background(color!(0xff0000))
}

And using it is straightforward:

button("I am red!").style(red_button)

Each widget module exposes some built-in style helpers that are readily available. For instance, button exposes primary, secondary, success, danger, and text. Using them is analogous to the previous example:

button("I am the danger!").style(button::danger)

Since styles are just functions, this means you can easily compose and extend existing styles:

fn rounded_primary(theme: &Theme, status: Status) -> button::Appearance {
    button::Appearance {
        border: Border::with_radius(10),
        ..button::primary(theme, status)
    }
}

I think this design is glaringly simple; and it was the obvious approach all along. I guess it's not so easy to escape my past trauma as a web developer...

Capturing state

But wait! If styles are just function pointers and not closures... Does that mean that we cannot style widgets based on application state? Let's say we have a Color in our state. Can we use it to style a widget? Yes, thanks to the new themer widget!

The themer widget introduced in #2209 can be used to change the Theme type for a subtree of the widget tree. This means we can set our Color as the Theme generic type and use that in our style function. For instance:

themer(
    color,
    button("I am dynamic!").style(|color, _status| button::Appearance::default().with_background(color)),
)

In fact, since Color can be directly used as the default style of a button, we can just write:

themer(color, button("I am dynamic!"))

By making this kind of dynamism opt-in, we avoid either boxing closures or introducing yet another generic type to all the widgets.

Custom themes

Finally, custom themes do not need to worry about a myriad of StyleSheet traits and Style enums anymore. Instead, they only need to implement the DefaultStyle trait of each widget. For instance, let's say we have a custom theme named MyCustomTheme and we want all of its buttons to be red by default (oh god, why):

impl button::DefaultStyle for MyCustomTheme {
    fn default_style() -> fn(&Self, button::Status) -> button::Appearance {
        red_button
    }
}

And that's all! I think this considerably simplifies the most painful part of iced 🥳

@hecrj hecrj added feature New feature or request widget styling labels Mar 7, 2024
@hecrj hecrj force-pushed the theming-reloaded branch 2 times, most recently from 30b7ab9 to 7161cb4 Compare March 8, 2024 00:43
@hecrj hecrj enabled auto-merge March 8, 2024 12:55
@hecrj hecrj merged commit edf7d7c into master Mar 8, 2024
24 checks passed
@hecrj hecrj deleted the theming-reloaded branch March 8, 2024 13:00
@alex-ds13
Copy link
Contributor

alex-ds13 commented Mar 8, 2024

This entire PR is looking great to me! I've already tried to upgrade some of my apps using this and it feels really good.

I also really like the DefaultStyle implementation which should simplify custom themes quite a lot. And I also like the way you made it work with the Themer widget like this:

themer(
    color,
    button("I am dynamic!").style(|color, _status| button::Appearance::default().with_background(color)),
)

There is only one problem I can see with it right now. This works because you've implemented the button's DefaultStyle trait to Color. You've also done so for the container, as did you for the gradient, so that you can do this on the gradient example:

let gradient_box = themer(
    gradient,
    container(horizontal_space())
        .width(Length::Fill)
        .height(Length::Fill),
);

The problem is that the themer propagates the NewTheme to all its contents (which is intended and a good thing, since it allows having some parts with different themes), so if we've tried to add some other content to that container like this:

        let gradient_content = column![
            text("Input:"),
            text_input(
                "placeholder",
                if self.transparent { "transparent" } else { "not transparent" }
            ),
            horizontal_space(),
        ];

        let gradient_box = themer(
            gradient,
            container(gradient_content)
                .width(Length::Fill)
                .height(Length::Fill),
        );

It would fail because TextInput doesn't implement it's DefaultStyle for Gradient or Gradient::Linear. And the user can't implement it either because of the orphan rule. This means that every implementation done like this for the container would have to be done on every widget that has an Appearance.

It also means that if the user wants to change something else besides the ones you've implemented in a dynamic way, it is not possible. For example, let's say I have an application where I want the border width of some container to change according to some value on my App, how would I do that? We can't do this:

container(gradient_content)
.style(|t, s| {
    let width = if self.transparent {
        2.0
    } else {
        1.0
    };
    container::Appearance {
        border: iced::Border {
            color: Color::BLACK,
            width,
            radius: 0.0.into(),
        },
        ..container::bordered_box(t, s)
    }
});

Because of [E0308] closures can only be coerced to fn types if they do not capture any variables. So the Themer would be the obvious choice, but since you haven't implemented container::DefaultStyle to Border I can't do this:

let border = iced::Border {
    color: Color::BLACK,
    width: if self.transparent { 2.0 } else { 1.0 },
    radius: 0.0.into(),
};
let c = themer(
    border,
    container(gradient_content)
);

I'm not sure what the solution should be other than implementing every widget's DefaultStyle for every Appearance property that they can have like Border, Background, Color, Shadow, Scrollbar... And implement all of the properties for all the container like widgets that can have content and an appearance like Container, Button, Menu, PickList, ComboBox...

@hecrj
Copy link
Member Author

hecrj commented Mar 8, 2024

The problem is that the themer propagates the NewTheme to all its contents, so if we've tried to add some other content to that container like this:

You can nest themers.

It also means that if the user wants to change something else besides the ones you've implemented in a dynamic way, it is not possible.

You can use anything as the new theme:

themer((theme, self.transparent), ...)

We just need to add a with_style constructor for most widgets that skips the DefaultStyle bound. Some of them already have this, like Menu and Scrollable.

@hecrj
Copy link
Member Author

hecrj commented Mar 8, 2024

Until then, Themer::new accepts a closure that allows derivation from the current theme:

Themer::new(
    move |theme| {
        let width = if self.transparent {
            2.0
        } else {
            1.0
        };
        container::Appearance {
            border: Border {
                color: Color::BLACK,
                width,
                radius: 0.0.into(),
            },
            ..container::bordered_box(theme, container::Status::Idle)
        }
    },
    my_container,
)

@alex-ds13
Copy link
Contributor

alex-ds13 commented Mar 8, 2024

You can use anything as the new theme:

themer((theme, self.transparent), ...)

This doesn't work with the error that the container doesn't implement DefaultStyle for (_, _).

So I've tried using the nested Themer to have the inner content use the app's theme again but the compiler still complains and I can't seem to figure out why. Here is what I'm doing:

The full code: (the error is on a comment on the line it happens)
use iced::application;
use iced::widget::{
    checkbox, column, container, horizontal_space, row, slider, text, text_input, themer, Themer
};
use iced::{gradient, window};
use iced::{
    Alignment, Color, Element, Length, Radians, Sandbox, Settings, Theme,
};

pub fn main() -> iced::Result {
    tracing_subscriber::fmt::init();

    Gradient::run(Settings {
        window: window::Settings {
            transparent: true,
            ..Default::default()
        },
        ..Default::default()
    })
}

#[derive(Debug, Clone, Copy)]
struct Gradient {
    start: Color,
    end: Color,
    angle: Radians,
    transparent: bool,
}

#[derive(Debug, Clone, Copy)]
enum Message {
    StartChanged(Color),
    EndChanged(Color),
    AngleChanged(Radians),
    TransparentToggled(bool),
}

impl Sandbox for Gradient {
    type Message = Message;

    fn new() -> Self {
        Self {
            start: Color::WHITE,
            end: Color::new(0.0, 0.0, 1.0, 1.0),
            angle: Radians(0.0),
            transparent: false,
        }
    }

    fn title(&self) -> String {
        String::from("Gradient")
    }

    fn update(&mut self, message: Message) {
        match message {
            Message::StartChanged(color) => self.start = color,
            Message::EndChanged(color) => self.end = color,
            Message::AngleChanged(angle) => self.angle = angle,
            Message::TransparentToggled(transparent) => {
                self.transparent = transparent;
            }
        }
    }

    fn view(&self) -> Element<Message> {
        let Self {
            start,
            end,
            angle,
            transparent,
        } = *self;

        let gradient = gradient::Linear::new(angle)
            .add_stop(0.0, start)
            .add_stop(1.0, end);

        let inner_content = themer(
            iced::Sandbox::theme(self),
            column![
                text("Input:"),
                text_input(
                    "placeholder",
                    if self.transparent { "transparent" } else { "not transparent" }
                ),
                horizontal_space(),
            ]
        );

        let content_themer = Themer::new(
            move |theme| {
                let width = if self.transparent {
                    2.0
                } else {
                    1.0
                };
                container::Appearance {
                    border: iced::Border {
                        color: Color::BLACK,
                        width,
                        radius: 0.0.into(),
                    },
                    ..container::bordered_box(theme, container::Status::Idle)
                }
            },
            container(inner_content)
            .width(500)
            .height(200)
        );

        let gradient_box = themer(
            gradient,
            // HERE IS WHERE IT COMPLAINS:
            // Diagnostics:
            // 1. the trait bound `iced::advanced::iced_graphics::iced_core::Element<'_, _, iced::gradient::Linear, _>: std::convert::From<iced::widget::Container<'_, Message>>` is not satisfied
            //    the following other types implement trait `std::convert::From<T>`:
            //      <iced::advanced::iced_graphics::iced_core::Element<'a, Message, Theme, Renderer> as std::convert::From<iced::widget::Column<'a, Message, Theme, Renderer>>>
            //      <iced::advanced::iced_graphics::iced_core::Element<'a, Message, Theme, Renderer> as std::convert::From<iced::widget::MouseArea<'a, Message, Theme, Renderer>>>
            //      <iced::advanced::iced_graphics::iced_core::Element<'a, Message, Theme, Renderer> as std::convert::From<iced::widget::Row<'a, Message, Theme, Renderer>>>
            //      <iced::advanced::iced_graphics::iced_core::Element<'a, Message, Theme, Renderer> as std::convert::From<iced::widget::Themer<'a, Message, Theme, NewTheme, F, Renderer>>>
            //      <iced::advanced::iced_graphics::iced_core::Element<'a, Message, Theme, Renderer> as std::convert::From<iced::widget::Button<'a, Message, Theme, Renderer>>>
            //      <iced::advanced::iced_graphics::iced_core::Element<'a, Message, Theme, Renderer> as std::convert::From<iced::widget::Checkbox<'a, Message, Theme, Renderer>>>
            //      <iced::advanced::iced_graphics::iced_core::Element<'a, Message, Theme, Renderer> as std::convert::From<iced::widget::ComboBox<'a, T, Message, Theme, Renderer>>>
            //      <iced::advanced::iced_graphics::iced_core::Element<'a, Message, Theme, Renderer> as std::convert::From<iced::widget::Container<'a, Message, Theme, Renderer>>>
            //    and 25 others
            //    required for `iced::widget::Container<'_, Message>` to implement `std::convert::Into<iced::advanced::iced_graphics::iced_core::Element<'_, _, iced::gradient::Linear, _>>` [E0277]
            container(content_themer)
                .width(Length::Fill)
                .height(Length::Fill),
        );

        let angle_picker = row![
            text("Angle").width(64),
            slider(Radians::RANGE, self.angle, Message::AngleChanged)
                .step(0.01)
        ]
        .spacing(8)
        .padding(8)
        .align_items(Alignment::Center);

        let transparency_toggle = iced::widget::Container::new(
            checkbox("Transparent window", transparent)
                .on_toggle(Message::TransparentToggled),
        )
        .padding(8);

        column![
            color_picker("Start", self.start).map(Message::StartChanged),
            color_picker("End", self.end).map(Message::EndChanged),
            angle_picker,
            transparency_toggle,
            gradient_box,
        ]
        .into()
    }

    fn style(&self, theme: &Theme) -> application::Appearance {
        if self.transparent {
            application::Appearance {
                background_color: Color::TRANSPARENT,
                text_color: theme.palette().text,
            }
        } else {
            application::default(theme)
        }
    }
}

fn color_picker(label: &str, color: Color) -> Element<'_, Color> {
    row![
        text(label).width(64),
        slider(0.0..=1.0, color.r, move |r| { Color { r, ..color } })
            .step(0.01),
        slider(0.0..=1.0, color.g, move |g| { Color { g, ..color } })
            .step(0.01),
        slider(0.0..=1.0, color.b, move |b| { Color { b, ..color } })
            .step(0.01),
        slider(0.0..=1.0, color.a, move |a| { Color { a, ..color } })
            .step(0.01),
    ]
    .spacing(8)
    .padding(8)
    .align_items(Alignment::Center)
    .into()
}

I'm probably doing something wrong. But still haven't find out what...

From what I understand there is no problem if the content of the container to which we are giving the gradient with themer has no content that requires an appearance, like Text, Space... But if the content needs to have an appearance from a Theme it complains, because we will have elements of type Element<'a, Message, Theme, Renderer> along with elements of type Element<'a, Message, gradient::Linear, Renderer> which doesn't work.

How would you approach something like this?

@alex-ds13
Copy link
Contributor

Oh I see it now. The problem is here:

        let content_themer = Themer::new(
            move |theme| {
                let width = if self.transparent {
                    2.0
                } else {
                    1.0
                };
                container::Appearance {
                    border: iced::Border {
                        color: Color::BLACK,
                        width,
                        radius: 0.0.into(),
                    },
                    ..container::bordered_box(theme, container::Status::Idle)
                }
            },
            container(inner_content)
            .width(500)
            .height(200)
        );

When using the above it is expecting the incoming theme (OldTheme) to be an iced::Theme but is isn't it is a gradient so we need to change it here already not only on the inner_content like I was doing. The inner_content still needs it because here we are changing the NewTheme to a container::Appearance. Like this:

        let inner_content = themer(
            iced::Sandbox::theme(self),
            column![
                text("Input:"),
                text_input(
                    "placeholder",
                    if self.transparent { "transparent" } else { "not transparent" }
                ),
                horizontal_space(),
            ]
        );

        let content_themer = Themer::new(
            move |_theme| {
                let width = if self.transparent {
                    2.0
                } else {
                    1.0
                };
                container::Appearance {
                    border: iced::Border {
                        color: Color::BLACK,
                        width,
                        radius: 0.0.into(),
                    },
                    ..container::bordered_box(&iced::Sandbox::theme(self), container::Status::Idle)
                }
            },
            container(inner_content)
            .width(500)
            .height(200)
        );

This way it works! Thank you for the explication!

@hecrj
Copy link
Member Author

hecrj commented Mar 8, 2024

@alex-ds13 If it's just a simple toggle, why not something like this?

container(column![...])
    .style(if self.transparent { thick_bordered_box } else { container::bordered_box })

Where

fn thick_bordered_box(theme: &Theme, status: &container::Status) -> container::Appearance {
    container::Appearance {
        border: iced::Border {
            color: Color::BLACK,
            width: 2.0,
            radius: 0.0.into(),
        },
        ..container::bordered_box(theme, status)
    }
}

@alex-ds13
Copy link
Contributor

This was just an experiment to see if I could still change some appearance aspects according to the App's state. But yes that would make sense for this case.

Even the case I'm using right now could be done like that. I'm using a modal to show the errors of the app and I want that modal to always have the same theme and look, regardless of the app's theme. I'm using a Themer for that. The errors inside it then have different appearances according to their severity. So I need the content inside the themer to be able to change their style according to the app's state. Which can be done like that by matching on the error's severity and have a different style function for each one.

@hecrj
Copy link
Member Author

hecrj commented Mar 8, 2024

@alex-ds13 Remember that you can always implement your own sub-theme for just the modal and implement DefaultStyle for any of the widgets it uses.

// modal.rs
pub enum Theme {
    Information,
    Warning,
    Error(error::Severity),
    // ...
}

impl container::DefaultStyle for Theme {
    // ...
}

impl text_input::DefaultStyle for Theme {
    // ...
}

pub enum Message {
    // ...
}

pub fn view<'a>() -> Element<'a, Message, Theme> {
    // ...
}

Then:

themer(modal::Theme::Error(self.error.severity()), modal::view())

@hecrj
Copy link
Member Author

hecrj commented Mar 12, 2024

@alex-ds13 #2326 should let you use closures now!

@hecrj hecrj mentioned this pull request Mar 24, 2024
@derezzedex derezzedex mentioned this pull request Mar 25, 2024
@hecrj hecrj changed the title Theming reloaded Theming Reloaded Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants