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

Follow the system theme in egui #4860

Merged
merged 10 commits into from
Aug 6, 2024
Merged

Conversation

bash
Copy link
Contributor

@bash bash commented Jul 21, 2024

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.

  • 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

@emilk
Copy link
Owner

emilk commented Jul 31, 2024

Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Code looks very good; I'll test once the winit 0.30 changes has been merged in

crates/egui/src/memory.rs Show resolved Hide resolved
crates/egui/src/memory/theme.rs Outdated Show resolved Hide resolved
@bircni
Copy link
Contributor

bircni commented Aug 2, 2024

Looks like some doc comments are still missing:

warning: missing documentation for a method
  --> crates/egui/src/memory/theme.rs:13:5
   |
13 |     pub fn default_visuals(self) -> crate::Visuals {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: the lint level is defined here
  --> crates/egui/src/memory.rs:1:9
   |
1  | #![warn(missing_docs)] // Let's keep this file well-documented.` to memory.rs
   |         ^^^^^^^^^^^^

warning: missing documentation for an associated function
  --> crates/egui/src/memory/theme.rs:20:5
   |
20 |     pub fn from_dark_mode(dark_mode: bool) -> Self {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: `egui` (lib) generated 2 warnings

@bash
Copy link
Contributor Author

bash commented Aug 2, 2024

Looks like some doc comments are still missing:

Ah yes, I committed the review suggestions without thinking about this 😅 I'll fix it when I rebase :)

@bash
Copy link
Contributor Author

bash commented Aug 5, 2024

winit 0.30 changes are merged in 🎉

@bash bash requested a review from emilk August 5, 2024 10:00
Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Very nice, works great!

@emilk emilk added eframe Relates to epi and eframe egui labels Aug 6, 2024
@emilk emilk merged commit 2dac4a4 into emilk:master Aug 6, 2024
20 of 21 checks passed
@bash bash deleted the egui-follow-system-theme branch August 6, 2024 18:32
@emilk emilk changed the title Follow the System Theme in egui Follow the system theme in egui Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
eframe Relates to epi and eframe egui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants