Skip to content

Commit

Permalink
Add NativeColorType opaque type to normalizeColor() ahead of Platform…
Browse files Browse the repository at this point in the history
…Color PR. (#28040)

Summary:
The [PlatformColor PR](#27908) is currently open to implement the [PlatformColor proposal](react-native-community/discussions-and-proposals#126).   When that PR was imported into Facebooks internal builds it was found that the change to the `processColor()` function to return an opaque type or `number` instead of just `number` breaks internal components.

This PR is a simplification of the PlatformColor PR only changing the return type of `processColor()` from `?number` to `?number | NativeColorType` where `NativeColorType` is just an empty but opaque type.   This will allow changes to be made to these internal components but with less risk than the larger PR.

## Changelog

[General] [Changed] - Add NativeColorType opaque type to normalizeColor() ahead of PlatformColor PR
Pull Request resolved: #28040

Test Plan: Flow checks, Jest test, iOS unit tests, iOS integration tests, and manual testing performed on RNTester for iOS and Android.

Differential Revision: D19860205

Pulled By: TheSavior

fbshipit-source-id: 799662c6621d3974158b375ccccfa136982c43b4
  • Loading branch information
tom-un authored and facebook-github-bot committed Feb 20, 2020
1 parent 372771d commit dbe0d7d
Show file tree
Hide file tree
Showing 9 changed files with 80 additions and 16 deletions.
7 changes: 6 additions & 1 deletion Libraries/ActionSheetIOS/ActionSheetIOS.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,15 @@ const ActionSheetIOS = {
destructiveButtonIndices = [destructiveButtonIndex];
}

const processedTintColor = processColor(tintColor);
invariant(
processedTintColor == null || typeof processedTintColor === 'number',
'Unexpected color given for ActionSheetIOS.showActionSheetWithOptions tintColor',
);
RCTActionSheetManager.showActionSheetWithOptions(
{
...remainingOptions,
tintColor: processColor(tintColor),
tintColor: processedTintColor,
destructiveButtonIndices,
},
callback,
Expand Down
27 changes: 21 additions & 6 deletions Libraries/Components/CheckBox/CheckBox.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

const React = require('react');
const StyleSheet = require('../../StyleSheet/StyleSheet');
const invariant = require('invariant');
const processColor = require('../../StyleSheet/processColor');

const nullthrows = require('nullthrows');
Expand Down Expand Up @@ -152,12 +153,26 @@ class CheckBox extends React.Component<Props> {
};

_getTintColors(tintColors) {
return tintColors
? {
true: processColor(tintColors.true),
false: processColor(tintColors.false),
}
: undefined;
if (tintColors) {
const processedTextColorTrue = processColor(tintColors.true);
invariant(
processedTextColorTrue == null ||
typeof processedTextColorTrue === 'number',
'Unexpected color given for tintColors.true',
);
const processedTextColorFalse = processColor(tintColors.true);
invariant(
processedTextColorFalse == null ||
typeof processedTextColorFalse === 'number',
'Unexpected color given for tintColors.false',
);
return {
true: processedTextColorTrue,
false: processedTextColorFalse,
};
} else {
return undefined;
}
}

render() {
Expand Down
8 changes: 7 additions & 1 deletion Libraries/Components/Picker/PickerAndroid.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import AndroidDialogPickerNativeComponent, {
} from './AndroidDialogPickerNativeComponent';
import * as React from 'react';
import StyleSheet from '../../StyleSheet/StyleSheet';
import invariant from 'invariant';
import processColor from '../../StyleSheet/processColor';

import type {SyntheticEvent} from '../../Types/CoreEventTypes';
Expand Down Expand Up @@ -61,8 +62,13 @@ function PickerAndroid(props: Props): React.Node {
selected = index;
}
const {color, label} = child.props;
const processedColor = processColor(color);
invariant(
processedColor == null || typeof processedColor === 'number',
'Unexpected color given for PickerAndroid color prop',
);
return {
color: color == null ? null : processColor(color),
color: color == null ? null : processedColor,
label,
};
});
Expand Down
8 changes: 7 additions & 1 deletion Libraries/Components/Picker/PickerIOS.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const React = require('react');
const StyleSheet = require('../../StyleSheet/StyleSheet');
const View = require('../View/View');

const invariant = require('invariant');
const processColor = require('../../StyleSheet/processColor');

import RCTPickerNativeComponent, {
Expand Down Expand Up @@ -86,10 +87,15 @@ class PickerIOS extends React.Component<Props, State> {
if (child.props.value === props.selectedValue) {
selectedIndex = index;
}
const processedTextColor = processColor(child.props.color);
invariant(
processedTextColor == null || typeof processedTextColor === 'number',
'Unexpected color given for PickerIOSItem color',
);
items.push({
value: child.props.value,
label: child.props.label,
textColor: processColor(child.props.color),
textColor: processedTextColor,
});
});
return {selectedIndex, items};
Expand Down
9 changes: 9 additions & 0 deletions Libraries/Components/StatusBar/StatusBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
const Platform = require('../../Utilities/Platform');
const React = require('react');

const invariant = require('invariant');
const processColor = require('../../StyleSheet/processColor');

import NativeStatusBarManagerAndroid from './NativeStatusBarManagerAndroid';
Expand Down Expand Up @@ -322,6 +323,10 @@ class StatusBar extends React.Component<Props> {
);
return;
}
invariant(
typeof processedColor === 'number',
'Unexpected color given for StatusBar.setBackgroundColor',
);

NativeStatusBarManagerAndroid.setColor(processedColor, animated);
}
Expand Down Expand Up @@ -466,6 +471,10 @@ class StatusBar extends React.Component<Props> {
} parsed to null or undefined`,
);
} else {
invariant(
typeof processedColor === 'number',
'Unexpected color given in StatusBar._updatePropsStack',
);
NativeStatusBarManagerAndroid.setColor(
processedColor,
mergedProps.backgroundColor.animated,
Expand Down
18 changes: 13 additions & 5 deletions Libraries/Components/Touchable/TouchableNativeFeedback.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import Platform from '../../Utilities/Platform';
import View from '../../Components/View/View';
import processColor from '../../StyleSheet/processColor';
import * as React from 'react';
import invariant from 'invariant';

type Props = $ReadOnly<{|
...React.ElementConfig<TouchableWithoutFeedback>,
Expand Down Expand Up @@ -131,11 +132,18 @@ class TouchableNativeFeedback extends React.Component<Props, State> {
borderless: boolean,
color: ?number,
type: 'RippleAndroid',
|}> = (color: string, borderless: boolean) => ({
type: 'RippleAndroid',
color: processColor(color),
borderless,
});
|}> = (color: string, borderless: boolean) => {
const processedColor = processColor(color);
invariant(
processedColor == null || typeof processedColor === 'number',
'Unexpected color given for Ripple color',
);
return {
type: 'RippleAndroid',
color: processedColor,
borderless,
};
};

/**
* Whether `useForeground` is supported.
Expand Down
5 changes: 5 additions & 0 deletions Libraries/Share/Share.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ class Share {
return new Promise((resolve, reject) => {
const tintColor = processColor(options.tintColor);

invariant(
tintColor == null || typeof tintColor === 'number',
'Unexpected color given for options.tintColor',
);

invariant(
NativeActionSheetManager,
'NativeActionSheetManager is not registered on iOS, but it should be.',
Expand Down
9 changes: 8 additions & 1 deletion Libraries/StyleSheet/processColor.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,15 @@ const Platform = require('../Utilities/Platform');

const normalizeColor = require('./normalizeColor');

// TODO: This is an empty object for now, just to enforce that everything using this
// downstream is correct. This will be replaced with an import to other files
// with a platform specific implementation. See the PR for more information
// https://github.com/facebook/react-native/pull/27908
opaque type NativeColorType = {};
export type ProcessedColorValue = ?number | NativeColorType;

/* eslint no-bitwise: 0 */
function processColor(color?: ?(string | number)): ?number {
function processColor(color?: ?(string | number)): ProcessedColorValue {
if (color === undefined || color === null) {
return color;
}
Expand Down
5 changes: 4 additions & 1 deletion Libraries/StyleSheet/processColorArray.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@
'use strict';

const processColor = require('./processColor');
import type {ProcessedColorValue} from './processColor';

function processColorArray(colors: ?Array<string>): ?Array<?number> {
function processColorArray(
colors: ?Array<string>,
): ?Array<ProcessedColorValue> {
return colors == null ? null : colors.map(processColor);
}

Expand Down

0 comments on commit dbe0d7d

Please sign in to comment.