turnrye commented 5 years ago

Edit: A more general proposal has been posted by @tom-un to support generic platform defined colors. Jump to that proposal. That proposal now supersedes the one in this top post.

Original Proposal

### Introduction Android Q introduced a [Dark theme](https://developer.android.com/preview/features/darktheme) across the system and applications. From their presentation, ["Users will expect apps to have a dark theme"](https://youtu.be/OCHEjeLC_UY?t=1327). The way we do styling on RN today isn't directly compatible with their solution, but it's close. ### The Core of It I opened https://github.com/facebook/react-native/pull/24790 with a proposed solution, but it admittedly had some issues: - It was on `Platform`, but it's more prone to change than the rest of those values - It closely followed Android's implementation, but doesn't anticipate what iOS or out-of-tree platforms might use - Android normally recreates the activities when this value changes, but that isn't a safe way to lead to re-rendering (which would be common if the user has enabled it by time-of-day or as part of battery saver) ### Discussion Points - What's a better home for this than `Platform`? While it is a device setting, it's not quite in the same class as things like `Version`. - Should we stick with the interface that we know (Android's) or anticipate a more universal solution using perhaps https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-color-scheme

acoates-ms commented 5 years ago

@tom-un, might be able to fill in some more details. But for our macOS RN platform we extended the valid values for color. So you can specify either system colors, or be able to specify colors that change based on the native theme Something like:

I believe there was some plan to look at contributing that back for iOS too.

mmmulani commented 5 years ago

thanks for starting the proposal!

Android normally recreates the activities when this value changes, but that isn't a safe way to lead to re-rendering (which would be common if the user has enabled it by time-of-day or as part of battery saver)

from my reading of the docs, it seem like this changed and depending on how it's implemented, activities might not be recreated when the system theme changed. Because of that, I think a pattern similar to how we treat orientation makes more sense

turnrye commented 5 years ago

@acoates-ms will you describe that a bit more? That seems like a good solution for brownfield applications too where they need robust native themes. It's a solution that helps beyond just this light/dark mode use case and instead pushes themes into the native platform, which may or may not be desirable.

@mmmulani that makes a lot of sense. It'd also give more flexibility for other platforms as well that might not recreate automatically.

TheSavior commented 5 years ago

Also worth mentioning that we recently added an accessibility prop on View called accessibilityIgnoresInvertColors. This is used in iOS inverted mode to keep things like images from being inverted. At Google IO they mentioned that Android will have support for auto dark mode'ing 😄 apps but that images can look weird then. I imagine Android has a similar flag for this. A PR that wires up accessibilityIgnoresInvertColors to the new Android thing would be much appreciated.

turnrye commented 5 years ago

@TheSavior I experimented with that some -- it works relatively well, but there are definitely places where people are going to need to manually handle this. In our apps that's the case, and when I tried some other popular RN apps it was mixed as well.

I'll definitely submit that PR -- wasn't familiar with that. I don't think it directly addresses this discussion though.

necolas commented 5 years ago

tom-un commented 5 years ago

On macOS Mojave, things works best when using semantic colors: one sets colors on NSView using semantic colors such as [NSColor windowBackgroundColor] etc. Every view in the hierarchy has an 'effectiveAppearance' property that can change when the system changes from light to dark mode or vis versa. When the layers backing the views re-render the conversion of the NSColor to a CGColor is automatically aware of the effectiveAppearance and you'll get a different RGB value in the CGColor. iOS doesn't have appearance changes yet, but its a good bet it will in the same way as Mojave soon -- looking forward to WWDC for more info there.

I prototyped a couple approaches when making our macOS react-native Mojave theme aware. The first approach was to make a JS Api similar to Platform. I called it SemanticColor.js and it behaved similar to Platform: on JS load it used NativeModule constants to initialize a map of semantic color names to RGB string. Then the JS subscribed to an 'theme has changed' event that would be send a new map -- similar to how Platform caches the screen orientation and then gets updated orientation values from an event.

The problem with this solution was that the JS App itself also had to subscribe to the "theme has changed" event and then had to do a hard re-render of the components so that StyleSheets would be reevaluated because the various StyleSheets now contains RGB values fetched from the SematnicColors.js dictionary. Doing such a hard re-render would invariably reset some App state that the App developer didn't bother persisting. A simple example RNTester which does persist which test panel the user has navigated to, and the filter string, but does't persist the scroll position in the list. So with every theme change the list pops back to the top. Also, the time to re-render the whole App can be too slow so that instead of the RN app participating in the nice Mojave theme change animation it would just pop to the new theme a second or two after the rest of the desktop.

