Skip to content
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

Merged
merged 11 commits into from
Jul 25, 2019
24 changes: 24 additions & 0 deletions docs/designers-developers/developers/data/data-core-editor.md
Original file line number Diff line number Diff line change
Expand Up @@ -932,6 +932,18 @@ _Returns_

- `boolean`: Is locked.

<a name="isPostTitleSelected" href="#isPostTitleSelected">#</a> **isPostTitleSelected**

Returns true if post title is selected.

_Parameters_

- _state_ `Object`: Global application state.

_Returns_

- `boolean`: Whether current post title is selected.

<a name="isPreviewingPost" href="#isPreviewingPost">#</a> **isPreviewingPost**

Returns true if the post is being previewed, or false otherwise.
Expand Down Expand Up @@ -1314,6 +1326,18 @@ _Related_

- toggleBlockMode in core/block-editor store.

<a name="togglePostTitleSelection" href="#togglePostTitleSelection">#</a> **togglePostTitleSelection**

Returns an action object that enables or disables post title selection.

_Parameters_

- _isSelected_ `[boolean]`: Whether post title is currently selected.

_Returns_

- `Object`: Action object.

<a name="toggleSelection" href="#toggleSelection">#</a> **toggleSelection**

_Related_
Expand Down
34 changes: 3 additions & 31 deletions packages/edit-post/src/components/visual-editor/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,33 +15,6 @@ import { ReadableContentView } from '@wordpress/components';
import styles from './style.scss';

