Skip to content

Inserter - show Picker with registered blocks to choose from #112

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.

Sign up for GitHub

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

Merged
koke merged 4 commits into from
Aug 16, 2018
Merged

Inserter - show Picker with registered blocks to choose from #112

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b7c557c
inserter: added RN Picker component to choose from registered block t…
mzorz Aug 14, 2018
39f6500
removed unneeded creation of intermediate block object
mzorz Aug 14, 2018
656819f
cascading merge from feature branch - updated guteberg hash
mzorz Aug 15, 2018
017a8cc
Merge branch 'feature/inserter' into try/inserter-take1-state-props-s…
mzorz Aug 15, 2018
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
65 changes: 58 additions & 7 deletions src/block-management/block-manager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@
*/

import React from 'react';
import { Platform, Switch, Text, View, FlatList } from 'react-native';
import { Platform, Switch, Text, View, FlatList, Picker } from 'react-native';
import RecyclerViewList, { DataSource } from 'react-native-recyclerview-list';
import BlockHolder from './block-holder';
import { ToolbarButton } from './constants';
import type { BlockType } from '../store/';
import styles from './block-manager.scss';
// Gutenberg imports
import { getBlockType, serialize, createBlock } from '@wordpress/blocks';
import { getBlockType, getBlockTypes, serialize, createBlock } from '@wordpress/blocks';

export type BlockListType = {
onChange: ( clientId: string, attributes: mixed ) => void,
Expand All @@ -29,16 +29,21 @@ type PropsType = BlockListType;
type StateType = {
dataSource: DataSource,
showHtml: boolean,
blockTypePickerVisible: boolean,
selectedBlockType: string,
};

export default class BlockManager extends React.Component<PropsType, StateType> {
_recycler = null;
availableBlockTypes = getBlockTypes();

constructor( props: PropsType ) {
super( props );
this.state = {
dataSource: new DataSource( this.props.blocks, ( item: BlockType ) => item.clientId ),
showHtml: false,
blockTypePickerVisible: false,
selectedBlockType: 'core/paragraph', // just any valid type to start from
};
}

Expand All @@ -56,6 +61,41 @@ export default class BlockManager extends React.Component<PropsType, StateType>
return -1;
}

findDataSourceIndexForFocusedItem() {
for ( let i = 0; i < this.state.dataSource.size(); ++i ) {
const block = this.state.dataSource.get( i );
if ( block.focused === true ) {
return i;
}
}
return -1;
}

// TODO: in the near future this will likely be changed to onShowBlockTypePicker and bound to this.props
// once we move the action to the toolbar
showBlockTypePicker() {
this.setState( { ...this.state, blockTypePickerVisible: true } );
}

onBlockTypeSelected( itemValue: string ) {
this.setState( { ...this.state, selectedBlockType: itemValue, blockTypePickerVisible: false } );

// find currently focused block
const focusedItemIndex = this.findDataSourceIndexForFocusedItem();
const clientIdFocused = this.state.dataSource.get( focusedItemIndex ).clientId;

// create an empty block of the selected type
const newBlock = createBlock( itemValue, { content: 'new test text for a ' + itemValue + ' block' } );
newBlock.focused = false;

// set it into the datasource, and use the same object instance to send it to props/redux
this.state.dataSource.splice( focusedItemIndex + 1, 0, newBlock );
Copy link
Member

@koke koke Aug 16, 2018

Choose a reason for hiding this comment

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

Aren't you supposed to modify state only by calling setState? I still have a lot to learn about React and I'm not sure which side effects this might have.

Copy link
Contributor Author

@mzorz mzorz Aug 16, 2018

Choose a reason for hiding this comment

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

I know, right? 😉 That's exactly my understanding from my learnup that ended in the elaborate description of #95. But, one of the troublesome things that appear here is that the list will redraw itself on each state change as per React's mandate, which is something we still have trouble digesting (feels super counterintuitive after having learnt to deal with recycled views and making good use of list updates for years). Also this kills the native animations when moving blocks, which was something that was tested early on when picking the List component, as ultimately we want to have a best UX on Gutenberg when moving blocks around.

Long story short, we are keeping the same state on both sides (RecyclerView and redux) to make sure we can still use the fine grained RecyclerView API to move items around and get native animations to work, without incurring in heavy lifting. More info in #108.

Copy link
Member

@koke koke Aug 16, 2018

Choose a reason for hiding this comment

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

Got it. I'm going to go with 👍 for now since I don't want to be blocking on this, but it seems to me as if the recyclerlist library wasn't implemented in a way that's consistent with React's declarative style. If I understood the code correctly, the list isn't implementing shouldComponentUpdate, and the README seems to imply that you should update the contents by manipulating the data source directly.

What I would expect is that you'd call setState and the list would deal with calculating what changed from the previous one and what needs to be inserted/deleted/updated. But this is what we have for now 🤷‍♂️

Copy link
Contributor Author

@mzorz mzorz Aug 16, 2018

Choose a reason for hiding this comment

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

On the same boat here 😄. We could set some time apart to further investigate and see if using a FlatList and non-native animations makes the cut for both platforms and eventually go with it, making good use of the React pattern 👍

this.props.createBlockAction( newBlock.clientId, newBlock, clientIdFocused );

// now set the focus
this.props.focusBlockAction( newBlock.clientId );
}

onToolbarButtonPressed( button: number, clientId: string ) {
const dataSourceBlockIndex = this.getDataSourceIndexFromUid( clientId );
switch ( button ) {
Expand All @@ -72,11 +112,7 @@ export default class BlockManager extends React.Component<PropsType, StateType>
this.props.deleteBlockAction( clientId );
break;
case ToolbarButton.PLUS:
// TODO: block type picker here instead of hardcoding a core/code block
const newBlock = createBlock( 'core/paragraph', { content: 'new test text for a core/paragraph block' } );
const newBlockWithFocusedState = { ...newBlock, focused: false };
this.state.dataSource.splice( dataSourceBlockIndex + 1, 0, newBlockWithFocusedState );
this.props.createBlockAction( newBlockWithFocusedState.clientId, newBlockWithFocusedState, clientId );
this.showBlockTypePicker();
break;
case ToolbarButton.SETTINGS:
// TODO: implement settings
Expand Down Expand Up @@ -146,6 +182,20 @@ export default class BlockManager extends React.Component<PropsType, StateType>
);
}

const blockTypePicker = (
<View>
<Picker
selectedValue={ this.state.selectedBlockType }
onValueChange={ ( itemValue ) => {
this.onBlockTypeSelected( itemValue );
} } >
{ this.availableBlockTypes.map( ( item, index ) => {
return ( <Picker.Item label={ item.title } value={ item.name } key={ index + 1 } /> );
} ) }
</Picker>
</View>
);

return (
<View style={ styles.container }>
<View style={ { height: 30 } } />
Expand All @@ -160,6 +210,7 @@ export default class BlockManager extends React.Component<PropsType, StateType>
</View>
{ this.state.showHtml && <Text style={ styles.htmlView }>{ this.serializeToHtml() }</Text> }
{ ! this.state.showHtml && list }
{ this.state.blockTypePickerVisible && blockTypePicker }
</View>
);
}
Expand Down