So the second approach, which is what is in our macos fork, is the extension to the color type itself, so that a color can be expressed as the current RGB string values or a JSON object like { semantic: 'windowBackground' }. The RCTConvert object turns such JSON objects into a true NSColor semantic color. mac and iOS also have the concept of Asset colors: app defined named colors that have a light and dark variant. I made a RCTDynamic color (a sublcass of NSColor) that works like an Asset color. The JS can express the tuple { dynamic: { light: 'rgb', dark:'rgb' } } and RCTConvert turns it into a RCTDynamic.

The semantic names I came up with for { semantic: 'name' } map 1:1 to the mac NSColor semantic names. And hopefully those same names will be used in a future iOS. But obviously don't map to other platform 1:1. Perhaps we could define a platform agnostic semantic namespace that would serve apple/android/windows.

DimitarNestorov commented 5 years ago

This should continue the conversation: https://developer.apple.com/videos/play/wwdc2019/214/

kikisaints commented 5 years ago

Hey! Poking this thread a bit with some interest on the React Native for Windows side.

I'm a PM on that team (RN for Windows) and we're looking to enable support for the light and dark themes that we have.

With the introduction of Dark theme in iOS 13 - as @dimitarnestorov mentioned, and the Dark theme mode in Android it's probably time we had something like this for all systems.

We have an internal proposal to support both themes, as they're integral to developing on Windows, but I'd like to append that here to get the opinions of the community before we submit a larger spec PR to the main React Native github.

Here's a look at part of the spec I've written that I would like to introduce to React Native core. I actually work closely with @acoates-ms, so a lot of this may look familiar to his suggested approach above as well as similar to what @tom-un was looking at. I would love feedback :smile:

Responding to Light/Dark Themes in React Native

Summary

With the introduction of React Native for Windows, there needs to be a way to utilize/work with the Light and Dark themes at the JavaScript layer.

This feature was pitched in the React Native for Windows repo here.

Motivation

Any React Native developer that wishes to follow the guidance of a Windows app will need to have access to the theme brushes, or at the very least a way to know when the system theme has changed.

There are several reasons why this is important to the React Native developer when they run their app on Windows:

Many native Windows components will honor the Light/Dark theme changes automatically. Giving the app-developer access/knowledge of those changes reduces the "zebra" UI effect (zebra UI = a page in an app where some controls honor a theme system, while others do not, or honor a non-complimentary one)
Having access to a theme changed event allows a React Native developer to better brand and customize their app for Windows, by accommodating their own styles based on the theme selected by the user (typically set through system settings)
Exposing the Windows theme up to the JavaScript layer means React Native apps can blend more seamlessly into the Fluent Design system and native Windows platform more easily
Both Android and iOS 13 have or will have support for Dark themes, thus it is a necessity that we expose some way of detecting and handling theme changes at the RN layer.

Scope

There are two core aspects that need to be available to the app developer in order to have a Windows Light/Dark theme sensitive app:

#	Feature
1	Expose a way, at the JavaScript layer, to detect current theme and when the theme has changed

Basic examples

Example 1 : Set up and behaviors on theme changed

In this example, we'll look at three things:

How to set up your React Native app to be style and event sensitive to the system themes
How to switch styles when a theme change has occurred
Handling a theme changed event

Setting up your app to be sensitive to theme changes

First import the Platform API into your React Native app.

import { AppThemeState } from 'react-native';

Create a local variable to use in style conditionals or to reference elsewhere easily.

class MyAppClass extends Component {
  state = {
    appThemeState: AppThemeState.currentTheme,
  };
  ...
}

Switching styles based on the app's theme

If the app author wants to switch the style of their component manually based on the system's theme (Dark or Light), they can do so with CSS style conditionals.

<TextInput
    style={[styles.inputStyle, this.state.appThemeState.currentTheme == 'dark' ? styles.darkInput : styles.lightInput]}
</TextInput>

Handling a theme changed event

In this case an app author would like to do something or preform some behavior when the app's theme has changed.

Note: AppThemeState will be a subclass of NativeEventEmitter.

componentDidMount() {
  AppThemeState.currentTheme.addListener('themechanged', this.onAppThemeChanged);
}

onAppThemeChanged = (event) => {
  /*Logic on theme changed goes here*/
  this.setState({appThemeState: AppThemeState.currentTheme});
}

API Design Overview

A look at the APIs for the features and scope described above.

AppThemeState.currentTheme

The specifics of the event or API being fired/used.

API	Args	Returns	Discription
currentTheme	none	string	currentTheme returns the state of the system theme that the user or native app has set.

App theme enum

The currentTheme returns one of the following string values:

Property	Type	Description
dark	string	A string value defining that the native app is in Dark theme.
light	string	A string value defining that the native app is in Light theme.

Could potentially need a AppThemeState.isHighContrast to detect when the system has initiated a theme for accessible users.

DimitarNestorov commented 5 years ago

I thought of a bit different approach on the API though. I was thinking about developer experience and the thought of event emitters just scared me.

Let's start with the basics: useDarkMode - a hook that returns a boolean whether to use dark mode or not. useDarkModeContext - the hook that every other hook depends on. Returns 'light' or 'dark'.

I borrowed the idea from Apple about dynamic values (UIColor with a callback, 20:50 of the talk). However React Native doesn't use objects or functions for values, so I thought of dynamic stylesheets. Basically our good old StyleSheet with support for dynamic values.

const dynamicStyles = new DynamicStyleSheet({
    container: {
        backgroundColor: new DynamicValue('white', 'black'),
        flex: 1
    },
    text: {
        color: new DynamicValue('black', 'white'),
        textAlign: 'center'
    }
})

function Component() {
    const styles = useDynamicStyleSheet(dynamicStyles)

    return (
        <View style={styles.container}>
            <Text style={styles.text}>My text</Text>
        </View>
    )
}

And here's the benefit to such an approach: let's say that this component is being reused inside a view that has to be only in dark mode. No problem, useDynamicStyleSheet will use context underneath so all you'll have to do is provide the theme you want that component to be in:

function MyScreen() {
    return (
        <>
            <DarkModeProvider mode="dark">
                <Component />{/* will be rendered using dark theme */}
            </DarkModeProvider>

            <DarkModeProvider mode="light">
                <Component />{/* will be rendered using light theme */}
            </DarkModeProvider>

            <Component />{/* will be rendered using current theme */}
        </>
    )
}

DynamicValue would be just an object that holds values for each theme. Literally the source code:

export class DynamicValue<T> {
    constructor(public readonly light: T, public readonly dark: T) { }
}

DynamicStyleSheet creates two stylesheet objects for each theme upon construction. useDynamicStyleSheet returns the appropriate stylesheet.

useDynamicValue which just returns the appropriate value from a DynamicValue object or from args (first arg for light theme, second for dark theme).

const logoUri = new DynamicValue(require('./light.png'), require('./dark.png'))
function Logo() {
    const source = useDynamicValue(logoUri)
    return <Image source={source} />
}

function Input() {
    const placeholderColor = useDynamicValue('black', 'white')
    return <TextInput placeholderTextColor={placeholderColor} />
}

I got excited last week about dark mode on iOS and I already implemented those ideas: https://github.com/codemotionapps/react-native-dark-mode

