Skip to content

Commit

Permalink
Clearer code in Button.js
Browse files Browse the repository at this point in the history
Summary:
Thanks for submitting a PR! Please read these instructions carefully:

- [x] Explain the **motivation** for making this change.
- [x] Provide a **test plan** demonstrating that the code is solid.
- [x] Match the **code formatting** of the rest of the codebase.
- [x] Target the `master` branch, NOT a "stable" branch.

What existing problem does the pull request solve?
* Combined repeating `if` checks for clearer logic.
* `defaultBlue` is inlined because it is only used once for each OS.

- Updated the `Button.js` file in an existing project and all buttons behave exactly the same. Here is a simple demo:
  ![](http://i.makeagif.com/media/6-11-2017/B0Bhry.gif)

- There is no change in the underlying logic of the code.
Closes #14455

Differential Revision: D5330112

Pulled By: hramos

fbshipit-source-id: 7fd1a0609f0bb2123208d0e1b3da2bc779f9583d
  • Loading branch information
dpgao authored and facebook-github-bot committed Jun 27, 2017
1 parent 14538aa commit 7ee051d
Showing 1 changed file with 14 additions and 19 deletions.
33 changes: 14 additions & 19 deletions Libraries/Components/Button.js
Expand Up @@ -97,25 +97,25 @@ class Button extends React.Component {
} = this.props; } = this.props;
const buttonStyles = [styles.button]; const buttonStyles = [styles.button];
const textStyles = [styles.text]; const textStyles = [styles.text];
const Touchable = Platform.OS === 'android' ? TouchableNativeFeedback : TouchableOpacity; if (color) {
if (color && Platform.OS === 'ios') { if (Platform.OS === 'ios') {
textStyles.push({color: color}); textStyles.push({color: color});
} else if (color) { } else {
buttonStyles.push({backgroundColor: color}); buttonStyles.push({backgroundColor: color});
}
} }
const accessibilityTraits = ['button'];
if (disabled) { if (disabled) {
buttonStyles.push(styles.buttonDisabled); buttonStyles.push(styles.buttonDisabled);
textStyles.push(styles.textDisabled); textStyles.push(styles.textDisabled);
accessibilityTraits.push('disabled');
} }
invariant( invariant(
typeof title === 'string', typeof title === 'string',
'The title prop of a Button must be a string', 'The title prop of a Button must be a string',
); );
const formattedTitle = Platform.OS === 'android' ? title.toUpperCase() : title; const formattedTitle = Platform.OS === 'android' ? title.toUpperCase() : title;
const accessibilityTraits = ['button']; const Touchable = Platform.OS === 'android' ? TouchableNativeFeedback : TouchableOpacity;
if (disabled) {
accessibilityTraits.push('disabled');
}
return ( return (
<Touchable <Touchable
accessibilityComponentType="button" accessibilityComponentType="button"
Expand All @@ -132,32 +132,27 @@ class Button extends React.Component {
} }
} }


// Material design blue from https://material.google.com/style/color.html#color-color-palette
let defaultBlue = '#2196F3';
if (Platform.OS === 'ios') {
// Measured default tintColor from iOS 10
defaultBlue = '#007AFF';
}

const styles = StyleSheet.create({ const styles = StyleSheet.create({
button: Platform.select({ button: Platform.select({
ios: {}, ios: {},
android: { android: {
elevation: 4, elevation: 4,
backgroundColor: defaultBlue, // Material design blue from https://material.google.com/style/color.html#color-color-palette
backgroundColor: '#2196F3',
borderRadius: 2, borderRadius: 2,
}, },
}), }),
text: Platform.select({ text: Platform.select({
ios: { ios: {
color: defaultBlue, // iOS blue from https://developer.apple.com/ios/human-interface-guidelines/visual-design/color/
color: '#007AFF',
textAlign: 'center', textAlign: 'center',
padding: 8, padding: 8,
fontSize: 18, fontSize: 18,
}, },
android: { android: {
textAlign: 'center',
color: 'white', color: 'white',
textAlign: 'center',
padding: 8, padding: 8,
fontWeight: '500', fontWeight: '500',
}, },
Expand Down

0 comments on commit 7ee051d

Please sign in to comment.