Skip to content

A clickable button that changes the theme to dark/light #94

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

oliveira-sh
Copy link

This PR try to close issue #90.

Changes in #93

  • Add a VISUALS_LIGHT and rename VISUALS to VISUALS_DARK
  • Add a dark_mode boolean in App struct.
  • Add the GUI button with switching icon based on the current bool value

Changes in 2a1b1a9

  • saved the egui::Context; in a var inside the App struct, to be able to use later by ctx.set_visuals(visuals);
  • Add: cc: Option<Context> inside App.

Using Option<egui::Context> works but isn’t ideal.

simplescreenrecorder-2025-05-08_20.17.15.mp4

We initialize it to None in the constructor and assign it later in pub fn cc. Currently, the theme resets on app restart. If desired, we can extend this by saving dark_mode

Please let me know if you’d like adjustments or additional tests!

@Cyrix126
Copy link
Owner

Hello, thanks for working on this !

To save the theme across restart, you must add a field in the struct of Gupax in src/disk/state.rs and check it when the App is created at startup. In this case, don't forget to include it in the eframe_impl.rs file to get the button save/restore working correctly with it.

Note: egui has a feature called "persistence" that isn't used in Gupaxx yet but that could maybe replace some logic in the future.

Possibly some color will need to be tweaked for the light theme. Ex: at 7s in the video, the token field is unreadable because it is almost white on white. That could be part of this PR if you want to.

Having the cc field as an option is really not an issue.

I am hesitant between keeping the button on the bottom bar (it's good looking) and having the setting only in the Gupaxx tab since it won't be used very often by the user. We can keep it that way for now.

@oliveira-sh
Copy link
Author

oliveira-sh commented May 15, 2025

To save the theme across restart, you must add a field in the struct of Gupax in src/disk/state.rs

Done, and a removed dark_mode boolean from app, now relies on app.state.gupax.dark_mode.

Possibly some color will need to be tweaked for the light theme. Ex: at 7s in the video, the token field is unreadable because it is almost white on white. That could be part of this PR if you want to.

It seems that this text type is hard-coded as a fixed color (BONE).

image
image

I tried to remove it, but from some reason, just works in the first column, any ideas why?

image

I am hesitant between keeping the button on the bottom bar (it's good looking) and having the setting only in the Gupaxx tab since it won't be used very often by the user. We can keep it that way for now.

I first put it there, then I changed it to this bar because I thought it looked more elegant and didn't take up much space.

Please let me know if you’d like adjustments or additional tests!

@Cyrix126
Copy link
Owner

Cyrix126 commented May 15, 2025

I tried to remove it, but from some reason, just works in the first column, any ideas why?

It is changing the color of the other columns when you remove .color(BONE) from the code. It's just that the other columns here are disabled since the corresponding service is not started.

Copy link
Owner

@Cyrix126 Cyrix126 left a comment

Choose a reason for hiding this comment

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

Changes of the theme should not be saved unless the user click on the "Save" button, even if it's applied in the current session. You can look at the "hero mode" button of the XvB tab as an example. Beware there is a "Save on exit" option in Gupaxx tab.

@Cyrix126
Copy link
Owner

One more thing, the default should not be dark, but be whatever the operating system prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants