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

[RNMobile] RangeCell styling refactor #18157

Merged
merged 41 commits into from
Nov 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
5f3a400
Revert package-lock.json
lukewalczak Oct 17, 2019
25832f0
Merge branch 'master' into rnmobile/spacer
lukewalczak Oct 17, 2019
335154d
Setting attributes for spacer height
lukewalczak Oct 18, 2019
d137d38
Correct the condition for setting maximum value
lukewalczak Oct 21, 2019
8d37161
Merge branch 'master' into rnmobile/spacer
lukewalczak Oct 21, 2019
ae5715f
Small code refactor
lukewalczak Oct 23, 2019
a1cf07b
Improve Accessibility in range-cell
lukewalczak Oct 23, 2019
f0b3b28
Merge branch 'master' into rnmobile/spacer
lukewalczak Oct 23, 2019
a807c31
More accessibility improvements
lukewalczak Oct 23, 2019
ca3b8d4
Small code refactor
lukewalczak Oct 23, 2019
a390d88
Styling spacer refactor
lukewalczak Oct 24, 2019
90fe027
Move logic to RangeCell
lukewalczak Oct 24, 2019
a1d9a10
Keep Slider along with TextInput within RangeCell
lukewalczak Oct 24, 2019
833cc76
Small cleanup
lukewalczak Oct 24, 2019
103493e
Merge branch 'master' into rnmobile/spacer
lukewalczak Oct 24, 2019
74d020a
Fix missing binds
lukewalczak Oct 24, 2019
8c127b9
Fix focusing slider on iphoneX when VO is on
lukewalczak Oct 28, 2019
84ade89
Adjust a11y voice over
lukewalczak Oct 28, 2019
11e0911
Refactor RangeCell styles
lukewalczak Oct 28, 2019
1debc8c
Refactor pointerEvents when screen reader is on
lukewalczak Oct 28, 2019
4a897fd
Announce current value when finished
lukewalczak Oct 28, 2019
80a4714
Merge branch 'rnmobile/spacer' into rnmobile/rangecell-styling
lukewalczak Oct 28, 2019
047a08e
Small cleanup
lukewalczak Oct 29, 2019
7ee822e
Improve a11y
lukewalczak Oct 29, 2019
6d4dde4
Merge branch 'rnmobile/spacer' into rnmobile/rangecell-styling
lukewalczak Oct 29, 2019
014cd20
Fix a11y on iPhoneX
lukewalczak Oct 30, 2019
9f28b60
Merge branch 'rnmobile/spacer' into rnmobile/rangecell-styling
lukewalczak Oct 30, 2019
2e5c346
Refactor after CR
lukewalczak Oct 30, 2019
2d643c0
Update info for translators
lukewalczak Oct 30, 2019
f66a118
Merge branch 'rnmobile/spacer' into rnmobile/rangecell-styling
lukewalczak Oct 30, 2019
80104f1
Correct styles
lukewalczak Oct 30, 2019
93b7d58
Merge branch 'master' into rnmobile/rangecell-styling
lukewalczak Nov 5, 2019
26e7939
Refactor texts localizing
lukewalczak Nov 5, 2019
d321afd
merge master
jbinda Nov 6, 2019
b9c2b64
fix allowReset prop
jbinda Nov 7, 2019
cf85c25
Patch allowReset
lukewalczak Nov 7, 2019
500d414
a11y improvements
lukewalczak Nov 7, 2019
5d89738
Refactor custom button
lukewalczak Nov 8, 2019
7cd2f4f
Remove numberOfLines from cell label
lukewalczak Nov 8, 2019
ac07efa
Merge branch 'master' into rnmobile/rangecell-styling
lukewalczak Nov 15, 2019
135c4eb
Merge branch 'master' into rnmobile/rangecell-styling
lukewalczak Nov 15, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/block-library/src/spacer/edit.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ const SpacerEdit = ( { isSelected, attributes, setAttributes, getStylesFromColor
<InspectorControls>
<PanelBody title={ __( 'Spacer Settings' ) } >
<RangeControl
icon={ 'admin-settings' }
label={ __( 'Height in pixels' ) }
minimumValue={ minSpacerHeight }
maximumValue={ sliderSpacerMaxHeight }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
.borderStyle {
border-bottom-width: 1px;
}

.isSelected {
border-bottom-width: 2px;
border-color: $blue-wordpress;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
.isSelected {
border-width: 2px;
border-color: $blue-wordpress;
}

.borderStyle {
border-width: 1px;
border-radius: 4px;
}
40 changes: 27 additions & 13 deletions packages/components/src/mobile/bottom-sheet/cell.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,15 @@ class BottomSheetCell extends Component {
leftAlign,
labelStyle = {},
valueStyle = {},
cellContainerStyle = {},
cellRowContainerStyle = {},
onChangeValue,
children,
editable = true,
separatorType,
style = {},
getStylesFromColorScheme,
customActionButton,
...valueProps
} = this.props;

Expand All @@ -86,11 +89,14 @@ class BottomSheetCell extends Component {
const cellLabelCenteredStyle = getStylesFromColorScheme( styles.cellLabelCentered, styles.cellTextDark );
const cellLabelLeftAlignNoIconStyle = getStylesFromColorScheme( styles.cellLabelLeftAlignNoIcon, styles.cellTextDark );
const defaultMissingIconAndValue = leftAlign ? cellLabelLeftAlignNoIconStyle : cellLabelCenteredStyle;
const defaultLabelStyle = showValue || icon !== undefined ? cellLabelStyle : defaultMissingIconAndValue;
const defaultLabelStyle = showValue || icon !== undefined || customActionButton ? cellLabelStyle : defaultMissingIconAndValue;

const drawSeparator = ( separatorType && separatorType !== 'none' ) || separatorStyle === undefined;
const drawTopSeparator = drawSeparator && separatorType === 'topFullWidth';

const cellContainerStyles = [ styles.cellContainer, cellContainerStyle ];
const rowContainerStyles = [ styles.cellRowContainer, cellRowContainerStyle ];

const onCellPress = () => {
if ( isValueEditable ) {
startEditing();
Expand Down Expand Up @@ -183,7 +189,9 @@ class BottomSheetCell extends Component {
};

const iconStyle = getStylesFromColorScheme( styles.icon, styles.iconDark );
const resetButtonStyle = getStylesFromColorScheme( styles.resetButton, styles.resetButtonDark );
const containerPointerEvents = this.state.isScreenReaderEnabled && accessible ? 'none' : 'auto';
const { title, handler } = customActionButton || {};

return (
<TouchableOpacity
Expand All @@ -196,22 +204,28 @@ class BottomSheetCell extends Component {
accessibilityHint
}
onPress={ onCellPress }
style={ { ...styles.clipToBounds, ...style } }
style={ [ styles.clipToBounds, style ] }
>
{ drawTopSeparator && (
<View style={ separatorStyle() } />
) }
<View style={ styles.cellContainer } pointerEvents={ containerPointerEvents }>
<View style={ styles.cellRowContainer }>
{ icon && (
<View style={ styles.cellRowContainer }>
<Dashicon icon={ icon } size={ 24 } color={ iconStyle.color } />
<View style={ platformStyles.labelIconSeparator } />
</View>
) }
<Text numberOfLines={ 1 } style={ [ defaultLabelStyle, labelStyle ] }>
{ label }
</Text>
<View style={ cellContainerStyles } pointerEvents={ containerPointerEvents }>
<View style={ rowContainerStyles }>
<View style={ styles.cellRowContainer }>
{ icon && (
<View style={ styles.cellRowContainer }>
<Dashicon icon={ icon } size={ 24 } color={ iconStyle.color } />
<View style={ platformStyles.labelIconSeparator } />
</View>
) }
<Text style={ [ defaultLabelStyle, labelStyle ] }>
{ label }
</Text>
</View>
{ customActionButton && <TouchableOpacity onPress={ handler } accessibilityRole={ 'button' }>
<Text style={ resetButtonStyle }>{ title }
</Text>
</TouchableOpacity> }
</View>
{ showValue && getValueComponent() }
{ children }
Expand Down
100 changes: 68 additions & 32 deletions packages/components/src/mobile/bottom-sheet/range-cell.native.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
/**
* External dependencies
*/
import { Platform, AccessibilityInfo, findNodeHandle, TextInput, Slider } from 'react-native';
import { Platform, AccessibilityInfo, findNodeHandle, TextInput, Slider, View, PixelRatio, AppState } from 'react-native';

/**
* WordPress dependencies
*/
import { _x, __, sprintf } from '@wordpress/i18n';
import { Component } from '@wordpress/element';
import { withPreferredColorScheme } from '@wordpress/compose';

/**
* Internal dependencies
*/
import Cell from './cell';
import styles from './range-cell.scss';
import borderStyles from './borderStyles.scss';

class BottomSheetRangeCell extends Component {
constructor( props ) {
Expand All @@ -24,10 +26,12 @@ class BottomSheetRangeCell extends Component {
this.handleReset = this.handleReset.bind( this );
this.onChangeValue = this.onChangeValue.bind( this );
this.onCellPress = this.onCellPress.bind( this );
this.handleChangePixelRatio = this.handleChangePixelRatio.bind( this );

const initialValue = this.validateInput( props.value || props.defaultValue || props.minimumValue );
const fontScale = this.getFontScale();

this.state = { accessible: true, sliderValue: initialValue, initialValue, hasFocus: false };
this.state = { accessible: true, sliderValue: initialValue, initialValue, hasFocus: false, fontScale };
}

componentDidUpdate( ) {
Expand All @@ -37,8 +41,23 @@ class BottomSheetRangeCell extends Component {
}
}

componentDidMount() {
AppState.addEventListener( 'change', this.handleChangePixelRatio );
}

componentWillUnmount() {
this.handleToggleFocus();
AppState.removeEventListener( 'change', this.handleChangePixelRatio );
}

getFontScale() {
return PixelRatio.getFontScale() < 1 ? 1 : PixelRatio.getFontScale();
}

handleChangePixelRatio( nextAppState ) {
if ( nextAppState === 'active' ) {
this.setState( { fontScale: this.getFontScale() } );
}
}
Comment on lines +44 to 61
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I think that it's worth to save that info in redux store, because it can be useful in the future to control other components 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does sound like it would be helpful! But maybe it's too much for this PR.


handleChange( text ) {
Expand Down Expand Up @@ -115,13 +134,16 @@ class BottomSheetRangeCell extends Component {
maximumValue = 10,
disabled,
step = 1,
minimumTrackTintColor = '#00669b',
preferredColorScheme,
minimumTrackTintColor = preferredColorScheme === 'light' ? '#00669b' : '#5198d9',
maximumTrackTintColor = Platform.OS === 'ios' ? '#e9eff3' : '#909090',
thumbTintColor = Platform.OS === 'android' && '#00669b',
getStylesFromColorScheme,
allowReset = true,
...cellProps
} = this.props;

const { hasFocus, sliderValue, accessible } = this.state;
const { hasFocus, sliderValue, accessible, fontScale } = this.state;

const accessibilityLabel =
sprintf(
Expand All @@ -130,49 +152,63 @@ class BottomSheetRangeCell extends Component {
cellProps.label, value
);

const defaultSliderStyle = getStylesFromColorScheme( styles.sliderTextInput, styles.sliderDarkTextInput );
const resetButtonText = Platform.OS === 'ios' ? __( 'Reset' ) : __( 'RESET' );
const resetButton = { title: resetButtonText, handler: this.handleReset };

return (
<Cell
{ ...cellProps }
cellContainerStyle={ styles.cellContainerStyles }
cellRowContainerStyle={ styles.cellRowStyles }
accessibilityRole={ 'none' }
editable={ false }
accessible={ accessible }
onPress={ this.onCellPress }
accessibilityLabel={ accessibilityLabel }
customActionButton={ allowReset ? resetButton : undefined }
accessibilityHint={
/* translators: accessibility text (hint for focusing a slider) */
__( 'Double tap to change the value using slider' )
}
>
<Slider
value={ this.validateInput( sliderValue ) }
defaultValue={ defaultValue }
disabled={ disabled }
step={ step }
minimumValue={ minimumValue }
maximumValue={ maximumValue }
minimumTrackTintColor={ minimumTrackTintColor }
maximumTrackTintColor={ maximumTrackTintColor }
thumbTintColor={ thumbTintColor }
onValueChange={ this.handleChange }
onSlidingComplete={ this.handleValueSave }
ref={ ( slider ) => {
this.sliderRef = slider;
} }
style={ styles.slider }
accessibilityRole={ 'adjustable' }
/>
<TextInput
style={ [ styles.sliderTextInput, hasFocus ? styles.isSelected : {} ] }
onChangeText={ this.handleChange }
onFocus={ this.handleToggleFocus }
onBlur={ this.handleToggleFocus }
keyboardType="number-pad"
returnKeyType="done"
value={ `${ sliderValue }` }
/>
<View style={ styles.container }>
<Slider
value={ this.validateInput( sliderValue ) }
defaultValue={ defaultValue }
disabled={ disabled }
step={ step }
minimumValue={ minimumValue }
maximumValue={ maximumValue }
minimumTrackTintColor={ minimumTrackTintColor }
maximumTrackTintColor={ maximumTrackTintColor }
thumbTintColor={ thumbTintColor }
onValueChange={ this.handleChange }
onSlidingComplete={ this.handleValueSave }
ref={ ( slider ) => {
this.sliderRef = slider;
} }
style={ styles.slider }
accessibilityRole={ 'adjustable' }
/>
<TextInput
style={ [
defaultSliderStyle,
borderStyles.borderStyle,
hasFocus && borderStyles.isSelected,
{ width: 40 * fontScale },
] }
onChangeText={ this.handleChange }
onFocus={ this.handleToggleFocus }
onBlur={ this.handleToggleFocus }
keyboardType="number-pad"
returnKeyType="done"
value={ `${ sliderValue }` }
/>
</View>
</Cell>
);
}
}

export default BottomSheetRangeCell;
export default withPreferredColorScheme( BottomSheetRangeCell );
30 changes: 25 additions & 5 deletions packages/components/src/mobile/bottom-sheet/range-cell.native.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,38 @@
}

.sliderTextInput {
width: 40px;
height: 25px;
min-height: 25px;
align-self: center;
margin-left: 10px;
border-width: 1px;
border-radius: 4px;
border-color: $dark-gray-150;
border-color: $light-gray-400;
padding-top: 0;
padding-bottom: 0;
text-align: center;
}

.sliderDarkTextInput {
border-color: $gray-70;
}

.isSelected {
border-width: 2px;
border-color: $blue-wordpress;
}

.container {
flex-direction: row;
align-items: center;
min-height: 48px;
}

.cellContainerStyles {
flex-direction: column;
align-items: flex-start;
}

.cellRowStyles {
min-height: 48px;
margin-bottom: -13px;
width: 100%;
justify-content: space-between;
}
10 changes: 10 additions & 0 deletions packages/components/src/mobile/bottom-sheet/styles.native.scss
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,15 @@
padding: 5px;
}

.resetButton {
font-size: 17px;
color: $blue-wordpress;
}

.resetButtonDark {
color: $blue-30;
}

// Cell

.cellContainer {
Expand All @@ -101,6 +110,7 @@
.cellRowContainer {
flex-direction: row;
align-items: center;
flex-shrink: 1;
}

.cellIcon {
Expand Down