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

Simplifying StyleSheet implementations #755

Closed
TimUntersberger opened this issue Mar 1, 2021 · 8 comments
Closed

Simplifying StyleSheet implementations #755

TimUntersberger opened this issue Mar 1, 2021 · 8 comments
Labels
feature New feature or request good first issue Good for newcomers
Milestone

Comments

@TimUntersberger
Copy link
Contributor

Right now any StyleSheet has a list of functions without a default implementation. Implementing a StyleSheet is really annoying, because you have to override all of the functions and not just the thing you want to change.

I recently wanted to just change the border_radius of a text_input and I had to write the following code.

impl StyleSheet for Styles {
    fn active(&self) -> Style {
        Style {
            border_radius: 0,
            ..Style::default()
        }
    }

    fn focused(&self) -> Style {
        Style {
            border_color: Color::from_rgb(0.5, 0.5, 0.5),
            ..self.active()
        }
    }

    fn placeholder_color(&self) -> Color {
        Color::from_rgb(0.7, 0.7, 0.7)
    }

    fn value_color(&self) -> Color {
        Color::from_rgb(0.3, 0.3, 0.3)
    }

    fn selection_color(&self) -> Color {
        Color::from_rgb(0.8, 0.8, 1.0)
    }
}

As you can see I mostly just copied your sourcecode.

If StyleSheets would have these default styles as default implementation like this:

trait StyleSheet {
    fn active(&self) -> Style {
        Style {
            background: Background::Color(Color::WHITE),
            border_radius: 0,
            border_width: 1.0,
            border_color: Color::from_rgb(0.7, 0.7, 0.7),
        }
    }

    fn focused(&self) -> Style {
        Style {
            border_color: Color::from_rgb(0.5, 0.5, 0.5),
            ..self.active()
        }
    }

    fn placeholder_color(&self) -> Color {
        Color::from_rgb(0.7, 0.7, 0.7)
    }

    fn value_color(&self) -> Color {
        Color::from_rgb(0.3, 0.3, 0.3)
    }

    fn selection_color(&self) -> Color {
        Color::from_rgb(0.8, 0.8, 1.0)
    }
}

Implementing StyleSheets would be way more easy. Changing the border_radius would now only require the following code:

impl StyleSheet for Styles {
    fn active(&self) -> Style {
        Style {
            border_radius: 0,
            ..Style::default()
        }
    }
}

Is there a reason why StyleSheets are implemented like this right now?

@hecrj
Copy link
Member

hecrj commented Mar 2, 2021

I think it's a good idea.

Is there a reason why StyleSheets are implemented like this right now?

No particular reason. In fact, the button::StyleSheet trait already does it.

If anyone wants to give this a shot, feel free!

@hecrj hecrj added feature New feature or request good first issue Good for newcomers labels Mar 2, 2021
@hecrj hecrj added this to the 0.3.0 milestone Mar 2, 2021
@TimUntersberger
Copy link
Contributor Author

TimUntersberger commented Mar 2, 2021

I will open a PR for this later this week.

@TimUntersberger
Copy link
Contributor Author

How would you want to do default styles which can be extended?

What if any function that returns a Style receives the default style as argument?

@13r0ck
Copy link
Member

13r0ck commented Mar 15, 2021

I don't think that is how button::Stylesheet does it. I'm not against different techniques though, why do you think that that would be better?

@TimUntersberger
Copy link
Contributor Author

TimUntersberger commented Mar 15, 2021

I'm not against different techniques though, why do you think that that would be better?

If you are talking about my previous comment then because I don't know how to override the active style of a button for example without having to set each field.

Example:

impl button::StyleSheet for Style {
  fn active(&self) -> button::Style {
    // here I have to create the whole button::Style struct which requires me to know about the default values
  }
}

After thinking about this some more I discovered that button::Default exists. Maybe making the Default styles accessible to the users would we a solution?

Example:

impl button::StyleSheet for Style {
  fn active(&self) -> button::Style {
    let mut style = button::DefaultStyle.active(); // renamed to DefaultStyle for clarity
    // do something with style
    style
  }
}

@13r0ck
Copy link
Member

13r0ck commented Mar 16, 2021

Maybe this is something that could be solved with code generation?
Like from your first comment, you are basically setting what you want, then copying and pasting the rest from iced's source. Which code generation could totally do

Something like?

#[derive(Styles)]
impl StyleSheet for Styles {
    fn active(&self) -> Style {
        Style {
            border_radius: 0,
            ..Style::default()
        }
    }
}

@TimUntersberger
Copy link
Contributor Author

I created a draft PR which prototypes the idea I mentioned above using a button.

@hecrj hecrj modified the milestones: 0.3.0, 0.4.0 Mar 31, 2021
@hecrj hecrj modified the milestones: 0.4.0, 0.5.0 Apr 12, 2022
@13r0ck
Copy link
Member

13r0ck commented Sep 25, 2022

@hecrj I think thi should be closed because of #1362, as #781 was for the same reason.

@hecrj hecrj closed this as completed Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants