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

Support group block on mobile #17251

Merged
merged 28 commits into from
Sep 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
c3fbe4c
First working version of the MediaText component for native mobile
Tug Jun 26, 2019
2f26450
Fix adding a block to an innerblock list
Tug Jun 27, 2019
fe3cb13
Disable mediaText on production
Tug Jul 9, 2019
d4b30d8
MediaText native: improve editor visuals
Tug Jul 16, 2019
a390c7e
Move BlockToolbar from BlockList to Layout
Tug Jul 17, 2019
2bc590b
Remove BlockEditorProvider from BlockList and add native version of E…
Tug Jul 19, 2019
80e4e94
Update BlockMover for native to hide if locked or if it's the only block
Tug Aug 6, 2019
2ef9cff
Make the vertical align button work, add more styling options for too…
Tug Aug 6, 2019
dff6ef7
Make sure registerCoreBlocks does not break in production
Tug Aug 14, 2019
c62606b
Copy docblock comment from the web version for registerCoreBlocks
Tug Aug 14, 2019
201dd3e
Fix focusing on the media placeholder
Tug Aug 14, 2019
7dc86a4
Only support adding image for now
Tug Aug 14, 2019
f6df91a
Update usage of MediaPlaceholder in MediaContainer
Tug Aug 14, 2019
f91fd8a
Enable autoScroll for just the out most block list
pinarol Aug 22, 2019
cba09ec
Merge branch 'rnmobile/master' into rnmobile/add/media-text
etoledom Aug 28, 2019
4173518
Fix JS Unit tests
etoledom Aug 28, 2019
bb550ac
Roll back to IconButton refactor and fix tests
etoledom Aug 28, 2019
7818564
Fix BlockVerticalAlignmentToolbar buttons style on mobile
etoledom Aug 28, 2019
dbe42d3
Fix thing for web and ensure ariaPressed is always passed down
gziolo Aug 29, 2019
f05d666
Use AriaPressed directly to style SVG on mobile
etoledom Aug 29, 2019
7794404
Update snapshots
etoledom Aug 29, 2019
021f528
Support group block on mobile
lukewalczak Aug 29, 2019
443d401
Merge branch 'rnmobile/add/media-text' into rnmobile/add/group
lukewalczak Aug 29, 2019
4e6e39e
Merge branch 'rnmobile/master' into rnmobile/add/group
lukewalczak Aug 30, 2019
d751e5c
Merge branch 'rnmobile/master' into rnmobile/add/group
lukewalczak Sep 2, 2019
d93f94a
Extend shouldShowInsertionPoint condition to be false when group is s…
lukewalczak Sep 2, 2019
0ad701b
Code refactor
lukewalczak Sep 2, 2019
470c254
Update package-lock
lukewalczak Sep 3, 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
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,18 @@ function BlockListAppender( {
rootClientId,
canInsertDefaultBlock,
isLocked,
renderAppender: CustomAppender,
} ) {
if ( isLocked ) {
return null;
}

if ( CustomAppender ) {
return (
<CustomAppender />
);
}

if ( canInsertDefaultBlock ) {
return (
<DefaultBlockAppender
Expand Down
36 changes: 22 additions & 14 deletions packages/block-editor/src/components/block-list/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { KeyboardAwareFlatList, ReadableContentView, useStyle, withTheme } from
*/
import styles from './style.scss';
import BlockListBlock from './block';
import DefaultBlockAppender from '../default-block-appender';
import BlockListAppender from '../block-list-appender';

const innerToolbarHeight = 44;

Expand Down Expand Up @@ -60,23 +60,21 @@ export class BlockList extends Component {
renderDefaultBlockAppender() {
return (
<ReadableContentView>
<DefaultBlockAppender
<BlockListAppender
rootClientId={ this.props.rootClientId }
containerStyle={ [
styles.blockContainerFocused,
this.blockHolderBorderStyle(),
{ borderColor: 'transparent' },
] }
renderAppender={ this.props.renderAppender }
/>
</ReadableContentView>
);
}

render() {
const { clearSelectedBlock, blockClientIds, isFullyBordered, title, header, withFooter = true, renderAppender } = this.props;

return (
<View
style={ { flex: 1 } }
onAccessibilityEscape={ this.props.clearSelectedBlock }
onAccessibilityEscape={ clearSelectedBlock }
>
<KeyboardAwareFlatList
{ ...( Platform.OS === 'android' ? { removeClippedSubviews: false } : {} ) } // Disable clipping on Android to fix focus losing. See https://github.com/wordpress-mobile/gutenberg-mobile/pull/741#issuecomment-472746541
Expand All @@ -86,16 +84,23 @@ export class BlockList extends Component {
extraScrollHeight={ innerToolbarHeight + 10 }
keyboardShouldPersistTaps="always"
style={ useStyle( styles.list, styles.listDark, this.context ) }
data={ this.props.blockClientIds }
extraData={ [ this.props.isFullyBordered ] }
data={ blockClientIds }
extraData={ [ isFullyBordered ] }
keyExtractor={ identity }
renderItem={ this.renderItem }
shouldPreventAutomaticScroll={ this.shouldFlatListPreventAutomaticScroll }
title={ this.props.title }
ListHeaderComponent={ this.props.header }
title={ title }
ListHeaderComponent={ header }
ListEmptyComponent={ this.renderDefaultBlockAppender }
ListFooterComponent={ this.renderBlockListFooter }
ListFooterComponent={ withFooter && this.renderBlockListFooter }
/>

{ renderAppender && blockClientIds.length > 0 &&
<BlockListAppender
rootClientId={ this.props.rootClientId }
renderAppender={ this.props.renderAppender }
/>
}
</View>
);
}
Expand Down Expand Up @@ -160,12 +165,15 @@ export default compose( [
getSelectedBlockClientId,
getBlockInsertionPoint,
isBlockInsertionPointVisible,
getSelectedBlock,
} = select( 'core/block-editor' );

const selectedBlockClientId = getSelectedBlockClientId();
const blockClientIds = getBlockOrder( rootClientId );
const insertionPoint = getBlockInsertionPoint();
const blockInsertionPointIsVisible = isBlockInsertionPointVisible();
const selectedBlock = getSelectedBlock();
const isSelectedGroup = selectedBlock && selectedBlock.name === 'core/group';
const shouldShowInsertionPoint = ( clientId ) => {
return (
blockInsertionPointIsVisible &&
Expand All @@ -177,7 +185,7 @@ export default compose( [
const selectedBlockIndex = getBlockIndex( selectedBlockClientId );
const shouldShowBlockAtIndex = ( index ) => {
const shouldHideBlockAtIndex = (
blockInsertionPointIsVisible &&
! isSelectedGroup && blockInsertionPointIsVisible &&
// if `index` === `insertionPoint.index`, then block is replaceable
index === insertionPoint.index &&
// only hide selected block
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* External dependencies
*/
import { View } from 'react-native';

/**
* WordPress dependencies
*/
import { Button, Dashicon } from '@wordpress/components';

/**
* Internal dependencies
*/
import Inserter from '../inserter';
import styles from './styles.scss';

function ButtonBlockAppender( { rootClientId } ) {
return (
<>
<Inserter
rootClientId={ rootClientId }
renderToggle={ ( { onToggle, disabled, isOpen } ) => (
<Button
onClick={ onToggle }
aria-expanded={ isOpen }
disabled={ disabled }
fixedRatio={ false }
>
<View style={ [ styles.appender, { flex: 1 } ] }>
<Dashicon
icon="plus-alt"
style={ styles.addBlockButton }
color={ styles.addBlockButton.color }
size={ styles.addBlockButton.size }
/>
</View>
</Button>
) }
isAppender
/>
</>
);
}

/**
* @see https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/components/button-block-appender/README.md
*/
export default ButtonBlockAppender;
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
.appender {
align-items: center;
justify-content: center;
background-color: $gray-light;
padding: 12px;
background-color: $white;
Copy link
Contributor

Choose a reason for hiding this comment

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

@lukewalczak - I noticed that here background-color is defined two times.
It's a small detail but it was giving a CI error on this PR: #17109

It's interesting that after re-running CI it now succeeded, but the issue is real. So it would be nice if we could fixe it.
I also wonder why it was catch once on another branch but it wasn't catch on this PR, and locally it also succeed 🤔

Would you mind taking a look? Thanks!

cc @pinarol

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there an option to remove redundant bg color background-color: $gray-light; within mentioned PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is unclear when that PR will be merged, so it'd be better if we open a new PR to remove the redundant line. We can get the new one merged quickly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, will create a PR for that.

Copy link
Member Author

Choose a reason for hiding this comment

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

border: $border-width solid $light-gray-500;
border-radius: 4px;
}

.addBlockButton {
color: $white;
background-color: $gray;
border-radius: $icon-button-size-small / 2;
overflow: hidden;
size: $icon-button-size-small;
}
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ class InnerBlocks extends Component {
<BlockList
rootClientId={ clientId }
renderAppender={ renderAppender }
withFooter={ false }
/>
) }
</>
Expand Down
51 changes: 51 additions & 0 deletions packages/block-library/src/group/edit.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@

/**
* External dependencies
*/
import { View } from 'react-native';

/**
* WordPress dependencies
*/
import { withSelect } from '@wordpress/data';
import { compose } from '@wordpress/compose';
import {
InnerBlocks,
withColors,
} from '@wordpress/block-editor';
/**
* Internal dependencies
*/
import styles from './editor.scss';

function GroupEdit( {
hasInnerBlocks,
isSelected,
} ) {
if ( ! isSelected && ! hasInnerBlocks ) {
return (
<View style={ styles.groupPlaceholder } />
);
}

return (
<InnerBlocks
renderAppender={ isSelected && InnerBlocks.ButtonBlockAppender }
/>
);
}

export default compose( [
withColors( 'backgroundColor' ),
withSelect( ( select, { clientId } ) => {
const {
getBlock,
} = select( 'core/block-editor' );

const block = getBlock( clientId );

return {
hasInnerBlocks: !! ( block && block.innerBlocks.length ),
};
} ),
] )( GroupEdit );
6 changes: 6 additions & 0 deletions packages/block-library/src/group/editor.native.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
.groupPlaceholder {
padding: 12px;
background-color: $white;
border: $border-width dashed $light-gray-500;
border-radius: 4px;
}
5 changes: 4 additions & 1 deletion packages/block-library/src/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import * as textColumns from './text-columns';
import * as verse from './verse';
import * as video from './video';
import * as tagCloud from './tag-cloud';
import * as group from './group';

export const coreBlocks = [
// Common blocks are grouped at the top to prioritize their display
Expand Down Expand Up @@ -142,7 +143,9 @@ export const registerCoreBlocks = () => {
list,
quote,
// eslint-disable-next-line no-undef
typeof __DEV__ !== 'undefined' && __DEV__ ? mediaText : null,
!! __DEV__ ? mediaText : null,
// eslint-disable-next-line no-undef
!! __DEV__ ? group : null,
].forEach( registerBlock );

setDefaultBlockName( paragraph.name );
Expand Down
5 changes: 4 additions & 1 deletion packages/components/src/button/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ const styles = StyleSheet.create( {
flexDirection: 'row',
justifyContent: 'center',
alignItems: 'center',
},
fixedRatio: {
aspectRatio: 1,
},
buttonActive: {
Expand All @@ -28,7 +30,6 @@ const styles = StyleSheet.create( {
alignItems: 'center',
borderRadius: 6,
borderColor: '#2e4453',
aspectRatio: 1,
backgroundColor: '#2e4453',
},
subscriptInactive: {
Expand All @@ -55,6 +56,7 @@ export default function Button( props ) {
onClick,
disabled,
hint,
fixedRatio = true,
'aria-disabled': ariaDisabled,
'aria-label': ariaLabel,
'aria-pressed': ariaPressed,
Expand All @@ -65,6 +67,7 @@ export default function Button( props ) {

const buttonViewStyle = {
opacity: isDisabled ? 0.2 : 1,
...( fixedRatio && styles.fixedRatio ),
...( ariaPressed ? styles.buttonActive : styles.buttonInactive ),
};

Expand Down
1 change: 1 addition & 0 deletions packages/components/src/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export { default as Spinner } from './spinner';
export { createSlotFill, Slot, Fill, Provider as SlotFillProvider } from './slot-fill';
export { default as BaseControl } from './base-control';
export { default as TextareaControl } from './textarea-control';
export { default as Button } from './button';

// Higher-Order Components
export { default as withConstrainedTabbing } from './higher-order/with-constrained-tabbing';
Expand Down