These hooks currently work because of a native module, which publishes changes to the theme using event emitter. And if you want to subscribe to that event emitter all you have to do is import it (hopefully you don't).

To detect the theme change in iOS I decided to use method swizzling on UIScreens traitCollectionDidChange and a lot of static methods 😅. This approach can be avoided if theming becomes a part of the React Native core 🙌 (override traitCollectionDidChange on the root React Native view controller).

My proposal: A similar high level API, with an alternative for class components. And a better solution for the iOS implementation, something that would require a tiny change to RN core to keep it lean, have it as a module like AsyncStorage and the others.

tom-un commented 5 years ago

@dimitarnestorov : that's a really intriguing proposal. The DynamicValues and DynamicStyleSheet approach has advantages over an event emitter approach. (I prototyped an event emitter approach for macOS dark mode and noted the downsides in my post above). I also feel the semantic color values of the platform are also important. They're important for continuity across the device, but really important for continuity in apps that are hybrids contain a mix of pure native views and react-native views.

I finally made the time to extend the work I did in macOS react-native dark mode to iOS. As expected, the iOS mechanism revealed at WWDC is very similar to macOS. Here's what the prototype looks like:

RNTester-ios-dark-mode

The branch with the code changes is here: https://github.com/microsoft/react-native/compare/master...tom-un:tomun/ios-darkmode

My proposed change is a change to core: its a change to the ColorType itself to support a platform specific named color. Every platform has some notion of named colors, and adding them to each platform's implementation I believe is a win for the react-native platform. Dimitar's proposal is also really interesting in that it provides a generic dynamic type that is applicable for things beyond just colors, like dynamic image sets.

hramos commented 5 years ago

👋🏼 I'm tracking issues related to iOS 13 and Android Q, and supporting Dark themes is my current focus. First I'd like to thank everyone in the thread for the thoughtful discussion and for collecting all the relevant information. I've been having discussions around Dark Mode with other members of the core team, and I want to make sure folks here are aware of our current thinking.

@tom-un's proposal is compelling. By exposing the underlying [UI|NS]Color support for adaptive colors, it provides a seamless experience when switching between light and dark themes. It has a higher chance of appealing to iOS developers, and is already in use in the react-native-macos fork due to the introduction of Dark Mode in Mojave last year.

This does bring some questions:

What would adaptive color support on Android look like? This is an area I'm less familiar with.
Semantic colors work great for apps that use the default system appearance. Do we expect folks building apps with custom color palettes to maintain their own set of named dynamic colors?

For reference, my commit at https://github.com/hramos/react-native/commit/1cf85c727ee4b0e9902fb162f5e16c466951e836 takes Tom's work from https://github.com/microsoft/react-native/compare/master...tom-un:tomun/ios-darkmode and applies it on top of the core React Native repository, sans macOS-specific code given macOS is not yet supported in core.

It seems like providing a useDarkMode hook, as @dimitarnestorov suggested above, would be sufficient for supporting dark mode on both iOS and Android. In fact, your DarkModeProvider context approach is similar to how we handle light and dark themes in our components at Facebook. I probably would not go as far as supporting dynamic values / stylesheets yet, though.

How do folks feel about moving forward with a React Hook approach first, and revisit adaptive color support later?

necolas commented 5 years ago

Why not design something based on the web api that isn't limited to one theme?

DimitarNestorov commented 5 years ago

If you plan on using context we can then do a dynamic style sheet library similar to my approach in https://github.com/react-native-community

hramos commented 5 years ago

@dimitarnestorov At this point, I am not considering extending StyleSheet or adding a new DynamicStylesheet to core. The hook would be in core, however. It should be possible then to create a dynamic StyleSheet library then.

Nicolas brings up a good point. I had not fully considered the current approach to theme preferences in web yet. Would folks prefer something closer to usePreferredTheme as a hook that returns 'light', 'dark', or 'unspecified'/'no-preference'?

ide commented 5 years ago

A generic theme is definitely better. Dark-mode only will probably look short-sighted in a few years if there are other themes (not necessarily just color-oriented either e.g. 2d theme vs 3d themes).

brentvatne commented 5 years ago

A concern that I have about using the semantic and dynamic platform colors is that this won't translate well over to web because no equivalent exists there. This might be a good option to support but we should be careful about adding it, and we'll need to consider things like how well we can normalize the names across platforms and so on.

usePreferredTheme and an emitter that backs it seem like a good start. Should this be built as an external library that is included in new React Native projects and versioned separately? I think this could be helpful for folks who can't yet update to the latest React Native version but want to add support for the OS dark-mode to their app.

necolas commented 5 years ago

It might be that we (or RNWindows) can surface whether Windows High Contrast mode is active via this hook too (and hopefully MS will eventually expose it to web apps via the preferred-theme media query)

hramos commented 5 years ago

I've landed on the following approach:

Create a Appearance native module that exposes the current appearance, and emits changes.
Provide a useAppearance hook (similar to new useWindowDimensions hook).

What do folks think we should do with StatusBar and its barStyle prop? As it is, you could use the new Appearance module to choose the correct light-content/dark-content style as needed.

necolas commented 5 years ago

FWIW, the web platform has specced several independent queries related to this feature (roughly "user preferences"), including:

prefers-color-scheme (light, dark, no-preference)
prefers-reduced-motion (reduce, no-preference)
prefers-contrast (low, high, no-preference)
prefers-reduced-transparency (reduce, no-preference)

The first 2 are supported by modern browsers, the last 2 are not yet supported. In the future, there are likely to be queries related to ambient light level, font-size, audio, and whether the UA/OS is forcing colors (another part of OS-level high contrast modes). This is in addition to all the existing queries related to viewport dimensions, aspect ratio, orientation, resolution, etc.

Each of these queries can be passed to window.matchMedia - https://developer.mozilla.org/en-US/docs/Web/API/Window/matchMedia. It also allows you to subscribe only to changes you're interested in, rather than being notified every time something like viewport dimensions change.

brentvatne commented 5 years ago

What do folks think we should do with StatusBar and its barStyle prop? As it is, you could use the new Appearance module to choose the correct light-content/dark-content style as needed.

I think it's preferred to leave it as-is for now and discuss different pieces like this in separate RFCs. It's pretty easy to make a wrapper to account for theme in userspace: https://github.com/react-navigation/native/blob/f9334201893f93c81d6900811b481cee3818c748/src/Themed.js#L18-L37.

brentvatne commented 5 years ago

@hramos @necolas - do you think UserPreferences would be a better name than Appearance for this module?

type UserPreferencesConfig = {
  colorScheme: 'light' | 'dark' | 'no-preference';
  reducedMotion: 'reduce' | 'no-preference';
  contrast: 'low' | 'high' | 'no-preference';
  reducedTransparency: 'reduce' | 'no-preference';
};

type UserPreferencesListener = (config: UserPreferencesConfig) => void;
type UserPreferencesEvents = 'change';

const get = () => UserPreferencesConfig;
const addChangeListener = (listener: UserPreferencesListener) => EventSubscription;
// or if you prefer this style
const addListener = (event: UserPreferencesEvent, listener: UserPreferencesListener) => EventSubscription;

for this first pass we could just expose the colorScheme property.

edit: perhaps UserPreferences is too generic - it could refer to preferred way to render dates like mm/dd/yyyy vs dd/mm/yyyy) and also clashes with NSUserDefaults a bit. Perhaps Appearance is fine after all, the above config still makes sense under that name.

