psst icon indicating copy to clipboard operation
psst copied to clipboard

Published 20 hours ago • verified publisher icon jpochyla

Detect light/dark OS theme

Open KeKsBoTer opened this issue 3 years ago • 6 comments

Hey,

I implemented basic light/dark theme detection for macOS. I am new to the project and would appreciate your input.

Location in Code

At this point in time I am just reading the OS theme as the default value for the Theme Enum. This seems a bit sketchy to me but I am not sure where else to put it.

Reactive Implementation

My approach only reads the OS theme once (at the application start) and does not react to theme changes during runtime. This project could serve as a basic implementation for a reactive implementation. Since a reactive implementation is much more complicated I would strive for a basic approach at this point in time.

Error Handling

I was unable to find a suitable error handling approach for my implementation. Neither panic::catch_unwind nor reading the OS value in a separate thread prevents the program from crashing when reading the OS theme fails ("rust cannot catch foreign exceptions"). This is not optimal since an theme reading error should not result in the program crashing. Do you have any idea how to fix this problem?

KeKsBoTer avatar Aug 26 '21 17:08 KeKsBoTer

I think this should be implemented upstream in druid. This is a general feature that should be handled and exposed by the druid-shell component rather than individual apps themselves

Kethku avatar Aug 26 '21 19:08 Kethku

Thanks a lot for the PR @KeKsBoTer! I also think this would be best suited for druid-shell, do you feel like opening a PR over there? If not, I can do it. winit also have some support for light/dark theme detection, although only on Windows, IIRC.

jpochyla avatar Aug 29 '21 09:08 jpochyla

Hey,

thanks for the feedback! I agree with your suggestion of implementing it in druid-shell. My plan is to implement it there and open the PR. I just don't have time for it before next week. Should I close the pull request here for now?

KeKsBoTer avatar Aug 30 '21 13:08 KeKsBoTer

Thanks! I think we can leave this open for now.

jpochyla avatar Aug 30 '21 14:08 jpochyla

Looks like #162 is a bit further with this, through using 3rd party crates for the detection? I haven't investigated it much, yet.

jpochyla avatar Sep 01 '21 15:09 jpochyla

I'm making some changes to that 3rd party crate to also support Linux, I'm using my own forks for now until my changes are merged upstream. I also thought that druid should implement dark mode detection and not the apps itself.

We could try to implement this functionality in @jpochyla's fork of druid in case that they aren't already working on that.

Let me know if i should stop working on it until we make changes to druid.

edfloreshz avatar Sep 04 '21 17:09 edfloreshz