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 mobile Spacer component #17896

Merged
merged 23 commits into from
Nov 1, 2019
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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: 1 addition & 0 deletions packages/block-library/src/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ export const registerCoreBlocks = () => {
mediaText,
// eslint-disable-next-line no-undef
!! __DEV__ ? group : null,
spacer,
lukewalczak marked this conversation as resolved.
Show resolved Hide resolved
].forEach( registerBlock );

setDefaultBlockName( paragraph.name );
Expand Down
71 changes: 71 additions & 0 deletions packages/block-library/src/spacer/edit.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@

/**
* External dependencies
*/
import { View } from 'react-native';
/**
* WordPress dependencies
*/
import {
PanelBody,
BottomSheet,
} from '@wordpress/components';
import { useState, useEffect } from '@wordpress/element';
import {
InspectorControls,
} from '@wordpress/block-editor';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import styles from './editor.scss';

const minSpacerHeight = 20;
const maxSpacerHeight = 500;

const SpacerEdit = ( { isSelected, attributes, setAttributes } ) => {
const { height } = attributes;
const [ sliderSpacerHeight, setSpacerHeight ] = useState( height );
const [ sliderSpacerMaxHeight, setSpacerMaxHeight ] = useState( height );

// Height defined on the web can be higher than
// `maxSpacerHeight`, so there is a need to `setSpacerMaxHeight`
// after the initial render.
useEffect( () => {
setSpacerMaxHeight( height > maxSpacerHeight ? height * 2 : maxSpacerHeight );
}, [] );

const changeSpacerHeight = ( value ) => {
let spacerHeight = value;
setSpacerHeight( spacerHeight );
if ( spacerHeight < minSpacerHeight ) {
spacerHeight = minSpacerHeight;
} else if ( spacerHeight > sliderSpacerMaxHeight ) {
spacerHeight = sliderSpacerMaxHeight;
}
setAttributes( {
height: spacerHeight,
} );
};

return (
<View style={ [ styles.staticSpacer, isSelected && styles.selectedSpacer, { height } ] }>
<InspectorControls>
<PanelBody title={ __( 'Spacer Settings' ) } >
<BottomSheet.RangeCell
icon={ 'admin-settings' }
label={ __( 'Height in pixels' ) }
value={ sliderSpacerHeight }
minimumValue={ minSpacerHeight }
maximumValue={ sliderSpacerMaxHeight }
separatorType={ 'fullWidth' }
onChangeValue={ ( value ) => changeSpacerHeight( value ) }
/>
</PanelBody>
</InspectorControls>
</View>
);
};

export default SpacerEdit;
10 changes: 10 additions & 0 deletions packages/block-library/src/spacer/editor.native.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
.staticSpacer {
height: 20px;
background-color: transparent;
border: $border-width dashed $light-gray-500;
border-radius: 1px;
}

