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

Android: Add black backgrounds toggle #11325

Merged
merged 1 commit into from Dec 10, 2022
Merged

Conversation

t895
Copy link
Contributor

@t895 t895 commented Dec 5, 2022

Makes all background colors black in dark mode when enabled through a ThemeOverlay. Applied the same way as a theme/mode.

Demo -
background-demo

@t895 t895 force-pushed the extra-dark branch 2 times, most recently from baaaef4 to 90ffeaa Compare December 5, 2022 02:09
@lioncash
Copy link
Member

lioncash commented Dec 5, 2022

Is there no way to roll this into the theme selector so that there doesn't need to be a separate option?

Like having the theme dropdown be something like:

  • System Default
  • Light
  • Dimmed (or some other appropriate adjective)
  • Dark

I think it'd be nicer from a UI standpoint if something like that was possible

@t895
Copy link
Contributor Author

t895 commented Dec 5, 2022

Is there no way to roll this into the theme selector so that there doesn't need to be a separate option?

Having it as a separate option is important because you lose the ability to have the black backgrounds when following the system theme otherwise. And we couldn't put it in the normal theme switcher because we'll lose being able to use multiple color schemes with black backgrounds.

Also it might not matter much to you but most other apps use this UX for themes

@t895 t895 force-pushed the extra-dark branch 2 times, most recently from a82f8ef to 75a39db Compare December 5, 2022 02:39
@MayImilae
Copy link
Contributor

MayImilae commented Dec 5, 2022

At first glance it feels a little weird to have checkmarks then two combopopups (I don't know what they are called but they are basically comboboxes that pop up) then another checkmark. Like, it -feels- like the combopopups should be grouped together, then have the checkmarks. But if you look at what those options do that would be terrible please don't do that. Additional, we do the mixing of combopopups and checkmarks elseswhere, and the placement of these options is good imo, so this is the best layout. LGTM.

Hmm, it may help to have a dividing line between the theme controls the rest of the interface tab items, just to help combat the "weird feeling". If you can even do that. But it's not a big deal.

Also, glad to see that the black background option actually turns the background black, RGB 0,0,0. You have to watch out for these things on Android now.

@mbc07
Copy link
Contributor

mbc07 commented Dec 6, 2022

Small nitpick, change "night theme" to "dark theme" in the checkbox description. That's how it's referenced everywhere else and it feels weird to have a "night theme" just here.

Also, glad to see that the black background option actually turns the background black, RGB 0,0,0. You have to watch out for these things on Android now.

Considering how most (if not all) OLED screens currently in use on Android devices experience bad smearing when scrolling with pure black themes, Google's Material dark theme using a dark gray instead of pure black is very likely intentional as that helps reducing the smearing...

@t895
Copy link
Contributor Author

t895 commented Dec 6, 2022

Small nitpick, change "night theme" to "dark theme" in the checkbox description. That's how it's referenced everywhere else and it feels weird to have a "night theme" just here.

Fair point

Considering how most (if not all) OLED screens currently in use on Android devices experience bad smearing when scrolling with pure black themes, Google's Material dark theme using a dark gray instead of pure black is very likely intentional as that helps reducing the smearing...

I agree, it's just a nice option to have an it's off by default.

Makes all background colors black in dark mode when enabled through a ThemeOverlay. Applied the same way as a theme/mode.
@JosJuice JosJuice merged commit 1fd8d47 into dolphin-emu:master Dec 10, 2022
11 checks passed
@t895 t895 deleted the extra-dark branch January 1, 2023 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants