Fix/1390 Make "title" a block #2714
Conversation
|
hey @karmatosed @melchoyce @mtias can I pls get some feedback on this? |
There was a problem hiding this comment.
Given the amount of special treatment we're putting toward post title specifically in these changes, I'm not sure treating post title as a block makes much sense.
I could see it being more reasonable if we added proper generic support for mapping between block values and post properties (like #2754) or generic support for locking blocks to specific locations within a post (title always shown first).
| registerBlockType( 'core/post-title', { | ||
| title: __( 'Post Title' ), | ||
|
|
||
| keywords: [ __( 'title' ) ], |
There was a problem hiding this comment.
This would be redundant, given that searching "title" would also match the "Post Title" title property, unless it was localized significantly different between "title" and "Post Title".
| }, | ||
|
|
||
| save( { attributes } ) { | ||
| return <h1> { attributes.content } </h1>; |
There was a problem hiding this comment.
- Even if we treat title as a block, I would expect its value should be saved as the
post_titlefield of a post, not as anh1within content - I don't think we need the space padding within the
<h1>
| * Internal Dependencies | ||
| */ | ||
| import './editor.scss'; | ||
| import { isEditedPostNew, getEditedPostAttribute } from '../../../editor/selectors'; |
There was a problem hiding this comment.
We don't want to be reaching out into the editor top-level directory. This could tie into #2473
| @@ -0,0 +1,74 @@ | |||
| /** | |||
There was a problem hiding this comment.
editor/post-permalink is redundant with this file and should be removed?
|
|
||
| const PostTitleEdit = ( { title, focus, setFocus, onChange } ) => [ | ||
| focus && <PostPermaLink />, | ||
| <Editable |
There was a problem hiding this comment.
I don't think this needs to be editable, just a plain input (we don't need to / cannot support rich text).
| style={ { textAlign: 'left' } } | ||
| formattingControls={ [] } /> ]; | ||
|
|
||
| export default connect( null, dispatch => ( { onChange: title => { |
There was a problem hiding this comment.
This line is a bit difficult to understand given the many levels of function nesting and differing implicit return / explicit arrow function blocks it creates.
| formattingControls={ [] } /> ]; | ||
|
|
||
| export default connect( null, dispatch => ( { onChange: title => { | ||
| dispatch( editPost( { title: title[ 0 ] } ) ); |
There was a problem hiding this comment.
What is the value of title here? If we were to consider PostTitleEdit the "presentational" component, I would think it's onChange callback should be called with the value we'd expect to be using, without any further manipulations like we're doing here.
| @@ -273,4 +276,8 @@ export default { | |||
|
|
|||
| return effects; | |||
| }, | |||
| RESET_BLOCKS( action, store ) { | |||
| const blocks = [ createBlock( 'core/post-title' ) ].concat( action.blocks ); | |||
| store.dispatch( { type: 'ACTUALLY_RESET_BLOCKS', blocks } ); | |||
There was a problem hiding this comment.
If we need to better distinguish between effects triggered by SET_INITIAL_POST and what is considered in resetting blocks, I think we should either come up with better names (avoid "ACTUALLY_" prefix) or a different pattern for manipulating how post title is inserted into the block set. A standalone middleware which modifies action.blocks before dispatch occurs is one option.
anna-harrison
changed the title
|
Closing as new approach in #2948 |
fixes #1390
This change moves the post title to a block.
Tests still need to be fixed however I wanted to check the approach is OK first.
Summary of changes:
isFixedwhen registering a block which wont show theBlockMoverandBlockRightMenuwhen the block is focusedRESET_BLOCKSaction is fired, a place holder block is always put before the other blocks for the post titlegetBlockselectorBlockMoverlabels had to be updatedQuestions:
RESET_BLOCKSaction in an effect to add the title block first. Should I useREQUEST_RESET_BLOCKSor similar or is that for an actual web request?