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] Add disabled style to Cell component #50665

Merged
merged 19 commits into from
May 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
74d18c0
Add disabled style to Cell component
fluiddot May 12, 2023
14c87a9
Add `disabled` prop to `BottomSheetTextControl` component
fluiddot May 12, 2023
c3a571f
Pass `disabled` prop in `BottomSheetSelectControl` component
fluiddot May 16, 2023
88cda9f
Pass `disabled` prop in `BottomSheetStepperCell` component
fluiddot May 16, 2023
4b93566
Pass `disabled` with the rest of cell props in `BottomSheetRangeCell`…
fluiddot May 16, 2023
44968cb
Pass `disabled` prop in `RangeControl` component
fluiddot May 16, 2023
763d198
Pass `disabled` with the rest of cell props in `BottomSheetSwitchCell…
fluiddot May 16, 2023
ec82149
Disable `TextInput` component in `Cell` component when disabled
fluiddot May 16, 2023
f877e8e
Add disabled state to `Cell` component children
fluiddot May 16, 2023
cbba7c6
Increase opacity of cell disabled style
fluiddot May 16, 2023
1723d2f
Allow customize `Cell` component disabled style
fluiddot May 16, 2023
f0ad3ff
Customize disabled style for `BottomSheetSwitchCell` component
fluiddot May 16, 2023
bd19b0a
Remove `aria-disabled` prop in children container of `Cell` component
fluiddot May 16, 2023
124e7be
Remove unneeded default value in `BottomSheetTextControl` component
fluiddot May 16, 2023
5b7590f
Pass `disabled` prop in `BottomSheetRangeCell` component
fluiddot May 16, 2023
2ac07ab
Only disable Slider component on Android
fluiddot May 16, 2023
78e23ba
Add placeholder text color for disabled state of `Cell`
fluiddot May 17, 2023
ad34c7f
Mock placeholder color style
fluiddot May 17, 2023
0a8de68
Update react-native-editor changelog
fluiddot May 17, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const BottomSheetSelectControl = ( {
options: items,
onChange,
value: selectedValue,
disabled,
} ) => {
const [ showSubSheet, setShowSubSheet ] = useState( false );
const navigation = useNavigation();
Expand Down Expand Up @@ -68,6 +69,7 @@ const BottomSheetSelectControl = ( {
__( 'Navigates to select %s' ),
label
) }
disabled={ disabled }
>
<Icon icon={ chevronRight }></Icon>
</BottomSheet.Cell>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const BottomSheetTextControl = ( {
icon,
footerNote,
cellPlaceholder,
disabled,
} ) => {
const [ showSubSheet, setShowSubSheet ] = useState( false );
const navigation = useNavigation();
Expand Down Expand Up @@ -62,6 +63,7 @@ const BottomSheetTextControl = ( {
onPress={ openSubSheet }
value={ initialValue || '' }
placeholder={ cellPlaceholder || placeholder || '' }
disabled={ disabled }
>
<Icon icon={ chevronRight }></Icon>
</BottomSheet.Cell>
Expand Down
31 changes: 26 additions & 5 deletions packages/components/src/mobile/bottom-sheet/cell.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ class BottomSheetCell extends Component {
accessibilityHint,
accessibilityRole,
disabled = false,
disabledStyle = styles.cellDisabled,
activeOpacity,
onPress,
onLongPress,
Expand Down Expand Up @@ -223,11 +224,22 @@ class BottomSheetCell extends Component {
styles.cellValue,
styles.cellTextDark
);
const finalStyle = {
const textInputStyle = {
...cellValueStyle,
...valueStyle,
...styleRTL,
};
const placeholderTextColor = disabled
? this.props.getStylesFromColorScheme(
styles.placeholderColorDisabled,
styles.placeholderColorDisabledDark
).color
: styles.placeholderColor.color;
Comment on lines +232 to +237
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I finally calculated the placeholder text color to handle the different cases.

Copy link
Member

Choose a reason for hiding this comment

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

This is great. I noticed this when I started my review and then noted you updated this code. :) The raw hex values stood out to me even when working on the Clear Button issue from a while ago -- thanks for addressing them here!

const textStyle = {
...( disabled && styles.cellDisabled ),
...cellValueStyle,
...valueStyle,
};

// To be able to show the `middle` ellipsizeMode on editable cells
// we show the TextInput just when the user wants to edit the value,
Expand All @@ -238,10 +250,10 @@ class BottomSheetCell extends Component {
<TextInput
ref={ ( c ) => ( this._valueTextInput = c ) }
numberOfLines={ 1 }
style={ finalStyle }
style={ textInputStyle }
value={ value }
placeholder={ valuePlaceholder }
placeholderTextColor={ '#87a6bc' }
placeholderTextColor={ placeholderTextColor }
onChangeText={ onChangeValue }
editable={ isValueEditable }
pointerEvents={
Expand All @@ -251,11 +263,12 @@ class BottomSheetCell extends Component {
onBlur={ finishEditing }
onSubmitEditing={ onSubmit }
keyboardType={ this.typeToKeyboardType( type, step ) }
disabled={ disabled }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The text input is also disabled.

{ ...valueProps }
/>
) : (
<Text
style={ { ...cellValueStyle, ...valueStyle } }
style={ textStyle }
numberOfLines={ 1 }
ellipsizeMode={ 'middle' }
>
Expand Down Expand Up @@ -418,7 +431,15 @@ class BottomSheetCell extends Component {
/>
) }
{ showValue && getValueComponent() }
{ children }
<View
style={ [
disabled && disabledStyle,
styles.cellRowContainer,
] }
pointerEvents={ disabled ? 'none' : 'auto' }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although the parent component (TouchableRipple) is disabled, I noticed that some controls render input elements that also need to be disabled to prevent interaction. Instead of disabling them one by one, I simply disabled all touch events for the children when the cell is disabled.

>
{ children }
</View>
</View>
{ help && (
<Text style={ [ cellHelpStyle, styles.placeholderColor ] }>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,15 @@ class BottomSheetRangeCell extends Component {
activeOpacity={ 1 }
accessible={ false }
valueStyle={ styles.valueStyle }
disabled={ disabled }
>
<View style={ containerStyle }>
{ preview }
<Slider
testID={ `Slider ${ cellProps.label }` }
value={ sliderValue }
defaultValue={ defaultValue }
disabled={ disabled }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

On iOS the default disabled style of the Slider already makes it transparent. So, in order to avoid making it too transparent with the Cell's disabled style, we only disable it on Android.

disabled={ disabled && ! isIOS }
step={ step }
minimumValue={ minimumValue }
maximumValue={ maximumValue }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ class BottomSheetStepperCell extends Component {
openUnitPicker,
decimalNum,
cellContainerStyle,
disabled,
} = this.props;
const { inputValue } = this.state;
const isMinValue = value === min;
Expand Down Expand Up @@ -215,6 +216,7 @@ class BottomSheetStepperCell extends Component {
labelStyle={ labelStyle }
leftAlign={ true }
separatorType={ separatorType }
disabled={ disabled }
>
<View style={ preview && containerStyle }>
{ preview }
Expand Down
14 changes: 13 additions & 1 deletion packages/components/src/mobile/bottom-sheet/styles.native.scss
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,15 @@

// used in both light and dark modes
.placeholderColor {
color: #87a6bc;
color: $gray;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed this color is already defined globally:

}

.placeholderColorDisabled {
color: lighten($gray, 20%);
}

.placeholderColorDisabledDark {
color: lighten($gray-dark, 10%);
}

.applyButton {
Expand Down Expand Up @@ -317,3 +325,7 @@
.cellSubLabelTextDark {
color: $sub-heading-dark;
}

.cellDisabled {
opacity: 0.3;
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { __, _x, sprintf } from '@wordpress/i18n';
*/
import Cell from './cell';

const EMPTY_STYLE = {};

export default function BottomSheetSwitchCell( props ) {
const { value, onValueChange, disabled, ...cellProps } = props;

Expand Down Expand Up @@ -61,6 +63,7 @@ export default function BottomSheetSwitchCell( props ) {
editable={ false }
value={ '' }
disabled={ disabled }
disabledStyle={ EMPTY_STYLE }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the case of the Switch component, we don't want to make it transparent as it already provides a disabled style via the disabled prop. Hence, we pass an empty style to override it.

>
<Switch
value={ value }
Expand Down
3 changes: 3 additions & 0 deletions packages/components/src/range-control/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const RangeControl = memo(
max,
type,
separatorType,
disabled,
...props
} ) => {
if ( type === 'stepper' ) {
Expand All @@ -36,6 +37,7 @@ const RangeControl = memo(
onChange={ onChange }
separatorType={ separatorType }
value={ value }
disabled={ disabled }
/>
);
}
Expand All @@ -61,6 +63,7 @@ const RangeControl = memo(
allowReset={ allowReset }
defaultValue={ initialSliderValue }
separatorType={ separatorType }
disabled={ disabled }
{ ...props }
/>
);
Expand Down
1 change: 1 addition & 0 deletions packages/react-native-editor/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ For each user feature we should also add a importance categorization label to i

## Unreleased
- [**] Tapping on all nested blocks gets focus directly instead of having to tap multiple times depending on the nesting levels. [#50672]
- [*] Add disabled style to `Cell` component [#50665]

## 1.95.0
- [*] Fix crash when trying to convert to regular blocks an undefined/deleted reusable block [#50475]
Expand Down
3 changes: 3 additions & 0 deletions test/native/__mocks__/styleMock.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,4 +184,7 @@ module.exports = {
'components-picker__button-title': {
color: 'white',
},
placeholderColor: {
color: 'gray',
},
};