hramos commented 5 years ago

I'm afraid UserPreferences by itself could be confusing. It's too similar to NSUserDefaults and might imply it's a module for storing user preferences, when in fact this is a read-only module. I don't have a suggestion, other than perhaps PreferredAppearance or UserPreferredAppearance. I'm including Appearance in the name because it looks like the proposed spec so far is all related to the user's preferred interface style or appearance. Can you think of a property that might be needed in the future that does not relate to appearance?

Edit: Naming nits aside, I agree with breaking this down into individual preferences (e.g. colorScheme, contrast). This is more in line with the existing Dimensions module.

brentvatne commented 5 years ago

@hramos - good timing, I just edited my comment with the same note re: UserPreferences being confusing. maybe best to just stick with Appearance and avoid bikeshedding too much on it for now.

hramos commented 5 years ago

This is what I've landed on, roughly:

// Appearance.js
const COLOR_SCHEME_NAME = {
  light: 'light',
  dark: 'dark',
  noPreference: 'no-preference',
};
type ColorSchemeName = $Keys<{
  light: string,
  dark: string,
  noPreference: string,
}>;
type AppearanceListener = (preferences: AppearancePreferences) => void;
type AppearancePreferences = {|
  colorScheme?: string,
/* Potential future keys:
  reducedMotion?: ReducedMotionName,
  contrast?: ContrastName,
  reducedTransparency?: ReducedTransparencyName,
*/
|};

class Appearance {
    static get(preference: string): ColorSchemeName;  
    static set(preferences: AppearancePreferences): void;
    static addChangeListener(listener: AppearanceListener): void;
    static removeChangeListener(listener: AppearanceListener): void;
}

// useColorScheme.js (React Hook)
function useColorScheme(): ColorSchemeName;

fbartho commented 5 years ago

I prototyped dark mode and shipped it in our app as a hack day project a month ago, as a hidden setting.

We already had dynamic code building (and caching) our style sheets, so that we could have adaptive layouts when a screen rotated to landscape, or if a screen had multiple breakpoints based on the rendered aspect ratio. (Think iPad split screen, live resizing). We also use it as a key part of having semantic color palettes that could potentially differ from screen to screen. Final major feature it interacts with is Semantic Text Styling and careful control over Dynamic Type.

So it was relatively straightforward to add a new dynamic dependency on a “dark mode” flag. We had a hook much like the one proposed above to read a setting somewhere, and the plan was to wrap a custom native API event emitter if React-Native Core didn’t implement this feature.

~The biggest problem we ran into was that react-navigation doesn’t make it straightforward to change Navigation Bar, Tab Bar,… styling, without completely resetting our navigation hierarchy. We speculated that we would have to guard against automatic theme adaptation, if the user was in certain Application-specific flows, or on screens with unsaved changes. (This obviously is undesirable)~ EDIT: @BrentVatne has links on how to resolve this below!

brentvatne commented 5 years ago

@fbartho - have you seen https://reactnavigation.org/docs/en/themes.html? also https://twitter.com/notbrent/status/1161718496247422976

fbartho commented 5 years ago

Oh man @brentvatne! Thanks! I totally hadn't seen the Themeing guide. -- We already pass stuff through React Context, and use navigationOptions-as-a-function on various screens, but I completely forgot the part where screenProps was a viable place to pass in things that otherwise might come through ReactContext (We're doing it today to provide react-intl when we're adjusting tabs/nav titles, and some analytics stuff). I also got tunnel vision & forgot that we could convert all our top-level Navigation Router configuration so that each navigationOptions hash instead dynamically evaluates using screenProps.

Thanks again!

gaodeng commented 5 years ago

Any ideas on how to add transition animation when switch theme ?

brentvatne commented 5 years ago

@gaodeng - you have to do this yourself if you want it. we won't get that out of the box until dynamic colors are supported

hramos commented 5 years ago

iOS support is on master, Android is landing soon: https://github.com/facebook/react-native/commit/63fa3f21c5ab308def450bffb22054241a8842ef

Dexwell commented 5 years ago

@hramos What about the proposed semantic color support? iOS 13 has the concept of elevation:

A clever nuance of Dark Mode is that the base colour defaults to black, but the system has a concept of ‘elevation’. If a view is elevated, then the base colour changes to a dark grey. This applies to modal presentations inside the app (which now default to a card style by the way) and even the app itself. On the iPad, Slide Over apps are adapted to the elevated appearance for instance. This means they can visually contrast against the apps beneath them. It’s clever.

This would be a pain to implement manually.

DimitarNestorov commented 5 years ago

Let's take a step back @Dexwell. As far as I know it's not possible to create an "elevated" view in React Native, and how would that be handled in Android. Semantic colors are a problem for much further down the road IMO.

