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

Allow checkbox's TextStyle modification #406

Closed
Mubelotix opened this issue May 17, 2021 · 2 comments
Closed

Allow checkbox's TextStyle modification #406

Mubelotix opened this issue May 17, 2021 · 2 comments
Labels
design Some architectual design work needed feature New feature or request

Comments

@Mubelotix
Copy link

Mubelotix commented May 17, 2021

Feature request
Make it possible to change the TextStyle of checkboxes, so that there is a basic font size support before #56.

Solution

Checkbox::new(&mut enabled, "Enable").text_style(TextStyle::Body);

Alternatives
Change the TextStyle::Button font size, but that would break other buttons.

@Mubelotix Mubelotix added the feature New feature or request label May 17, 2021
@emilk emilk added the design Some architectual design work needed label May 17, 2021
@emilk
Copy link
Owner

emilk commented May 17, 2021

There are many widgets we want to change text styles and text color on. Adding modifiers for them all may be the wrong approach.

An alternative is to instead set a new default in Style. This would also make it more convenient to change it for a lot of checkboxes:

ui.style().override_text_style = Some(TextStyle::Monospace);
ui.checkbox();
ui.checkbox();

What is the user story behind this? Why would one want to change the text style of only some checkboxes?

PS: I think you linked to the wrong issue

@Mubelotix
Copy link
Author

Mubelotix commented May 18, 2021

What is the user story behind this?

I don't like how the widgets have larger font than body text. It does not integrate well. Especially since I changed the TextStyle::Button font size to a larger value because I needed to display larger items in a custom panel. I can't wait for #56 to be closed!

An alternative is to instead set a new default in Style. This would also make it more convenient to change it for a lot of checkboxes:

Your suggested solution would solve my problem. However, I wonder how complex a PR adding this would be. There are so many places where text_style is hardcoded. I would have to find and track all of them. Anyway I am already having problem with this in #407. That's why I switched back to a draft PR because as soon as I want to make it reviewable I discover another widget that lacks this feature.

Adding modifiers for them all may be the wrong approach.

Indeed. CSS implemented both solutions and I almost never use the element-specific one. However:

  • Sometimes it's better to have it. Otherwise if we had like to change a single checkbox among others we would have to embed it into its own container. That's ugly.
  • CSS is very powerful at selecting elements among others. It is not possible with egui yet (but that would be great). So the style-related solution would not be as great as in CSS. But you probably thought about this when you added other style fields.

So why don't we add both solutions? We could even (crazy idea incoming) add non-required methods on the Widget trait. We could include setters for all similar generic properties like text_style and text_color for the sake of completeness. The Widgets would be able to implement these methods if applicable.

@emilk emilk closed this as completed in a892519 May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Some architectual design work needed feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants