Add a new empty para block on Enter.KEY #159
Conversation
…porary patch to by-pass an Android issue with focus
| // find currently focused block | ||
| let focusedItemIndex = this.findDataSourceIndexForFocusedItem(); | ||
| if ( focusedItemIndex == -1 ) { | ||
| // no focus yet? This is a well knwon bug on Android... |
There was a problem hiding this comment.
let's add a ToDo comment here to fix it instead of leaving the magic number 3 with no other explanation
…g-mobile into try/add-new-empty-block-after-focus-on-enter-key
|
Removed empty lines and added a TODO - Thanks for your review @mzorz ! |
…g-mobile into try/add-new-empty-block-after-focus-on-enter-key Resolved conflicts over # gutenberg
| @@ -139,6 +139,27 @@ export default class BlockManager extends React.Component<PropsType, StateType> | |||
| this.state.dataSource.setDirty(); | |||
| } | |||
|
|
|||
| insertBlocksAfter( blocks: Array<Object> ) { | |||
There was a problem hiding this comment.
I think that since we're passing this function to each of the blocks, we can actually specialize it to accept the block clientId and emit an action to add the blocks after this specific clientId. We won't need to rely on which block is selected at that moment. WDYT?
There was a problem hiding this comment.
Interesting...will try that!
There was a problem hiding this comment.
Awesome! Just to expand a bit, we can make block-manager's insertBlocksAfter accept the clientID but we can still pass a wrapped version down to the block-holder that only needs the blocks parameter.
… to BlockManager together with the blocks list. No need to find the current focused block since we already have the blockID that started the action.
src/block-management/block-holder.js
Outdated
| @@ -49,6 +50,14 @@ export default class BlockHolder extends React.Component<PropsType, StateType> { | |||
| return <View />; | |||
| } | |||
|
|
|||
| _insertBlocksAfter( blocks: Array<Object> ) { | |||
There was a problem hiding this comment.
In https://github.com/wordpress-mobile/gutenberg-mobile/pull/159/files#diff-2d50daf729e068ecebcce113d1ce7f9aR270, we pass the insertBlocksAfter method to all the rendered items so, I don't think we need to check whether the prop exists, right?
Then again, I think we don't even need this _insertBlocksAfter here at all as we can construct a wrapped method with just the blocks parameter right there in https://github.com/wordpress-mobile/gutenberg-mobile/pull/159/files#diff-2d50daf729e068ecebcce113d1ce7f9aR270.
For example we can have this in block-manager.js:
insertBlocksAfter={ ( blocks ) =>
this.insertBlocksAfter.bind( this )( value.item.clientId, blocks )
}
WDYT?
…lockManager instead.
src/block-management/block-holder.js
Outdated
| @@ -19,6 +19,7 @@ type PropsType = BlockType & { | |||
| onChange: ( clientId: string, attributes: mixed ) => void, | |||
| onToolbarButtonPressed: ( button: number, clientId: string ) => void, | |||
| onBlockHolderPressed: ( clientId: string ) => void, | |||
| insertBlocksAfter: ( clientId: string, blocks: Array<Object> ) => void, | |||
There was a problem hiding this comment.
I think we need to remove the clientId parameter from the definition here. Essentially, what we will be passing down to the block-holder is a function with the clientId string already "embedded" in it so, we will just call it by passing the new blocks.
There was a problem hiding this comment.
LGTM @daniloercoli ! Feel free to update to the GB side when that gets merged and merge this one too!
daniloercoli
deleted the
try/add-new-empty-block-after-focus-on-enter-key
branch
This PR updates GB hash, and adds the code to insert a new para block after the current focused when enter.key is pressed.
There are few problems at the moment, not strictly related to this PR, explained below:
this.props.focusBlockAction( newBlock.clientId )inblock-manager.jsis not working fine, the new block is not getting the focus, both if you tap+on the bottom toolbar and both if you press Enter.key in a para/heading block. cc @mzorzGB side PR is available here: WordPress/gutenberg#10553