Dexwell commented 5 years ago

@dimitarnestorov I think you confuse Android View elevation with the new iOS 13 concept of system screen elevation. The quote above provides two examples of when Apple tells a screen it's elevated by providing it different system colors. When using semantic colors, this would be done automatically.

Semantic colors would also allow automatic adoption of the Increased Contrast preference and future design/accessibility changes. It would further enable RN apps to behave as native as Swift/Objective-C apps.

What more would justify semantic colors not now but only further down the road?

MrLoh commented 5 years ago

@dexwell not shipping semantic colors now is justified, because neither you nor anyone else has built a full implementation yet and there is not even an architecture agreed on that will work across iOS, Android, windows, and web.

This is an open source project. It’ll be great to have support for dark mode detection for now.

Dexwell commented 5 years ago

@MrLoh Oh, I'm not opposing shipping dark mode detection! Just adding my 2 cents about semantic colors still being important :)

MrLoh commented 4 years ago

@hramos did this make it into the 61 release?

ksushiva commented 4 years ago

@MrLoh I just checked and it does not seem to be in the 61 or 61.1 release.

brentvatne commented 4 years ago

if someone wants to port the android side into react-native-appearance, which is where i extracted the ios side of @hramos' implementation to, it'd be welcome here: https://github.com/expo/react-native-appearance

you'll need to change it so it doesn't use turbomodules though!

Ashoat commented 4 years ago

Quick question - why is this module being integrated into core given the Lean Core effort? @dimitarnestorov's react-native-dark-mode seemingly already provides the functionality being discussed here, and allows for quicker iteration by being uncoupled from React Native releases. The approach of integrating this functionality into core seems like it's already proven at least a little problematic, given that no version of it has been released despite iOS 13 and Android 10 having both been released last month.

The closest reason I could see above was to avoid swizzling traitCollectionDidChange on iOS. Is there a related performance concern or is it primarily about taking a more elegant approach?

Apologies if this question was addressed earlier and I missed it.

kikisaints commented 4 years ago

Hey all, wanted to raise awareness about the fact that we have already implemented this api feature in the react-native-windows repo and I've created a proposal that details out the work that we did there.

I would imagine that the end result would be us making a PR from the react-native-windows repo of that implementation into the Facebook react-native repo.

Thoughts?

DimitarNestorov commented 4 years ago

