Editor: Refactor Post Author component #33695
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Mamaduka
commented
Jul 26, 2021
| return { | ||
| hasAssignAuthorAction: get( | ||
| post, | ||
| [ '_links', 'wp:action-assign-author' ], | ||
| false | ||
| ), | ||
| postType: select( editorStore ).getCurrentPostType(), |
There was a problem hiding this comment.
We weren't using this prop in PostAuthorCheck.
|
Size Change: +691 B (0%) Total Size: 1.08 MB
ℹ️ View Unchanged
|
Mamaduka
commented
Jul 27, 2021
Comment on lines
-68
to
-75
| // Initializes the post author properly | ||
| // Also ensures external changes are reflected. | ||
| useEffect( () => { | ||
| if ( postAuthor ) { | ||
| setFieldValue( postAuthor.name ); | ||
| } | ||
| }, [ postAuthor ] ); | ||
|
|
There was a problem hiding this comment.
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.
Mamaduka
commented
Jul 27, 2021
| @@ -0,0 +1,6 @@ | |||
| export const AUTHORS_QUERY = { | |||
| who: 'authors', | |||
| per_page: 50, | |||
There was a problem hiding this comment.
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.
Mamaduka
changed the title
Mamaduka
added
[Package] Editor
Mamaduka
commented
Jul 27, 2021
| @@ -46,7 +50,7 @@ function PostAuthorCombobox() { | |||
| const fetchedAuthors = ( authors ?? [] ).map( ( author ) => { | |||
| return { | |||
| value: author.id, | |||
| label: author.name, | |||
| label: decodeEntities( author.name ), | |||
There was a problem hiding this comment.
Added missing entity decoding. We were already doing this for select control.
|
@youknowriad, this isn't necessarily related to this PR, but I'm just curious. Is it okay to perform gutenberg/packages/editor/src/components/post-author/select.js Lines 23 to 26 in c6ba02b |
youknowriad
reviewed
Jul 27, 2021
There was a problem hiding this comment.
@Mamaduka mapping/filtering inside useSelect/withSelect should definitely be avoided as it will trigger too much rerendering. Favor useMemo outside the useSelect for these situations.
Awesome work on this PR btw
|
Thanks for the review, @youknowriad. |
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Refactors Editor's post author component to use
getUserandgetUsersselectors.The component previously was using temporary
getAuthorsand__unstableGetAuthorto display author selection for non-admin users. Temp selectors are no longer required as now we can specify context when requesting data records.We can now deprecated/remove those temporary selectors.
Query Improvements
id, name). The change reduces transferred data size by roughly 50%.How has this been tested?
Test creating/editing post.
Test with non-admin users. Again, the results should be the same.
Helpful WP CLI Commands
Generate 15 editors:
wp user generate --role=editor --count=15Generate 50 editors:
wp user generate --role=editor --count=50Cleanup:
wp user delete $(wp user list --role=editor --field=ID)Screenshots
Types of changes
Refactoring
Checklist:
*.native.jsfiles for terms that need renaming or removal).