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.

Sign up for GitHub

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

Jump to bottom

Make light & dark visuals customizable when following the system theme #4744

Merged
merged 20 commits into from
Sep 11, 2024
Merged

Make light & dark visuals customizable when following the system theme #4744

merged 20 commits into from
Sep 11, 2024

Conversation

bash
Copy link
Contributor

@bash bash commented Jun 30, 2024 •
edited
Loading

Unfortunately, this PR contains a bunch of breaking changes because Context no longer has one style, but two. I could try to add some of the methods back if that's desired.

The most subtle change is probably that style_mut mutates both the dark and the light style (which from the usage in egui itself felt like the right choice but might be surprising to users).

I decided to deviate a bit from the data structure suggested in the linked issue.
Instead of this:

pub theme: Theme, // Dark or Light
pub follow_system_theme: bool, // Change [`Self::theme`] based on `RawInput::system_theme`?

I decided to add a ThemePreference enum and track the current system theme separately.
This has a couple of benefits:

  • The user's theme choice is not magically overwritten on the next frame.
  • A widget for changing the theme preference only needs to know the ThemePreference and not two values.
  • Persisting the theme_preference is fine (as opposed to persisting the theme field which may actually be the system theme).

The small_toggle_button currently only toggles between dark and light (so you can never get back to following the system). I think it's easy to improve on this in a follow-up PR :)
I made the function pub(crate) for now because it should eventually be a method on ThemePreference, not Theme.

To showcase the new capabilities I added a new example that uses different "accent" colors in dark and light mode:

A screenshot of egui's widget gallery demo in dark mode using a purple accent color instead of the default blue accent A screenshot of egui's widget gallery demo in light mode using a green accent color instead of the default blue accent

@bash bash mentioned this pull request Jul 8, 2024
Closed
1 task
@bash bash force-pushed the custom-dark-light branch 5 times, most recently from f8efb7b to 6699144 Compare August 14, 2024 13:02
@bash bash marked this pull request as ready for review August 14, 2024 13:08
@bash bash mentioned this pull request Aug 15, 2024
Closed
@rustbasic rustbasic mentioned this pull request Aug 25, 2024
Closed
emilk
emilk reviewed Aug 26, 2024
View reviewed changes
crates/egui/src/widgets/mod.rs Show resolved Hide resolved
crates/egui/src/ui.rs Outdated Show resolved Hide resolved
crates/egui/src/memory.rs Outdated Show resolved Hide resolved
crates/egui/src/context.rs Outdated Show resolved Hide resolved
@emilk emilk added feature New feature or request egui style visuals and theming labels Sep 4, 2024
emilk
emilk reviewed Sep 4, 2024
View reviewed changes
crates/egui/src/containers/panel.rs Outdated Show resolved Hide resolved
crates/egui/src/context.rs Outdated Show resolved Hide resolved
crates/egui/src/context.rs Outdated Show resolved Hide resolved
crates/egui/src/context.rs Outdated
}

/// The [`Style`] used by all subsequent windows, panels etc.
pub fn style(&self, theme: Theme) -> Arc<Style> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted elsewhere, the active_style produces a big diff (=a lot of breaking changes). I suggest we instead call this style_of. I suspect this will be less used anyway, so keeping the often used function name shorter makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have additionally added back set_style (which sets the currently active style). Should I also add back set_visuals now that we have the _of suffix?

@bash bash requested a review from emilk September 6, 2024 11:54
@bash
Copy link
Contributor Author

bash commented Sep 6, 2024

All ready for re-review :)

emilk
emilk approved these changes Sep 10, 2024
View reviewed changes
crates/egui/src/memory/mod.rs Show resolved Hide resolved
crates/egui/src/memory/mod.rs Outdated Show resolved Hide resolved
crates/egui/src/memory/mod.rs Outdated Show resolved Hide resolved
@emilk emilk added this to the Next Major Release milestone Sep 10, 2024
@emilk emilk merged commit b5627c7 into emilk:master Sep 11, 2024
20 checks passed
@bash bash deleted the custom-dark-light branch September 11, 2024 18:15
This was referenced Sep 12, 2024
Merged
Closed
@CrazyboyQCD CrazyboyQCD mentioned this pull request Sep 26, 2024
Open
@emilk emilk changed the title Make Light & Dark Visuals Customizable When Following The System Theme * Make light & dark visuals customizable when following the system theme Sep 26, 2024
@emilk emilk changed the title * Make light & dark visuals customizable when following the system theme Make light & dark visuals customizable when following the system theme Sep 26, 2024
emilk pushed a commit that referenced this pull request Sep 26, 2024
@rustbasic
Add back Context::set_visuals() (#5100)
dae1979 
My opinion is, it would be better to keep `Context::set_visuals()` so
that you don't get confused.

* Related #4744
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilk emilk emilk approved these changes

Assignees
No one assigned
Labels
egui feature New feature or request style visuals and theming
Projects
None yet
Milestone
Next Major Release
Development

Successfully merging this pull request may close these issues.

Custom light & dark themes on system theme change
2 participants
@bash @emilk