@kikisaints am I missing something? Appearance is already in master (https://github.com/react-native-community/discussions-and-proposals/issues/126#issuecomment-526786767), why add a second module that does the same thing?

@Ashoat it's not because of method swizzling, https://github.com/expo/react-native-appearance already has it implemented without swizzling. I can do the same thing in react-native-dark-mode but why fix something that is not broken.

kikisaints commented 4 years ago

@dimitarnestorov

Clearly I do not move as quickly as the RN community does 😅 I'll get there. That being said, I don't see this documented... may I ask why is this being implemented separately - one for iOS and one for Android?

tom-un commented 4 years ago

Hi all,

The Appearance API is a great solution for making apps aware of system theme changes. As I've argued above, I still believe it is important for react-native to also support true native platform colors, such as mac and iOS semantic colors. Its of particular importance to brownfield apps that mix pure native and react-native views together.

I have a proposal and working prototype that implements a PlatformColor() type that can be used wherever ColorValue types are used: StyleSheets, props and method.

PlatformColor proposal

Background

Every native platform has the concept of system defined colors. Instead of specifying a concrete color value the app developer can choose a system color that varies in appearance depending on a system theme settings such Light or Dark mode, accessibility settings such as a High Contrast mode, and even its context within the app such as the traits of a containing view or window.

React-native requires that the app developer specify colors as RGB or HSL color values that are not aware of system theme, accessibility, or contextual state. A react-native app developer can approximate awareness of these states by using the new Appearance API and/or AccessibilityInfo API and making alternate sets of the RGB values depending on these states. These approximations are not only costly to develop but will always be just an approximation and will not be consistent between OS releases. In a brownfield app the inconsistencies between pure native component colors and react-native component colors can be pronounced.

Proposal

The proposal is to add true platform color support to react-native by extending the Flow type ColorValue with platform specific color type information for each platform and to provide a convenience function, PlatformColor(), for instantiating platform specific ColorValue objects.

PlatformColor(name, options) where name is a system color name on a given platform, and options is an Object with additional platform specific color parameters. The function returns a ColorValue.

iOS

On iOS, the name argument is one of the iOS UI Element or Standard Color names such as labelColor or systemFillColor

example: <View style={{ backgroundColor: PlatformColor('systemFillColor') }}/>

macOS

On macOS, the name argument is one the macOS UI Element or Standard Color names such as textColor or windowBackgroundColor

example: <View style={{ backgroundColor: PlatformColor('windowBackgroundColor') }}/>

iOS and macOS

On the Apple platforms the options argument may contain a tuple contains custom colors for light and dark themes, and/or normal and high contrast colors. Such dynamic colors are useful for branding colors or other app specific colors that still respond automatically to system setting changes.

example: <View style={{ backgroundColor: PlatformColor('', {dynamic: {light: 'black', dark: 'white'}}) }}/>

Windows

On Window, the name argument is a Windows system theme brush name such as SystemControlPageTextBaseHighBrush.

example: <View style={{ backgroundColor: PlatformColor('SystemControlPageTextBaseHighBrush') }}/>

Implementation

The ColorValue Flow type is currently defined in Libraries/StyleSheet/StyleSheetTypes.js. The current definition is:

export type ColorValue = null | string;

The ColorValue definition would move to new file Libraries/Colors/ColorValueTypes.js. The new definition of ColorValue is

export type ColorValue = null | string | NativeColorValue;

The NativeColorValue is defined in a NativeColorColorValueTypes.<platform>.js: one for each platform. Each platform defines its own NativeColorValue and its own PlatformColor() function. The NativeColorValue type contains platform specific data about a system color and would be coupled to the native bridge implementation, e.g. RCTConvert.m on iOS and macOS can convert colors with NativeColorValue data.

export type NativeColorValue = {
    // platform specific color data
};

The convenience function PlatformColor() is defined as:

export const PlatformColor = (
  name: string,
  options?: Object /* flowlint-line unclear-type: off */,
): ColorValue => {
    // platform specific code to construct platform specific ColorValue
};

There are two critical functions used in the processing of ColorValues: normalizeColor() and processColor(). Both are defined in .js files of the same names. These functions convert ColorValues into numeric values that encode an RGB value.

The normalizeColor() and processColor() return types need to be updated to return a number or native color objects. A new type ProcessedColorValue is defined for this purpose:

export type ProcessedColorValue = number | NativeColorValue;

The processColor() and normalizeColor() functions will still convert ColorValues that are string into numbers, but it will now also convert platform ColorValue's that are NativeColorValue objects into normalized NativeColorValues. To do so, it will be refactored to call new processColorObject() and normalizeColorObject() methods if the ColorValue contains platform information. A set of processColorObject.<platform>.js and normalizeColorObject.<platform>.js implementations must exist for each platform. The implementations of processColorObject() and normalizeColorObject() validate and process the PlatformColorObject as appropriate for the given platform, or just return null if the platform has no platform data.

Prototype

This proposal is an incremental change to platform color support that has already been implemented in the Microsoft fork of react-native for macOS and iOS (https://github.com/microsoft/react-native) and for Windows (https://github.com/microsoft/react-native-windows). The primary differences from what have already been implemented in the fork is the addition of the PlatformColor() function and the extension of the ColorValue type. There were several places in react-native where colors were not being specified as ColorValue but instead as string, string | number, or number. The prototype cleans up these instances.

The prototype has been implemented in branches of facebook/react-native, microsoft/react-native, and microsoft/react-native-windows.

The branch of facebook/react-native can be viewed here: https://github.com/facebook/react-native/compare/master...tom-un:fb-platformcolor
react-native for macOS branch: https://github.com/microsoft/react-native/compare/master...tom-un:tomun/platformcolors
react-native for Windows branch: https://github.com/microsoft/react-native-windows/compare/master...tom-un:tomun/platformcolors

iOS Demo

PlatformColor-ios

yungsters commented 4 years ago

@tom-un, as you already know… I am a huge supporter of the PlatformColor proposal. Thank you so much for taking the time to put this together in writing and code.

I have a couple pieces of feedback.

Cross-Platform and Cross-Version Colors

One concern I have is with cross-platform and backward compatibility.

I have not taken a close look at the code, but I assume that PlatformColor('does-not-exist') either renders as null would (e.g. transparent).

But what would the idiomatic way to specify a color that must be supported on multiple platforms where colors have different names? Or multiple versions of the same platform where system colors may have been added or renamed?

I have a proposal that borrows from how font families are defined in browsers. We should allow providing a list of named color strings, where the first valid named color is used. For example:

PlatformColor('systemFillColor')
PlatformColor('systemFillColor', 'windowBackgroundColor')
// or
PlatformColor(['systemFillColor'])
PlatformColor(['systemFillColor', 'windowBackgroundColor'])

If you think this is a good idea, I do not have a strong feeling which one of the above we end up using. (The decision may be informed by my following piece of feedback.) What do you think?

Platform Options

The example that you provided for options was:

PlatformColor('', {
  dynamic: {
    light: 'black',
    dark: 'white'
  }
})

Can you explain in what scenarios this would be necessary or preferred instead of extending the platform colors on the platform?

Ultimately for cross-platform convergence between native and web platforms, I see PlatformColor as an opportunity to converge with CSS Variables. If I wanted to define a new color that behaved differently from existing colors (e.g. a "Facebook blue" which should be the same on dark and light and high contrast modes), I would define a new CSS Variable. Why wouldn't I do the same for native platforms?

One reason I can think of for wanting options is to be able to stay in JavaScript. But I personally do not think that this is a sufficient reason and would rather see a solution that makes introducing new platform colors easier.

brentvatne commented 4 years ago

I think it would be useful to have a built-in tool for converting a NativeColorValue to a RGB or HSL value so the resolved color values aren't opaque to JS. The result would need to be recomputed when the theme or contrast changes.

I'm also curious to hear more about the Android and Web implementation details, and as @yungsters mentioned, how this API can be made to work great cross-platform.

acoates-ms commented 4 years ago

One problem with such a function is that the value may not have a RGB equivalent. My understanding is that the colors being used by the iOS/mac implementation here can be gradients or other textures that can be created on the native side. Similarly the windows implementation could be image brushes, or even more crazy things such as a reveal brush (which appears as a highly reflective texture reacting to a spot light from the cursor). These things are inherently more complex than a simple color.

We could try to make such a function that would work if the system color happens to be a simple color. But there are also threading concerns, so currently the function would likely have to be async, which probably limits the usefulness of such a function.

brentvatne commented 4 years ago

My understanding is that the colors being used by the iOS/mac implementation here can be gradients or other textures that can be created on the native side

I'm not familiar with the iOS/Mac gradient / texture colors, can you link me the relevant documentation?

Similarly the windows implementation could be image brushes, or even more crazy things such as a reveal brush (which appears as a highly reflective texture reacting to a spot light from the cursor). These things are inherently more complex than a simple color.

It doesn't feel right to me for this type of behavior to result from setting the "background color" property

TheSavior commented 4 years ago

I think it would be useful to have a built-in tool for converting a NativeColorValue to a RGB or HSL value so the resolved color values aren't opaque to JS.

@brentvatne, can you elaborate on why this would be helpful?

The purpose of this proposal is to support platform defined colors (and in Windows's case complex brushes which are exposed by the platform as colors).

For people who want to have these things controlled by JS, they could just use the existing APIs, right? This is an opt in approach for using colors defined by the system, or by native code.

Types

@tom-un, I'm not sure the proposed types using platform extension JS files would pass the type checker. For example, if platform supported options were defined in those platform files, then in a standard .js file, Flow would complain about code using windows options when type checking with .ios.js files, and vice versa.

I think to avoid that I'd propose avoiding options altogether.

Instead of needing something like this:

PlatformColor('', {
  dynamic: {
    light: 'black',
    dark: 'white'
  }
})

what about:

// As a NativeModule?
await SomeAPIToRegisterNativeColors.registerPlatformColor('BlackOnLightWhiteOnDark', someConfigurationOptions);

{
  backgroundColor: PlatformColor('BlackOnLightWhiteOnDark')
}

This way the validation of options and type checking can be separate from the callsite and I think the types become much simpler.

Some other invariants I'd like this implementation to enforce

PlatformColor returns some object that encodes the option for the native platform. I'd like to ensure people don't write these objects by hand and instead have to call PlatformColor. On the Flow side we've talked about PlatformColor returning an opaque type. I think this would be great, and hopefully doing something similar in TypeScript. I'd also want this to be enforced via an ESLint rule for those not using types.

I'd also like to make sure that this implementation comes with a lint rule enforcing that PlatformColor is called with a statically analyzable string name (not a variable). This will help enable some performance optimizations we have been doing with StyleX on web.

brentvatne commented 4 years ago

can you elaborate on why this would be helpful?

There are plenty of cases on native where you might want to manipulate a system defined color, and if you can't do that from JS then you will be writing a lot of color related native code. eg: what if I wanted to darken a color by 10%? or invert it? or use it with some API that only accepts rbg values?

The purpose of this proposal is to support platform defined colors (and in Windows's case complex brushes which are exposed by the platform as colors).

Do these colors work everywhere - are they actually colors? Does a "brush" work on text and every other place where color will typically work?

react-native-community / discussions-and-proposals

Platform specific colors via PlatformColor #126

Responding to Light/Dark Themes in React Native

Summary

Motivation

Scope

Basic examples

Example 1 : Set up and behaviors on theme changed

Setting up your app to be sensitive to theme changes

Switching styles based on the app's theme

Handling a theme changed event

API Design Overview

AppThemeState.currentTheme

App theme enum

PlatformColor proposal

Background

Proposal

iOS

macOS

iOS and macOS

Windows

Implementation

Prototype

iOS Demo

Cross-Platform and Cross-Version Colors

Platform Options

Types

Some other invariants I'd like this implementation to enforce