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

Editor: Refactor Post Author component #33695

Merged
merged 9 commits into from
Jul 27, 2021
9 changes: 5 additions & 4 deletions packages/editor/src/components/post-author/check.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@ import { store as coreStore } from '@wordpress/core-data';
*/
import PostTypeSupportCheck from '../post-type-support-check';
import { store as editorStore } from '../../store';
import { AUTHORS_QUERY } from './constants';

export function PostAuthorCheck( {
hasAssignAuthorAction,
authors,
hasAuthors,
children,
} ) {
if ( ! hasAssignAuthorAction || ! authors || 1 >= authors.length ) {
if ( ! hasAssignAuthorAction || ! hasAuthors ) {
return null;
}

Expand All @@ -35,14 +36,14 @@ export function PostAuthorCheck( {
export default compose( [
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
withSelect( ( select ) => {
const post = select( editorStore ).getCurrentPost();
const authors = select( coreStore ).getUsers( AUTHORS_QUERY );
return {
hasAssignAuthorAction: get(
post,
[ '_links', 'wp:action-assign-author' ],
false
),
postType: select( editorStore ).getCurrentPostType(),
Copy link
Member Author

Choose a reason for hiding this comment

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

We weren't using this prop in PostAuthorCheck.

authors: select( coreStore ).getAuthors(),
hasAuthors: authors?.length >= 1,
};
} ),
withInstanceId,
Expand Down
41 changes: 20 additions & 21 deletions packages/editor/src/components/post-author/combobox.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,36 +6,40 @@ import { debounce } from 'lodash';
/**
* WordPress dependencies
*/
import { useState, useMemo, useEffect } from '@wordpress/element';
import { useState, useMemo } from '@wordpress/element';
import { useSelect, useDispatch } from '@wordpress/data';
import { __ } from '@wordpress/i18n';
import { ComboboxControl } from '@wordpress/components';
import { decodeEntities } from '@wordpress/html-entities';
import { store as coreStore } from '@wordpress/core-data';

/**
* Internal dependencies
*/
import { store as editorStore } from '../../store';
import { AUTHORS_QUERY } from './constants';

function PostAuthorCombobox() {
const [ fieldValue, setFieldValue ] = useState();

const { authorId, isLoading, authors, postAuthor } = useSelect(
( select ) => {
const { __unstableGetAuthor, getAuthors, isResolving } = select(
coreStore
);
const { getUser, getUsers, isResolving } = select( coreStore );
const { getEditedPostAttribute } = select( editorStore );
const author = __unstableGetAuthor(
getEditedPostAttribute( 'author' )
);
const query =
! fieldValue || '' === fieldValue ? {} : { search: fieldValue };
const author = getUser( getEditedPostAttribute( 'author' ), {
context: 'view',
} );
const query = { ...AUTHORS_QUERY };

if ( fieldValue ) {
query.search = fieldValue;
}

return {
authorId: getEditedPostAttribute( 'author' ),
postAuthor: author,
authors: getAuthors( query ),
isLoading: isResolving( 'core', 'getAuthors', [ query ] ),
authors: getUsers( query ),
isLoading: isResolving( 'core', 'getUsers', [ query ] ),
};
},
[ fieldValue ]
Expand All @@ -46,7 +50,7 @@ function PostAuthorCombobox() {
const fetchedAuthors = ( authors ?? [] ).map( ( author ) => {
return {
value: author.id,
label: author.name,
label: decodeEntities( author.name ),
Copy link
Member Author

Choose a reason for hiding this comment

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

Added missing entity decoding. We were already doing this for select control.

};
} );

Expand All @@ -57,22 +61,17 @@ function PostAuthorCombobox() {

if ( foundAuthor < 0 && postAuthor ) {
return [
{ value: postAuthor.id, label: postAuthor.name },
{
value: postAuthor.id,
label: decodeEntities( postAuthor.name ),
},
...fetchedAuthors,
];
}

return fetchedAuthors;
}, [ authors, postAuthor ] );

// Initializes the post author properly
// Also ensures external changes are reflected.
useEffect( () => {
if ( postAuthor ) {
setFieldValue( postAuthor.name );
}
}, [ postAuthor ] );

Comment on lines -68 to -75
Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't track down why this effect was added precisely, but I don't think it's needed anymore.

It also initializes an unnecessary search query on the mount.

/**
* Handle author selection.
*
Expand Down
6 changes: 6 additions & 0 deletions packages/editor/src/components/post-author/constants.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export const AUTHORS_QUERY = {
who: 'authors',
per_page: 50,
Copy link
Member Author

@Mamaduka Mamaduka Jul 27, 2021

Choose a reason for hiding this comment

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

I changed the "per_page" argument from 100 to 50. However, I don't think this number makes a big difference when used with ComboboxControl. It just needs to be higher than 25 and reasonable :)

Happy to revert it if needed.

_fields: 'id,name',
context: 'view',
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
};
5 changes: 3 additions & 2 deletions packages/editor/src/components/post-author/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@ import { store as coreStore } from '@wordpress/core-data';
*/
import PostAuthorCombobox from './combobox';
import PostAuthorSelect from './select';
import { AUTHORS_QUERY } from './constants';

const minimumUsersForCombobox = 25;

function PostAuthor() {
const showCombobox = useSelect( ( select ) => {
// Not using `getUsers()` because it requires `list_users` capability.
const authors = select( coreStore ).getAuthors();
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
const authors = select( coreStore ).getUsers( AUTHORS_QUERY );

return authors?.length >= minimumUsersForCombobox;
}, [] );

Expand Down
4 changes: 3 additions & 1 deletion packages/editor/src/components/post-author/select.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ import { store as coreStore } from '@wordpress/core-data';
* Internal dependencies
*/
import { store as editorStore } from '../../store';
import { AUTHORS_QUERY } from './constants';

function PostAuthorSelect() {
const { editPost } = useDispatch( editorStore );
const { postAuthor, authors } = useSelect( ( select ) => {
const authorsFromAPI = select( coreStore ).getAuthors();
const authorsFromAPI = select( coreStore ).getUsers( AUTHORS_QUERY );

return {
postAuthor: select( editorStore ).getEditedPostAttribute(
'author'
Expand Down
46 changes: 7 additions & 39 deletions packages/editor/src/components/post-author/test/check.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,45 +9,10 @@ import { shallow } from 'enzyme';
import { PostAuthorCheck } from '../check';

describe( 'PostAuthorCheck', () => {
const users = {
data: [
{
id: 1,
name: 'admin',
capabilities: {
level_1: true,
},
},
{
id: 2,
name: 'subscriber',
capabilities: {
level_0: true,
},
},
{
id: 3,
name: 'andrew',
capabilities: {
level_1: true,
},
},
],
};

it( 'should not render anything if users unknown', () => {
const wrapper = shallow(
<PostAuthorCheck authors={ [] } hasAssignAuthorAction={ true }>
authors
</PostAuthorCheck>
);
expect( wrapper.type() ).toBe( null );
} );

it( 'should not render anything if single user', () => {
it( 'should not render anything if has no authors', () => {
const wrapper = shallow(
<PostAuthorCheck
authors={ users.data.slice( 0, 1 ) }
hasAuthors={ false }
hasAssignAuthorAction={ true }
>
authors
Expand All @@ -58,7 +23,10 @@ describe( 'PostAuthorCheck', () => {

it( "should not render anything if doesn't have author action", () => {
const wrapper = shallow(
<PostAuthorCheck authors={ users } hasAssignAuthorAction={ false }>
<PostAuthorCheck
hasAuthors={ true }
hasAssignAuthorAction={ false }
>
authors
</PostAuthorCheck>
);
Expand All @@ -67,7 +35,7 @@ describe( 'PostAuthorCheck', () => {

it( 'should render control', () => {
const wrapper = shallow(
<PostAuthorCheck authors={ users } hasAssignAuthorAction={ true }>
<PostAuthorCheck hasAuthors={ true } hasAssignAuthorAction={ true }>
authors
</PostAuthorCheck>
);
Expand Down