.selectedSpacer {
border: $border-width solid $blue-wordpress;
}
3 changes: 2 additions & 1 deletion packages/components/src/mobile/bottom-sheet/cell.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class BottomSheetCell extends Component {

render() {
const {
accessible,
accessibilityLabel,
accessibilityHint,
accessibilityRole,
Expand Down Expand Up @@ -160,7 +161,7 @@ class BottomSheetCell extends Component {

return (
<TouchableOpacity
accessible={ ! this.state.isEditingValue }
accessible={ accessible !== undefined ? accessible : ! this.state.isEditingValue }
accessibilityLabel={ getAccessibilityLabel() }
accessibilityRole={ accessibilityRole || 'button' }
accessibilityHint={ isValueEditable ?
Expand Down
40 changes: 37 additions & 3 deletions packages/components/src/mobile/bottom-sheet/range-cell.native.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
/**
* External dependencies
*/
import { Platform } from 'react-native';
import { Platform, AccessibilityInfo, findNodeHandle } from 'react-native';
/**
* WordPress dependencies
*/
import { _x, __, sprintf } from '@wordpress/i18n';
import { useState } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -20,14 +25,38 @@ export default function BottomSheetRangeCell( props ) {
step = 1,
minimumTrackTintColor = '#00669b',
maximumTrackTintColor = Platform.OS === 'ios' ? '#e9eff3' : '#909090',
thumbTintColor = Platform.OS === 'ios' ? '#fff' : '#00669b',
thumbTintColor = Platform.OS === 'android' && '#00669b',
Copy link
Member Author

@lukewalczak lukewalczak Oct 21, 2019

Choose a reason for hiding this comment

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

I've noticed that if thumbTintColor on iOS is set to white color it losing the shadow (probably the shadow color is based on the main color). That's why I've decided to use the custom color

with white color with default color
IMG_0008 IMG_0007

Currently we are using the Slider from react-native, which will be deprecated soon and we'll have to use the Slider from community library. Need to check if the bug doesn't exist in that library.

Copy link
Member Author

@lukewalczak lukewalczak Oct 23, 2019

Choose a reason for hiding this comment

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

I've tested this using slider from a community library and it seems like a bug which affects ios version ~12.

Tested on:

  • device - iPhone X - 12.3.1 - doesn't work properly
  • device - iPhone X - 13.1.3 - works as expected
  • device - iPhone 7 - 12.4 - doesn't work properly
  • device - iPhone 7 - 13.1.3 - works as expected

What's more it's a bug within pure ios, since I've tested it on clear swift project.

...cellProps
} = props;
const [ accessible, onChangeAccessible ] = useState( true );

const onCellPress = () => {
onChangeAccessible( false );
if ( this.sliderRef ) {
const reactTag = findNodeHandle( this.sliderRef );
AccessibilityInfo.setAccessibilityFocus( reactTag );
}
};
Copy link
Member Author

@lukewalczak lukewalczak Oct 23, 2019

Choose a reason for hiding this comment

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

  • What I wanted to achieve here is to make the parent component (Cell) not accessible, since I want to focus Slider and have also TextInput accessible as well.

  • Wdyt about having there only one component when screen reader is enabled?

  • Both platforms are reading the slider value in percents, e.g. 100 px is is read as 17 percents.

Copy link
Member Author

Choose a reason for hiding this comment

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

@etoledom Could you please look at it? I would like to know if something is wrong there. Thanks!


const accessibilityLabel =
sprintf(
/* translators: accessibility text. Inform about current spacer height value. %2$s: Spacer height current value. */
lukewalczak marked this conversation as resolved.
Show resolved Hide resolved
_x( '%1$s. Current value is %2$s px', 'range cell' ),
lukewalczak marked this conversation as resolved.
Show resolved Hide resolved
cellProps.label, value
);

return (
<Cell
editable={ false }
{ ...cellProps }
accessibilityRole={ 'none' }
editable={ true }
accessible={ accessible }
onPress={ onCellPress }
accessibilityLabel={ accessibilityLabel }
accessibilityHint={
/* translators: accessibility text (hint for focusing a slider) */
__( 'Double tap to change the value using slider' )
}
>
<Slider
value={ value }
Expand All @@ -40,6 +69,11 @@ export default function BottomSheetRangeCell( props ) {
maximumTrackTintColor={ maximumTrackTintColor }
thumbTintColor={ thumbTintColor }
onChangeValue={ onChangeValue }
ref={ ( slider ) => {
this.sliderRef = slider;
} }
accessibilityRole={ 'adjustable' }
accessible={ true }
/>
</Cell>
);
Expand Down
10 changes: 8 additions & 2 deletions packages/components/src/mobile/slider/index.native.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { Slider as RNSlider, TextInput, View } from 'react-native';
import { Slider as RNSlider, TextInput, View, Platform } from 'react-native';

/**
* WordPress dependencies
Expand Down Expand Up @@ -33,6 +33,10 @@ class Slider extends Component {
}
}

componentWillUnmount() {
this.handleToggleFocus();
}

handleToggleFocus( validateInput = true ) {
const newState = { hasFocus: ! this.state.hasFocus };

Expand Down Expand Up @@ -83,12 +87,13 @@ class Slider extends Component {
minimumTrackTintColor,
maximumTrackTintColor,
thumbTintColor,
...sliderProps
} = this.props;

const { hasFocus, sliderValue } = this.state;

return (
<View style={ styles.sliderContainer }>
<View style={ styles.sliderContainer } accessible={ Platform.OS === 'android' }>
<RNSlider
value={ this.validateInput( sliderValue ) }
disabled={ disabled }
Expand All @@ -101,6 +106,7 @@ class Slider extends Component {
thumbTintColor={ thumbTintColor }
onValueChange={ this.handleChange }
onSlidingComplete={ this.handleValueSave }
{ ...sliderProps }
/>
<TextInput
style={ [ styles.sliderTextInput, hasFocus ? styles.isSelected : {} ] }
Expand Down