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

Mobile bottom sheet component #13612

Merged
merged 18 commits into from
Feb 1, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
rnmobile: exporting component BottomSheet.Button to be used as bottom…
…-sheet header buttons
  • Loading branch information
etoledom committed Jan 31, 2019
commit d335a84f58a530034e0568842affb543c15fcac2
10 changes: 8 additions & 2 deletions packages/block-library/src/image/edit.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
import React from 'react';
import { View, Image, TextInput, Text, TouchableOpacity } from 'react-native';
import { View, Image, TextInput, Text, TouchableOpacity, Switch } from 'react-native';
import {
subscribeMediaUpload,
requestMediaPickFromMediaLibrary,
Expand Down Expand Up @@ -168,7 +168,13 @@ export default class ImageEdit extends React.Component {
isVisible={ this.state.showSettings }
title={ __( 'Image Settings' ) }
onClose={ onImageSettingsClose }
rightButtonConfig={ { text: __( 'Done' ), color: '#0087be', onPress: onImageSettingsClose } }
rightButton={
<BottomSheet.Button
text={ __( 'Done' ) }
color={ '#0087be' }
onPress={ onImageSettingsClose }
/>
}
>
<TouchableOpacity style={ inspectorStyles.bottomSheetCell } onPress={ () => { } }>
Copy link
Contributor

Choose a reason for hiding this comment

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

This could probably be part of BottomSheet which would have its default style and could accept a prop containerStyle or style to customize it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean, similar to the Button above?
To have a Cell kind-of component exposed from BottomSheet?

Copy link
Contributor

@Tug Tug Jan 31, 2019

Choose a reason for hiding this comment

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

I meant more like update the BottomSheet render method to something like:

render() {
	const { isVisible, leftButtonConfig, rightButtonConfig, containerStyle, onContainerPress } = this.props;
 	return (
		<Modal
			....
			<View style={ styles.separator } />
			<TouchableOpacity style={ { ... defaultContainerStyle, ...containerStyle } } onPress={ onContainerPress || noop }>
				{ this.props.children }
			</TouchableOpacity>
			<View style={ { flexGrow: 1 } }></View>
			....
		</Modal>
	);
}

Copy link
Contributor Author

@etoledom etoledom Jan 31, 2019

Choose a reason for hiding this comment

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

Right, the problem there is that the buttons or cells are defined by the caller side. And each of those cells will have a TouchableOpacity component. Later on we will add more cells to it:

<BottomSheet
	isVisible={ this.state.showSettings }
	title={ __( 'Image Settings' ) }
	onClose={ onImageSettingsClose }
	rightButtonConfig={ { text: __( 'Done' ), color: '#0087be', onPress: onImageSettingsClose } }
>
	<TouchableOpacity style={ inspectorStyles.bottomSheetCell } onPress={ () => { } }>
		<Text style={ inspectorStyles.bottomSheetCellLabel }>{ __( 'Alt Text' ) }</Text>
		<Text style={ inspectorStyles.bottomSheetCellValue }>{ __( 'None' ) }</Text>
	</TouchableOpacity>
	<TouchableOpacity style={ inspectorStyles.bottomSheetCell } onPress={ () => { } }>
		<Text style={ inspectorStyles.bottomSheetCellLabel }>{ __( 'Size' ) }</Text>
		<Text style={ inspectorStyles.bottomSheetCellValue }>{ __( 'Full Size' ) }</Text>
	</TouchableOpacity>
</BottomSheet>

That's why I thought that maybe extracting the whole cell as a small component would be good:

<TouchableOpacity style={ inspectorStyles.bottomSheetCell } onPress={ () => { } }>
	<Text style={ inspectorStyles.bottomSheetCellLabel }>{ __( 'Size' ) }</Text>
	<Text style={ inspectorStyles.bottomSheetCellValue }>{ __( 'Full Size' ) }</Text>
</TouchableOpacity>

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see, so the TouchableOpacity is for a "cell" I thought it was for the whole content. Yeah makes sense. I'm not sure the concept of a cell needs to be part or BottomSheet but it looks like a nice addition at this point 👍

<Text style={ inspectorStyles.bottomSheetCellLabel }>{ __( 'Alt Text' ) }</Text>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
/**
* External dependencies
*/
import { TouchableOpacity, View } from 'react-native';
import { TouchableOpacity, View, Text } from 'react-native';

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

export default function Button( props ) {
const {
children,
onPress,
disabled,
text,
color,
} = props;

return (
Expand All @@ -17,7 +23,9 @@ export default function Button( props ) {
disabled={ disabled }
>
<View style={ { flexDirection: 'row', justifyContent: 'center' } }>
{ children }
<Text style={ { ...styles.buttonText, color: color } }>
{ text }
</Text>
</View>
</TouchableOpacity>
);
Expand Down
17 changes: 4 additions & 13 deletions packages/editor/src/components/mobile/bottom-sheet/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,6 @@ class BottomSheet extends Component {
SafeArea.removeEventListener( 'safeAreaInsetsForRootViewDidChange', this.onSafeAreaInsetsUpdate );
}

headerButton( config ) {
return (
<Button onPress={ config.onPress }>
<Text style={ { ...styles.buttonText, color: config.color } }>
{ config.text }
</Text>
</Button>
);
}

onSafeAreaInsetsUpdate( result ) {
const { safeAreaInsets } = result;
if ( this.state.safeAreaBottomInset !== safeAreaInsets.bottom ) {
Expand All @@ -53,7 +43,7 @@ class BottomSheet extends Component {
}

render() {
const { isVisible, leftButtonConfig, rightButtonConfig } = this.props;
const { isVisible, leftButton, rightButton } = this.props;

return (
<Modal
Expand All @@ -71,15 +61,15 @@ class BottomSheet extends Component {
<View style={ styles.dragIndicator } />
<View style={ styles.head }>
<View style={ { flex: 1 } }>
{ leftButtonConfig && this.headerButton( leftButtonConfig ) }
{ leftButton }
</View>
<View style={ styles.titleContainer }>
<Text style={ styles.title }>
{ this.props.title }
</Text>
</View>
<View style={ { flex: 1 } }>
{ rightButtonConfig && this.headerButton( rightButtonConfig ) }
{ rightButton }
</View>
</View>

Expand All @@ -93,4 +83,5 @@ class BottomSheet extends Component {
}
}

BottomSheet.Button = Button;
export default BottomSheet;