-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Move the post title selection state to the store and update PostTitle #16704
Changes from 9 commits
630d474
4c7ca6a
e1cc965
e787c58
cc71382
6b38663
1e248f1
243cd5d
5424076
db39db5
b58b35a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,7 @@ import { isEmpty } from 'lodash'; | |
import { Component } from '@wordpress/element'; | ||
import { RichText } from '@wordpress/rich-text'; | ||
import { decodeEntities } from '@wordpress/html-entities'; | ||
import { withDispatch } from '@wordpress/data'; | ||
import { withDispatch, withSelect } from '@wordpress/data'; | ||
import { withFocusOutside } from '@wordpress/components'; | ||
import { withInstanceId, compose } from '@wordpress/compose'; | ||
import { __, sprintf } from '@wordpress/i18n'; | ||
|
@@ -91,27 +91,43 @@ class PostTitle extends Component { | |
} | ||
} | ||
|
||
const applyWithDispatch = withDispatch( ( dispatch ) => { | ||
const { | ||
undo, | ||
redo, | ||
} = dispatch( 'core/editor' ); | ||
export default compose( | ||
withSelect( ( dispatch ) => { | ||
const { | ||
isPostTitleSelected, | ||
} = dispatch( 'core/editor' ); | ||
|
||
const { | ||
insertDefaultBlock, | ||
} = dispatch( 'core/block-editor' ); | ||
return { | ||
isSelected: isPostTitleSelected(), | ||
}; | ||
} ), | ||
withDispatch( ( dispatch ) => { | ||
const { | ||
undo, | ||
redo, | ||
togglePostTitleSelection, | ||
} = dispatch( 'core/editor' ); | ||
|
||
return { | ||
onEnterPress() { | ||
insertDefaultBlock( undefined, undefined, 0 ); | ||
}, | ||
onUndo: undo, | ||
onRedo: redo, | ||
}; | ||
} ); | ||
const { | ||
clearSelectedBlock, | ||
insertDefaultBlock, | ||
} = dispatch( 'core/block-editor' ); | ||
|
||
export default compose( | ||
applyWithDispatch, | ||
return { | ||
onEnterPress() { | ||
insertDefaultBlock( undefined, undefined, 0 ); | ||
}, | ||
onUndo: undo, | ||
onRedo: redo, | ||
onSelect() { | ||
togglePostTitleSelection( true ); | ||
clearSelectedBlock(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Asking more for my understanding: is it worth considering handling the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A reducer should only ever update the state so I don't think it would be appropriate. An action could be calling another action but I think it's more explicit to have it here, especially since it's a different store! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point about the reducer just updating the store, and I totally overlooked the different store. Thanks! 👍 |
||
}, | ||
onUnselect() { | ||
togglePostTitleSelection( false ); | ||
}, | ||
}; | ||
} ), | ||
withInstanceId, | ||
withFocusOutside | ||
)( PostTitle ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
|
||
export * from './actions.js'; | ||
|
||
/** | ||
* Returns an action object that enables or disables post title selection. | ||
* | ||
* @param {boolean} [isSelected=true] Whether post title is currently selected. | ||
|
||
* @return {Object} Action object. | ||
*/ | ||
export function togglePostTitleSelection( isSelected = true ) { | ||
return { | ||
type: 'TOGGLE_POST_TITLE_SELECTION', | ||
isSelected, | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import optimist from 'redux-optimist'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { combineReducers } from '@wordpress/data'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { | ||
editor, | ||
initialEdits, | ||
currentPost, | ||
preferences, | ||
saving, | ||
postLock, | ||
reusableBlocks, | ||
template, | ||
previewLink, | ||
postSavingLock, | ||
isReady, | ||
editorSettings, | ||
} from './reducer.js'; | ||
|
||
export * from './reducer.js'; | ||
|
||
/** | ||
* Reducer returning the post title state. | ||
* | ||
* @param {PostTitleState} state Current state. | ||
* @param {Object} action Dispatched action. | ||
* | ||
* @return {Object} Updated state. | ||
*/ | ||
export const postTitle = combineReducers( { | ||
isSelected( state = false, action ) { | ||
switch ( action.type ) { | ||
case 'TOGGLE_POST_TITLE_SELECTION': | ||
return action.isSelected; | ||
} | ||
|
||
return state; | ||
}, | ||
} ); | ||
|
||
export default optimist( combineReducers( { | ||
editor, | ||
initialEdits, | ||
currentPost, | ||
preferences, | ||
saving, | ||
postLock, | ||
reusableBlocks, | ||
template, | ||
previewLink, | ||
postSavingLock, | ||
isReady, | ||
editorSettings, | ||
postTitle, | ||
} ) ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
|
||
export * from './selectors.js'; | ||
|
||
/** | ||
* Returns true if post title is selected. | ||
* | ||
* @param {Object} state Global application state. | ||
* | ||
* @return {boolean} Whether current post title is selected. | ||
*/ | ||
export function isPostTitleSelected( state ) { | ||
return state.postTitle.isSelected; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { togglePostTitleSelection } from '../actions'; | ||
|
||
describe( 'Editor actions', () => { | ||
describe( 'togglePostTitleSelection', () => { | ||
it( 'should return the TOGGLE_POST_TITLE_SELECTION action', () => { | ||
const result = togglePostTitleSelection( true ); | ||
expect( result ).toEqual( { | ||
type: 'TOGGLE_POST_TITLE_SELECTION', | ||
isSelected: true, | ||
} ); | ||
} ); | ||
} ); | ||
} ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/** | ||
* Internal dependencies | ||
*/ | ||
import { | ||
postTitle, | ||
} from '../reducer'; | ||
|
||
describe( 'state native', () => { | ||
describe( 'postTitle', () => { | ||
describe( 'isSelected()', () => { | ||
it( 'should not be selected by default', () => { | ||
expect( postTitle( undefined, {} ).isSelected ).toBe( false ); | ||
} ); | ||
|
||
it( 'should return false if not selecting the post title', () => { | ||
const action = { | ||
type: 'TOGGLE_POST_TITLE_SELECTION', | ||
isSelected: false, | ||
}; | ||
|
||
expect( postTitle( { isSelected: true }, action ).isSelected ).toBe( false ); | ||
} ); | ||
|
||
it( 'should return true if selecting the post title', () => { | ||
const action = { | ||
type: 'TOGGLE_POST_TITLE_SELECTION', | ||
isSelected: true, | ||
}; | ||
|
||
expect( postTitle( { isSelected: false }, action ).isSelected ).toBe( true ); | ||
} ); | ||
} ); | ||
} ); | ||
} ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
/** | ||
* Internal dependencies | ||
*/ | ||
import { isPostTitleSelected } from '../selectors'; | ||
|
||
describe( 'selectors native', () => { | ||
describe( 'isPostTitleSelected', () => { | ||
it( 'should return true if the post title is selected', () => { | ||
const state = { | ||
postTitle: { | ||
isSelected: true, | ||
}, | ||
}; | ||
|
||
expect( isPostTitleSelected( state ) ).toBe( true ); | ||
} ); | ||
|
||
it( 'should return false if the post title is not selected', () => { | ||
const state = { | ||
postTitle: { | ||
isSelected: false, | ||
}, | ||
}; | ||
|
||
expect( isPostTitleSelected( state ) ).toBe( false ); | ||
} ); | ||
} ); | ||
} ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, is there a reason to use the
isPostTitleSelected
selector here in visual-editor and pass the resulting value down to theBlockList
as a prop instead of just using theisPostTitleSelected
selector directly inside ofBlockList
itself?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I simply wanted to minimize the changes to the native part here. #16677 will be responsible of refactoring the VisualEditor and moving the block picker out of BlockList into an
Inserter
component.