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

Add option to toggle pure black mode when using system theme #1605

Merged
merged 2 commits into from
Nov 27, 2024

Conversation

hjiangsu
Copy link
Member

@hjiangsu hjiangsu commented Nov 27, 2024

Pull Request Description

This PR adds the ability to toggle on pure black mode (OLED) when using the system theme. When this is enabled, any time the system theme is set to dark mode, Thunder will use the pure black variant rather than the default dark mode variant for theming.

This setting is only visible when the user chooses "System" as the theme.

Let me know if you have a better setting name/description for this setting!

Issue Being Fixed

Issue Number: #1598

Screenshots / Recordings

Simulator.Screen.Recording.-.iPhone.16.Plus.-.2024-11-26.at.19.44.30.mp4

Checklist

  • If a new package was added, did you ensure it uses an appropriate license and is actively maintained?
  • Did you use localized strings (and added appropriate descriptions) where applicable?
  • Did you add semanticLabels where applicable for accessibility?

Copy link
Member

@micahmo micahmo left a comment

Choose a reason for hiding this comment

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

LGTM

@hjiangsu hjiangsu merged commit fd11512 into develop Nov 27, 2024
1 check passed
@hjiangsu hjiangsu deleted the feature-system-black-theme branch November 27, 2024 16:20
@glyph-se
Copy link

glyph-se commented Dec 13, 2024

Let me know if you have a better setting name/description for this setting!

I would combine this new setting with the above selector/dropdown.


Either making it have 5 values:

  • Light
  • Dark
  • Pure Black
  • System with Dark
  • System with Pure Black

Or changing it to be:

  • System
  • Light
  • Dark

And then if System/Dark is selected, show the toggle, which will affect both "Dark" and "System":

Pure Black - Use pure black theme instead of dark mode.


To me, the current split seems like an afterthought, and not intuitive as a user.

@micahmo
Copy link
Member

micahmo commented Dec 14, 2024

@glyph-se, thanks for the suggestions. I think @hjiangsu and I both both appreciate any advice on UX. 😊

  • Light

  • Dark

  • Pure Black

  • System with Dark

  • System with Pure Black

Personally I would find this a bit confusing. For example, the option "System with Dark" really means "follow the system setting (whether it's light or dark), but when it's dark, just use regular dark mode, not pure black mode". In my opinion, that full meaning can't really get communicated in a single option.

  • System

  • Light

  • Dark

And then if Dark is selected, show the toggle, which will affect both "Dark" and "System":

Pure Black - Use pure black theme instead of dark mode.

I like this idea a lot more! It's similar to what @hjiangsu already implemented (with a separate toggle for pure black) but without the extraneous explicit option for just pure black. In this way, the pure black mode is always an additional option on top of regular dark mode. I could even see the toggle being included in the bottom sheet so that everything color/theme related is in one dialog. So this would be my vote!

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.

3 participants