Visual Editor: Adding the arrangement toolbar (up/down arrows) #412
Conversation
youknowriad
added
the
Framework
|
This is really solid! I like it! 👍 If you can add |
| return ( | ||
| <div className="editor-arrangement-toolbar"> | ||
| <button | ||
| className={ classnames( 'editor-arrangement-toolbar__control', { 'is-disabled': isFirst } ) } |
There was a problem hiding this comment.
Line is a little long. If not as separate variables, at least some line breaks could help clear it up:
className={ classnames( 'editor-arrangement-toolbar__control', {
'is-disabled': isFirst
} ) }| function BlockArrangement( { onMoveUp, onMoveDown, isFirst, isLast } ) { | ||
| return ( | ||
| <div className="editor-arrangement-toolbar"> | ||
| <button |
There was a problem hiding this comment.
Seems appropriate as rendering with the IconButton component?
| top: 10px; | ||
| left: 0; | ||
|
|
||
| .editor-arrangement-toolbar__control { |
There was a problem hiding this comment.
Probably not! sorry copy/pasted
editor/modes/visual-editor/block.js
Outdated
| isSelected={ isSelected } | ||
| attributes={ block.attributes } | ||
| setAttributes={ setAttributes } /> | ||
| <div className="editor-visual-editor__block-edit"> |
There was a problem hiding this comment.
Hmm would it be possible to effect the padding on the parent .editor-visual-editor__block instead of creating a wrapper here?
There was a problem hiding this comment.
The desired behavior was really hard to achieve. I'll see if this works and update.
| case 'REPLACE_BLOCKS': | ||
| return action.blockNodes.map( ( { uid } ) => uid ); | ||
|
|
||
| case 'MOVE_BLOCK_UP': |
There was a problem hiding this comment.
I wonder if it would be easier to have only a single action, something like INCREMENT_BLOCK_POSITION, where an increment: 1 or increment: -1 property value could be passed to determine the increment effect.
There was a problem hiding this comment.
I have a small preference for the explicit actions but I don't mind if we change this later.
There was a problem hiding this comment.
Also need to consider drag and drop.
| export default connect( | ||
| ( state, ownProps ) => ( { | ||
| isFirst: first( state.blocks.order ) === ownProps.uid, | ||
| isLirst: last( state.blocks.order ) === ownProps.uid |
| @@ -0,0 +1,52 @@ | |||
| /** | |||
There was a problem hiding this comment.
@jasmussen I think we need to take a look at the glossary and figure out how to describe and group things. I've been thinking we could do block-controls and within that have the different pieces: toolbar, placement, switcher, etc.
There was a problem hiding this comment.
I will put on my todo-list to create a labelled blueprint for the UI as a start, that can hopefully grease the discussion for what the individual components and classes should be called.
|
Feedback addressed, this may be good to go? |
| return ( | ||
| <div className="editor-block-mover"> | ||
| <IconButton | ||
| className={ classnames( 'editor-block-mover__control', { 'is-disabled': isFirst } ) } |
There was a problem hiding this comment.
The classes should be block-mover__ only.
There was a problem hiding this comment.
🤔 We should update our coding guidelines https://github.com/WordPress/gutenberg/blob/master/docs/coding-guidelines.md
There was a problem hiding this comment.
Maybe we should keep it this way in this PR and drop the editor prefix from all the components in a specific PR. (To avoid having a mix of conventions)
There was a problem hiding this comment.
Right, this is following the fact I want to move these components out of the editor and make them only about blocks.
|
I'm ok with merging and iterating. |
|
Appears that an empty |
This PR adds the up/down arrows to move blocks. The arrangement toolbar is shown if the block is selected or hovered.
Missing in this PR: Selecting the block being moved, I'm waiting to the simplification of the reducers in #407 to add this.