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

Custom light & dark themes on system theme change #4490

Closed
kernelkind opened this issue May 12, 2024 · 7 comments · Fixed by #4744
Closed

Custom light & dark themes on system theme change #4490

kernelkind opened this issue May 12, 2024 · 7 comments · Fixed by #4744
Labels
egui rerun Desired for Rerun.io style visuals and theming

Comments

@kernelkind
Copy link

Is your feature request related to a problem? Please describe.
Currently, when the system theme changes, the Visuals theme is switched to the hard-coded egui::Visuals::dark() or egui::Visuals::light(). I would like for there to be an option to use custom Visuals for dark and light themes.

This seems to be where the system theme change is detected and the egui visuals are set:

self.egui_ctx.set_visuals(theme.egui_visuals());

And this seems to be where the hard-coded Visuals are set:

pub fn egui_visuals(self) -> egui::Visuals {
match self {
Self::Dark => egui::Visuals::dark(),
Self::Light => egui::Visuals::light(),
}
}

Describe the solution you'd like
There should be some way to pass a Visuals that corresponds to the dark theme, and another Visuals which corresponds to the light theme. Then, when a theme change is needed, those custom themes would be used instead of the hard-coded egui::Visuals::dark() or egui::Visuals::light().

Describe alternatives you've considered
None

Additional context
I am working on https://github.com/damus-io/notedeck and we would like to support custom theming at some point. It's not a high priority at the moment, and I would be open to submitting a PR for this feature in the future.

@emilk
Copy link
Owner

emilk commented May 13, 2024

100% agree with this.

The global style is stored in Options::style, and I suggest we split that into three fields:

struct Options {
    pub dark_mode_style: Arc<Style>,
    pub light_mode_style: Arc<Style>,
    pub theme: Theme, // Dark or Light
    pub follow_system_theme: bool, // Change [`Self::theme`] based on `RawInput::system_theme`?}

with enum Theme { Dark, Light } (move the one in eframe into egui).

Then move IntegrationInfo::system_theme into RawInput.

PRs welcome!

@emilk emilk added egui rerun Desired for Rerun.io style visuals and theming labels May 13, 2024
@emilk emilk added this to the Next Major Release milestone May 29, 2024
@bash bash mentioned this issue Jul 8, 2024
1 task
@rustbasic
Copy link
Contributor

Please also refer to issue #4312 while working.

emilk added a commit that referenced this issue Aug 6, 2024
* Some initial progress towards #4490

This PR just moves `Theme` and the "follow system theme" settings to
egui and adds `RawInput.system_theme`.
A follow-up PR can then introduce the two separate `dark_mode_style` and
`light_mode_style` fields on `Options`.


<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md)
before opening a Pull Request!

* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to test and add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->


* [x] I have followed the instructions in the PR template


### Breaking changes

The options `follow_system_theme` and `default_theme` has been moved
from `eframe` into `egui::Options`, settable with `ctx.options_mut`

---------

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
@bash
Copy link
Contributor

bash commented Aug 14, 2024

@rustbasic Do you have an minimal example that I can run to verify that visuals_mut works as expected for you?

@rustbasic
Copy link
Contributor

@rustbasic Do you have an minimal example that I can run to verify that visuals_mut works as expected for you?

If you call dark() or light() to change the theme, both will be initialized.
I think you need to save the settings for both of them separately so that they don't get initialized.

If you do it like the example, once the settings are set, it will be initialized when you change the theme.

Since the recent update, even the settings are initialized once.

Since text_cursor is easy to check with your eyes, I used text_cursor in the example.

use eframe::egui::*;

fn main() -> eframe::Result {
    let options = eframe::NativeOptions {
        viewport: egui::ViewportBuilder::default().with_inner_size([800.0, 600.0]),
        ..Default::default()
    };
    eframe::run_native(
        "My egui App",
        options,
        Box::new(|_cc| Ok(Box::<MyApp>::default())),
    )
}

#[derive(Default)]
struct MyApp {
    text: String,
    startup_complete: bool,
}

impl eframe::App for MyApp {
    fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
        egui::CentralPanel::default().show(ctx, |ui| {

            self.startup(ui);

            ui.horizontal(|ui| {
                ui.text_edit_multiline(&mut self.text);
            });

            ui.separator();

            ctx.settings_ui(ui);
        });
    }
}

impl MyApp {
    fn startup(&mut self, ui: &mut Ui) {
        if self.startup_complete {
            return;
        }
    
        self.startup_complete = true;
        ui.visuals_mut().text_cursor.on_duration = 2.0;
        ui.visuals_mut().text_cursor.off_duration = 0.1;
    }
}

@bash
Copy link
Contributor

bash commented Aug 15, 2024

If I'm not mistaken, ui.visuals_mut() changes the visuals for the current UI and its children only. Since UIs are created per-frame you immediately lose any changes on the next frame.

If you want a change to the visuals to be permanent you have two options:

  • Change the global visuals on Context: ctx.style_mut(|style| style.visuals.foo = bar);
  • Call ui.visuals_mut().foo = bar; once per frame.

Currently, if you have eframe configured to follow the system theme (on by default) it will override the visuals when the system theme changes and you lose your changes to the visuals. This will be fixed as soon as #4744 lands.

@rustbasic
Copy link
Contributor

rustbasic commented Aug 16, 2024

Yes, I think there are some ambiguous parts because I made the example in a hurry.
If the following is added at the example, it seems to be the example I was trying to make.

let mut style = ui.style_mut().clone();
ui.ctx().set_style(style);

What I want is that I want the Dark and Light modes to not be reset to the default value when I change them.
I think #4744 will solve it because it changes the settings for both. I haven't tested #4744 yet.

Thank you.

@bash
Copy link
Contributor

bash commented Aug 16, 2024

@rustbasic Yes, this should indeed be fixed by #4744.
With this PR you can either make changes to the dark or light style using set_dark_style / set_light_style. If you want to make changes to both, then there's a handy style_mut() function that lets you update both in one go.

emilk added a commit that referenced this issue Sep 11, 2024
#4744)

* Closes <#4490>
* [x] I have followed the instructions in the PR template

---

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:
```rust
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:

<img
src="https://github.com/user-attachments/assets/0bf728c6-2720-47b0-a908-18bd250d15a6"
width="250" alt="A screenshot of egui's widget gallery demo in dark mode
using a purple accent color instead of the default blue accent">

<img
src="https://github.com/user-attachments/assets/e816b380-3e59-4f11-b841-8c20285988d6"
width="250" alt="A screenshot of egui's widget gallery demo in light
mode using a green accent color instead of the default blue accent">

---------

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
egui rerun Desired for Rerun.io style visuals and theming
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants