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] Inner Block Navigate upward through hierarchy #17496

Merged
merged 12 commits into from
Oct 8, 2019
15 changes: 14 additions & 1 deletion packages/block-editor/src/components/block-list/block.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
* WordPress dependencies
*/
import { Component } from '@wordpress/element';
import { ToolbarButton, Toolbar } from '@wordpress/components';
import { withDispatch, withSelect } from '@wordpress/data';
import { compose } from '@wordpress/compose';
import { getBlockType } from '@wordpress/blocks';
Expand All @@ -24,6 +25,7 @@ import BlockEdit from '../block-edit';
import BlockInvalidWarning from './block-invalid-warning';
import BlockMobileToolbar from './block-mobile-toolbar';
import FloatingToolbar from './block-mobile-floating-toolbar';
import NavigateUpSVG from './nav-up-icon';

class BlockListBlock extends Component {
constructor() {
Expand Down Expand Up @@ -124,7 +126,18 @@ class BlockListBlock extends Component {
return (
<>
{ showFloatingToolbar && ( ! isFirstBlock || parentId === '' ) && <FloatingToolbar.Slot /> }
{ showFloatingToolbar && <FloatingToolbar /> }
{ showFloatingToolbar &&
( <FloatingToolbar>
<Toolbar passedStyle={ styles.toolbar }>
<ToolbarButton
Copy link
Contributor

Choose a reason for hiding this comment

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

This one is using a ToolbarButton while #17471 is using a custom component that replicates the functionality of IconButton. Can we make these two consistent?

I think we could keep FloatingToolbar as the generic UI component and something like BlockNavigationToolbar that puts the up button and breadcrumbs in a floating toolbar.

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 agree that we could keep button and breadcrumbs in separate component and make buttons consistent.

I think the best way to achieve that is merge this PR if rest of the code is fine. Then sync breadcrumb branch and do the refactor you mentioned. WDYT ?

title={ __( 'Navigate Up' ) }
onClick={ () => this.props.onSelect( parentId ) }
icon={ NavigateUpSVG }
/>
<View style={ styles.pipe } />
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine for this one, but it should become the toolbar's border when we add the breadcrumbs

</Toolbar>
</FloatingToolbar>
) }
<TouchableWithoutFeedback
onPress={ this.onFocus }
accessible={ ! isSelected }
Expand Down
17 changes: 17 additions & 0 deletions packages/block-editor/src/components/block-list/block.native.scss
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,20 @@
.blockText {
min-height: 50px;
}

.toolbar {
border-left-width: 0;
margin-right: auto;
padding-left: 2px;
height: 44px;
}

.pipe {
margin-top: auto;
margin-bottom: auto;
margin-left: 2px;
height: 28px;
width: 1px;
background-color: #e9eff3;
opacity: 0.4;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/**
* WordPress dependencies
*/
import { SVG, Path } from '@wordpress/components';

const NavigateUp = <SVG width="24" height="24" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"><Path fill="none" d="M0 0h24v24H0V0z" /><Path fill="white" fillRule="evenodd" clipRule="evenodd" d="M17,11 z L15.58,12.42 L12,8.83 L12,18 L22,18 L22,20 L10,20 L10,8.83 L6.42,12.42 L5,11 L11,5 L17,11" /></SVG>;

export default NavigateUp;