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

Automatically follow user's OS-level dark/light preference, take 1 #4009

Closed
gnprice opened this issue Mar 27, 2020 · 12 comments
Closed

Automatically follow user's OS-level dark/light preference, take 1 #4009

gnprice opened this issue Mar 27, 2020 · 12 comments
Assignees

Comments

@gnprice
Copy link
Member

gnprice commented Mar 27, 2020

(Edit: See #5533 for a fresh attempt at this.)

Over the last year or two it's become common for apps to have both a dark and a light color scheme, and let the user choose between them. Zulip has this.

Since 2019 this has become a thing at the OS level. And in recent months, I've seen more and more apps noticing my OS-level setting and following along. We should do the same, at least by default -- when the user goes into system-wide settings and says they want the device's screen to stay dark instead of bright, we shouldn't wait for an individual invitation.

This becomes especially significant now that it's an OS-level feature, in both Android and iOS, to have the light/dark mode switch back and forth every morning and evening. The only way that can work as a good experience is when apps all follow along automatically.

RN v0.62 adds an API for this. So I think it makes sense to hold off on trying to implement this behavior until we've done that upgrade, aka #3782. Done!

@gnprice gnprice added the blocked on other work To come back to after another related PR, or some other task. label Mar 27, 2020
@chrisbobbe
Copy link
Contributor

chrisbobbe commented Apr 24, 2020

As part of this, let's deprecate the 'default' name, like so:

- export type ThemeName = 'default' | 'night'
+ export type ThemeName = 'light' | 'night'

I said, over at #4046 (comment):

Hmm. Taking a look at Apple's HIG document on Dark Mode, though, it makes it sound like "default" semantics shouldn't apply to either theme at the application level, but rather at the device level:

In Settings, people can choose Dark Mode as their default interface style and schedule automatic changes between the appearance modes. Because people make these choices at a systemwide level, they generally expect all apps to respect their preferences.

Perhaps we might actually deprecate the "default" name, and use "night" or "light" everywhere (with our ThemeName type being 'light' | 'night' instead of 'default' | 'night' as it is currently), at or on the path to #4009.

and Greg agreed.

edit: Also on Android, as Greg points out here (Android doc).

@gnprice
Copy link
Member Author

gnprice commented Apr 24, 2020

It'd also be good to switch to calling it "dark mode" instead of "night mode" or "night theme" -- as "dark" is what both Google and Apple call it, and also is more logical because there's no inherent reason to only use it at night.

That, or at least the UI side of it, should happen in coordination with the webapp, though, so the same terminology is used across platforms.

[edit: this has been filed as its own issue, #5169]

chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Apr 25, 2020
We'll want to deprecate the 'default' name; as I explain on zulip#4009
[1], any 'default' semantics should only apply the device level, at
least on iOS, not within the app.

[1]: zulip#4009
gnprice pushed a commit to chrisbobbe/zulip-mobile that referenced this issue May 1, 2020
We'll want to deprecate the 'default' name; as I explain on zulip#4009
[1], any 'default' semantics should only apply the device level, at
least on iOS, not within the app.

[1]: zulip#4009
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Jul 20, 2020
We'll want to deprecate the 'default' name; as I explain on zulip#4009
[1], any 'default' semantics should only apply the device level, at
least on iOS, not within the app.

[1]: zulip#4009
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Jul 20, 2020
We'll want to deprecate the 'default' name; as I explain on zulip#4009
[1], any 'default' semantics should only apply the device level, at
least on iOS, not within the app.

[1]: zulip#4009
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Sep 4, 2020
Part of the RN v0.61 -> v0.62 changes to the template app [1],
corresponding to facebook/react-native@83a16b16c. This is a bugfix
that we're free to do at any time if we don't need to listen to
changes in the "UI mode". We don't currently need to, and we won't
consider it until zulip#4009, for which the API isn't available until the
upcoming RN v0.62 upgrade.

See more detail from Greg on GitHub [2].

[1] https://react-native-community.github.io/upgrade-helper/?from=0.61.5&to=0.62.2
[2] zulip#4244 (comment)
@gnprice
Copy link
Member Author

gnprice commented Sep 5, 2020

It'd also be good to switch to calling it "dark mode" instead of "night mode" or "night theme" -- as "dark" is what both Google and Apple call it, and also is more logical because there's no inherent reason to only use it at night.

(To be more precise on the Google side: it's "dark theme" in the UI and user-facing docs, and they follow that in the Material docs and guide-style developer docs. It's "night mode" in the API, reflecting API choices made for API 8 aka Android 2.2 Froyo, in 2010.)

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Sep 9, 2020
Part of the RN v0.61 -> v0.62 changes to the template app [1],
corresponding to facebook/react-native@83a16b16c. This is a bugfix
that we're free to do at any time if we don't need to listen to
changes in the "UI mode". We don't currently need to, and we won't
consider it until zulip#4009, for which the API isn't available until the
upcoming RN v0.62 upgrade.

See more detail from Greg on GitHub [2].

[1] https://react-native-community.github.io/upgrade-helper/?from=0.61.5&to=0.62.2
[2] zulip#4244 (comment)
gnprice pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Sep 11, 2020
Part of the RN v0.61 -> v0.62 changes to the template app [1],
corresponding to facebook/react-native@83a16b16c. This is a bugfix
that we're free to do at any time as long as we don't have any code
that depends on the "UI mode" but neglects to listen for changes to
it. We don't currently need use the UI mode at all, and we won't
consider it until zulip#4009, for which the API isn't available until the
upcoming RN v0.62 upgrade.

See more detail from Greg on GitHub [2].

[1] https://react-native-community.github.io/upgrade-helper/?from=0.61.5&to=0.62.2
[2] zulip#4244 (comment)
@chrisbobbe chrisbobbe removed the blocked on other work To come back to after another related PR, or some other task. label Feb 24, 2021
@Gautam-Arora24
Copy link
Contributor

Gautam-Arora24 commented Mar 8, 2021

I am working on this issue @chrisbobbe and @gnprice 😃
Kindly assign it to me.

@chrisbobbe
Copy link
Contributor

When in "follow the OS" mode, we'll want to make sure to listen for changes in the OS-level setting and apply them automatically, right away.

For that, I could imagine an approach that uses Appearance.addChangeListener(), with that listener logic probably living in src/boot/AppEventHandlers.js.

Or, one that uses the useColorScheme hook.

I think I slightly prefer using the useColorScheme hook, as I've been keeping an eye on the growth of AppEventHandlers and wondering if there's a better way we could handle the things it handles.

state.settings.theme is currently what the UI logic reads to decide whether to show light or dark components (see src/boot/ThemeProvider.js). It has two possible values, currently: one for light mode, one for dark mode.

Gautam-Arora24 added a commit to Gautam-Arora24/zulip-mobile that referenced this issue Mar 21, 2021
Make use of another component called "ThemeScreen" instead of a
switch for changing the theme. The new "ThemeScreen" component
now has three options called "System Default", "Dark", "Light".

The "System Default" option makes use of "useColorScheme" hook
and provides appropriate theme to ThemeProvider. Accordingly
the theme updates automatically when the system settings are
changed.

Discussion of this in CZO [1].

[1] https://chat.zulip.org/#narrow/stream/48-mobile/topic/Automatic.20dark.2Flight.20preference

Fixes: zulip#4009
Gautam-Arora24 added a commit to Gautam-Arora24/zulip-mobile that referenced this issue Mar 22, 2021
Make use of another component called "ThemeScreen" instead of a
switch for changing the theme. The new "ThemeScreen" component
now has three options called "System Default", "Dark", "Light".

The "System Default" option makes use of "useColorScheme" hook
and provides appropriate theme to ThemeProvider. Accordingly
the theme updates automatically when the system settings are
changed.

Discussion of this in CZO [1].

[1] https://chat.zulip.org/#narrow/stream/48-mobile/topic/Automatic.20dark.2Flight.20preference

Fixes: zulip#4009
Gautam-Arora24 added a commit to Gautam-Arora24/zulip-mobile that referenced this issue Mar 24, 2021
Replace OptionRow with OptionButton so as to navigate to
ThemeScreen. Make use of "navigateToTheme" action.

Discussion of this in CZO [1].

[1] https://chat.zulip.org/#narrow/stream/48-mobile/topic/Automatic.20dark.2Flight.20preference

Fixes part of zulip#4009
Gautam-Arora24 added a commit to Gautam-Arora24/zulip-mobile that referenced this issue Mar 24, 2021
Along with "night" and "default", made a third option called
"automatic". This option will check the system settings and
automatically provide the correct theme to ThemeProvider's
value prop.

Discussion of this in CZO [1].

[1] https://chat.zulip.org/#narrow/stream/48-mobile/topic/Automatic.20dark.2Flight.20preference

Fixes part of zulip#4009
Gautam-Arora24 added a commit to Gautam-Arora24/zulip-mobile that referenced this issue Mar 24, 2021
Along with "night" and "default", made a third option called
"automatic". This option will check the system settings and
automatically provide the correct theme to ThemeProvider's
value prop.

Discussion of this in CZO [1].

[1] https://chat.zulip.org/#narrow/stream/48-mobile/topic/Automatic.20dark.2Flight.20preference

Fixes part of zulip#4009
Gautam-Arora24 added a commit to Gautam-Arora24/zulip-mobile that referenced this issue Apr 29, 2021
This commit introduces a new component called 'ThemeScreen' which
will be used to provide different options (currently 'Dark' and
'Light') to change the theme in the app.

Fixes part of zulip#4009
Gautam-Arora24 added a commit to Gautam-Arora24/zulip-mobile that referenced this issue Apr 29, 2021
…onent.

Create a new action called 'navigateToThemeScreen' which will help
the 'Theme' OptionButton to navigate to 'ThemeScreen' component
on clicking. Also wired it up with the 'AppNavigator'.

Since, the logic for changing of theme is handled by the 'ThemeScreen' so
there is no use of 'handleThemeChange' function in the 'SettingsScreen'.
Similarly there is no use of Dispatch and the theme prop so all these
are removed in this commit.

Fixes part of zulip#4009
Gautam-Arora24 added a commit to Gautam-Arora24/zulip-mobile that referenced this issue Apr 29, 2021
Along with "night" and "default", made a third option called
"automatic". This option will check the system settings and
automatically provide the correct theme to ThemeProvider's
value prop.

Discussion of this in CZO [1].

[1] https://chat.zulip.org/#narrow/stream/48-mobile/topic/Automatic.20dark.2Flight.20preference

Fixes part of zulip#4009
Gautam-Arora24 added a commit to Gautam-Arora24/zulip-mobile that referenced this issue Apr 29, 2021
This commit introduces a new component called 'ThemeScreen' which
will be used to provide different options (currently 'Dark' and
'Light') to change the theme in the app.

Fixes part of zulip#4009
Gautam-Arora24 added a commit to Gautam-Arora24/zulip-mobile that referenced this issue Apr 29, 2021
…onent.

Create a new action called 'navigateToThemeScreen' which will help
the 'Theme' OptionButton to navigate to 'ThemeScreen' component
on clicking. Also wired it up with the 'AppNavigator'.

Since, the logic for changing of theme is handled by the 'ThemeScreen' so
there is no use of 'handleThemeChange' function in the 'SettingsScreen'.
Similarly there is no use of Dispatch and the theme prop so all these
are removed in this commit.

Fixes part of zulip#4009
Gautam-Arora24 added a commit to Gautam-Arora24/zulip-mobile that referenced this issue Apr 29, 2021
Along with "night" and "default", made a third option called
"automatic". This option will check the system settings and
automatically provide the correct theme to ThemeProvider's
value prop.

Discussion of this in CZO [1].

[1] https://chat.zulip.org/#narrow/stream/48-mobile/topic/Automatic.20dark.2Flight.20preference

Fixes part of zulip#4009
Gautam-Arora24 added a commit to Gautam-Arora24/zulip-mobile that referenced this issue May 1, 2021
Create a new action called 'navigateToThemeScreen' which will help
the 'Theme' OptionButton to navigate to 'ThemeScreen' component
on clicking. Also wired it up with the 'AppNavigator'.

Since, the logic for changing of theme is handled by the 'ThemeScreen' so
there is no use of 'handleThemeChange' function in the 'SettingsScreen'.
Similarly there is no use of Dispatch and the theme prop so all these
are removed in this commit.

Fixes part of zulip#4009
Gautam-Arora24 added a commit to Gautam-Arora24/zulip-mobile that referenced this issue May 1, 2021
Along with "night" and "default", made a third option called
"automatic". This option will check the system settings and
automatically provide the correct theme to ThemeProvider's
value prop.

Discussion of this in CZO [1].

[1] https://chat.zulip.org/#narrow/stream/48-mobile/topic/Automatic.20dark.2Flight.20preference

Fixes part of zulip#4009
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue May 10, 2021
Created by copy-pasting `LanguagePickerItem`.

In the next several commits, we'll make this into a more
general-purpose component that may be useful to represent a checkbox
or a radio button.

Then we'll switch `LanguagePickerItem`'s caller to use this
component instead, and remove `LanguagePickerItem`. We'll also use
it for the items on `UserStatusScreen`, which now incorrectly show
right-facing arrows, as though they'll navigate somewhere.

Future uses of this component (e.g., we may want it for zulip#4009) can
help refine its interface. For now, just let that process start --
and make it easier to provide a consistent look-and-feel -- by
making a reusable component.
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jun 25, 2021
Created by copy-pasting `LanguagePickerItem`.

In the next several commits, we'll make this into a more
general-purpose component that may be useful to represent a checkbox
or a radio button.

Then we'll switch `LanguagePickerItem`'s caller to use this
component instead, and remove `LanguagePickerItem`. We'll also use
it for the items on `UserStatusScreen`, which now incorrectly show
right-facing arrows, as though they'll navigate somewhere.

Future uses of this component (e.g., we may want it for zulip#4009) can
help refine its interface. For now, just let that process start --
and make it easier to provide a consistent look-and-feel -- by
making a reusable component.
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jul 2, 2021
As Android's dark theme guide says is necessary [1].

Done now, to follow the template-app change in
facebook/react-native@2b56011f9, on the path to the RN v0.64
upgrade.

The RN commit contains other changes to the template app that we
don't transfer to our project right now:

- The placeholder App.js starts using `useColorScheme` [1], an API
  that was new in RN v0.62. We may want to use this API in some
  places, during or after zulip#4009, but it doesn't seem necessary now.

- On iOS, the root view defined in AppDelegate.m gets its
  `.backgroundColor` set to `[UIColor systemBackgroundColor]` when
  possible. That seems intended to vary the background with the
  system-set theme (dark or light). We don't want to make this
  change: we've so far decided to set `.loadingView` instead of
  `.backgroundColor`, and we do so in a way that explicitly doesn't
  change with the theme (it uses the brand color).

- On iOS, the launch-screen storyboard is changed to adapt to the
  system-set theme. No need to make that change; we've so far chosen
  not to have it change with the theme, and just to use our brand
  color.

[1] https://developer.android.com/guide/topics/ui/look-and-feel/darktheme#support-dark-theme
[2] https://reactnative.dev/docs/usecolorscheme
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jul 2, 2021
As Android's dark theme guide says is necessary [1].

Done now, to follow the template-app change in
facebook/react-native@2b56011f9, on the path to the RN v0.64
upgrade.

The RN commit contains other changes to the template app that we
don't transfer to our project right now:

- The placeholder App.js starts using `useColorScheme` [2], an API
  that was new in RN v0.62. We may want to use this API in some
  places, during or after zulip#4009, but it doesn't seem necessary now.

- On iOS, the root view defined in AppDelegate.m gets its
  `.backgroundColor` set to `[UIColor systemBackgroundColor]` when
  possible. That seems intended to vary the background with the
  system-set theme (dark or light). We don't want to make this
  change: we've so far decided to set `.loadingView` instead of
  `.backgroundColor`, and we do so in a way that explicitly doesn't
  change with the theme (it uses the brand color).

- On iOS, the launch-screen storyboard is changed to adapt to the
  system-set theme. No need to make that change; we've so far chosen
  not to have it change with the theme, and just to use our brand
  color.

[1] https://developer.android.com/guide/topics/ui/look-and-feel/darktheme#support-dark-theme
[2] https://reactnative.dev/docs/usecolorscheme
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jul 7, 2021
As Android's dark theme guide says is necessary [1].

Done now, to follow the template-app change in
facebook/react-native@2b56011f9, on the path to the RN v0.64
upgrade.

The RN commit contains other changes to the template app that we
don't transfer to our project right now:

- The placeholder App.js starts using `useColorScheme` [2], an API
  that was new in RN v0.62. We may want to use this API in some
  places, during or after zulip#4009, but it doesn't seem necessary now.

- On iOS, the root view defined in AppDelegate.m gets its
  `.backgroundColor` set to `[UIColor systemBackgroundColor]` when
  possible. That seems intended to vary the background with the
  system-set theme (dark or light). We don't want to make this
  change: we've so far decided to set `.loadingView` instead of
  `.backgroundColor`, and we do so in a way that explicitly doesn't
  change with the theme (it uses the brand color).

- On iOS, the launch-screen storyboard is changed to adapt to the
  system-set theme. No need to make that change; we've so far chosen
  not to have it change with the theme, and just to use our brand
  color.

[1] https://developer.android.com/guide/topics/ui/look-and-feel/darktheme#support-dark-theme
[2] https://reactnative.dev/docs/usecolorscheme
chrisbobbe added a commit that referenced this issue Jul 8, 2021
Created by copy-pasting `LanguagePickerItem`.

In the next several commits, we'll make this into a more
general-purpose component that may be useful to represent a checkbox
or a radio button.

Then we'll switch `LanguagePickerItem`'s caller to use this
component instead, and remove `LanguagePickerItem`. We'll also use
it for the items on `UserStatusScreen`, which now incorrectly show
right-facing arrows, as though they'll navigate somewhere.

Future uses of this component (e.g., we may want it for #4009) can
help refine its interface. For now, just let that process start --
and make it easier to provide a consistent look-and-feel -- by
making a reusable component.
@chrisbobbe
Copy link
Contributor

chrisbobbe commented Sep 15, 2022

For explicitness, I think the task is to change the current "Night mode" switch on the settings screen, so that it's called "Theme" (or similar) and lets the user choose among these three options (naming TBD):

  • Light
  • Dark
  • Follow OS

And to rewire our Redux logic for this setting, and also use that bit of React Native API I mentioned above, so the setting works as expected.

For an example of changing a setting with two options ("on"/"off") into a setting with three options, see what we did with "Mark messages as read on scroll" in 6e88153 / #5469. It used a UI component that I think we should use here, which we hadn't built back when this issue was filed.

@BigHeadCreations
Copy link

Hey there @chrisbobbe and @gnprice, I'll take a shot at this Issue.

@chrisbobbe
Copy link
Contributor

Great, thanks! I see you've started a discussion on CZO; thanks! 🙂

@alya
Copy link
Collaborator

alya commented Oct 21, 2022

Does resolving this issue also solve #5169?

@gnprice
Copy link
Member Author

gnprice commented Oct 21, 2022

Yep, it does.

@gnprice
Copy link
Member Author

gnprice commented Nov 3, 2022

This is still a feature we want, but we've just discovered -- see #5527 (comment) -- that our intended approach for solving it won't work (due to a bug in React Native) and we'll need something a bit more complex.

Because this issue thread has gotten long, I just filed a fresh issue laying out the new approach: #5533. Closing this one in favor of that.

@gnprice gnprice closed this as not planned Won't fix, can't repro, duplicate, stale Nov 3, 2022
@gnprice
Copy link
Member Author

gnprice commented Nov 3, 2022

(I think I've covered in the new issue all the information from this thread that we'll need. But if I've missed something, please comment there.)

@gnprice gnprice changed the title Automatically follow user's OS-level dark/light preference Automatically follow user's OS-level dark/light preference, take 1 Nov 3, 2022
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 a pull request may close this issue.

5 participants