class VisualEditor extends Component {
constructor() {
super( ...arguments );

this.onPostTitleSelect = this.onPostTitleSelect.bind( this );
this.onPostTitleUnselect = this.onPostTitleUnselect.bind( this );

this.state = {
isPostTitleSelected: false,
};
}

static getDerivedStateFromProps( props ) {
if ( props.isAnyBlockSelected ) {
return { isPostTitleSelected: false };
}
return null;
}

onPostTitleSelect() {
this.setState( { isPostTitleSelected: true } );
this.props.clearSelectedBlock();
}

onPostTitleUnselect() {
this.setState( { isPostTitleSelected: false } );
}

renderHeader() {
const {
editTitle,
Expand All @@ -55,9 +28,6 @@ class VisualEditor extends Component {
innerRef={ setTitleRef }
title={ title }
onUpdate={ editTitle }
onSelect={ this.onPostTitleSelect }
onUnselect={ this.onPostTitleUnselect }
isSelected={ this.state.isPostTitleSelected }
placeholder={ __( 'Add title' ) }
borderStyle={
this.props.isFullyBordered ?
Expand Down Expand Up @@ -93,7 +63,7 @@ class VisualEditor extends Component {
isFullyBordered={ isFullyBordered }
rootViewHeight={ rootViewHeight }
safeAreaBottomInset={ safeAreaBottomInset }
isPostTitleSelected={ this.state.isPostTitleSelected }
isPostTitleSelected={ this.props.isPostTitleSelected }
Copy link
Contributor

@mchowning mchowning Jul 22, 2019

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 the BlockList as a prop instead of just using the isPostTitleSelected selector directly inside of BlockList itself?

Copy link
Contributor Author

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.

onBlockTypeSelected={ this.onPostTitleUnselect }
/>
</BlockEditorProvider>
Expand All @@ -106,6 +76,7 @@ export default compose( [
const {
getEditorBlocks,
getEditedPostAttribute,
isPostTitleSelected,
} = select( 'core/editor' );

const { getSelectedBlockClientId } = select( 'core/block-editor' );
Expand All @@ -114,6 +85,7 @@ export default compose( [
blocks: getEditorBlocks(),
title: getEditedPostAttribute( 'title' ),
isAnyBlockSelected: !! getSelectedBlockClientId(),
isPostTitleSelected: isPostTitleSelected(),
};
} ),
withDispatch( ( dispatch ) => {
Expand Down
35 changes: 14 additions & 21 deletions packages/editor/src/components/post-title/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,27 +32,12 @@ class PostTitle extends Component {
super( ...arguments );

this.onChange = this.onChange.bind( this );
this.onSelect = this.onSelect.bind( this );
this.onUnselect = this.onUnselect.bind( this );
this.onKeyDown = this.onKeyDown.bind( this );
this.redirectHistory = this.redirectHistory.bind( this );

this.state = {
isSelected: false,
};
}

handleFocusOutside() {
this.onUnselect();
}

onSelect() {
this.setState( { isSelected: true } );
this.props.clearSelectedBlock();
}

onUnselect() {
this.setState( { isSelected: false } );
this.props.onUnselect();
}

onChange( event ) {
Expand Down Expand Up @@ -93,11 +78,11 @@ class PostTitle extends Component {
isCleanNewPost,
isFocusMode,
isPostTypeViewable,
isSelected,
instanceId,
placeholder,
title,
} = this.props;
const { isSelected } = this.state;

// The wp-block className is important for editor styles.
const className = classnames( 'wp-block editor-post-title__block', {
Expand Down Expand Up @@ -126,9 +111,9 @@ class PostTitle extends Component {
value={ title }
onChange={ this.onChange }
placeholder={ decodedPlaceholder || __( 'Add title' ) }
onFocus={ this.onSelect }
onFocus={ this.props.onSelect }
onKeyDown={ this.onKeyDown }
onKeyPress={ this.onUnselect }
onKeyPress={ this.props.onUnselect }
/*
Only autofocus the title when the post is entirely empty.
This should only happen for a new post, which means we
Expand All @@ -149,14 +134,15 @@ class PostTitle extends Component {
}

const applyWithSelect = withSelect( ( select ) => {
const { getEditedPostAttribute, isCleanNewPost } = select( 'core/editor' );
const { getEditedPostAttribute, isCleanNewPost, isPostTitleSelected } = select( 'core/editor' );
const { getSettings } = select( 'core/block-editor' );
const { getPostType } = select( 'core' );
const postType = getPostType( getEditedPostAttribute( 'type' ) );
const { titlePlaceholder, focusMode, hasFixedToolbar } = getSettings();

return {
isCleanNewPost: isCleanNewPost(),
isSelected: isPostTitleSelected(),
title: getEditedPostAttribute( 'title' ),
isPostTypeViewable: get( postType, [ 'viewable' ], false ),
placeholder: titlePlaceholder,
Expand All @@ -174,6 +160,7 @@ const applyWithDispatch = withDispatch( ( dispatch ) => {
editPost,
undo,
redo,
togglePostTitleSelection,
} = dispatch( 'core/editor' );

return {
Expand All @@ -185,7 +172,13 @@ const applyWithDispatch = withDispatch( ( dispatch ) => {
},
onUndo: undo,
onRedo: redo,
clearSelectedBlock,
onSelect() {
togglePostTitleSelection( true );
clearSelectedBlock();
},
onUnselect() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't know es6 allowed us to declare functions this way. Thanks for the TIL! 😎

togglePostTitleSelection( false );
},
};
} );

Expand Down
54 changes: 35 additions & 19 deletions packages/editor/src/components/post-title/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
Copy link
Contributor

Choose a reason for hiding this comment

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

Asking more for my understanding: is it worth considering handling the clearSelectedBlock call inside a reducer for the TOGGLE_POST_TITLE_SELECTION action anytime it is set to true? My thinking is just that we would always want to clear any other selected block anytime we set the post title to be selected, and handling it in a reducer would avoid us needing to remember to always dispatch these two actions together. I do not feel strongly about this, but I would be interested in hearing any reasons for or against this in your mind.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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!

Copy link
Contributor

Choose a reason for hiding this comment

The 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 );
14 changes: 14 additions & 0 deletions packages/editor/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,20 @@ export function updatePostLock( lock ) {
};
}

/**
* 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,
};
}

/**
* Returns an action object used to fetch a single reusable block or all
* reusable blocks from the REST API into the store.
Expand Down
20 changes: 20 additions & 0 deletions packages/editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,25 @@ export function postSavingLock( state = {}, action ) {
return state;
}

/**
* 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 const reusableBlocks = combineReducers( {
data( state = {}, action ) {
switch ( action.type ) {
Expand Down Expand Up @@ -593,6 +612,7 @@ export default optimist( combineReducers( {
preferences,
saving,
postLock,
postTitle,
reusableBlocks,
template,
previewLink,
Expand Down
11 changes: 11 additions & 0 deletions packages/editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1097,6 +1097,17 @@ export function getActivePostLock( state ) {
return state.postLock.activePostLock;
}

/**
* 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;
}

/**
* Returns whether or not the user has the unfiltered_html capability.
*
Expand Down
10 changes: 10 additions & 0 deletions packages/editor/src/store/test/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -861,4 +861,14 @@ describe( 'Editor actions', () => {
} );
} );
} );

describe( 'togglePostTitleSelection', () => {
it( 'should return the TOGGLE_POST_TITLE_SELECTION action', () => {
const result = actions.togglePostTitleSelection( true );
expect( result ).toEqual( {
type: 'TOGGLE_POST_TITLE_SELECTION',
isSelected: true,
} );
} );
} );
} );
Loading