Editor: Refactor the 'PostVisibility' component #69451
Merged
Conversation
This file contains hidden or 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
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: -926 B (-0.05%) Total Size: 1.84 MB
ℹ️ View Unchanged
|
|
I understand it's a separate issue but the help text font size set to 12 pixels here is too small. There shouldn't be any font size smaller than 13 pixels in Gutenberg (and I would increase the minimum to at least 14). Since this refactored panel will now use the 12 pixels font size help text, it will be less readable than before and not an improvement. It's not a blocker for this PR as long as there's a follow up to increase the |
|
Thanks for noting that, @afercia! I think that's definitely a separate issue. The help text is universally 12px. The current refactoring just makes it consistent. Let's discuss the help text size change separately with the @WordPress/gutenberg-components team. |
|
Note: this PR brings in two more improvements that are also accessibility improvements I'd like to mention for history: 1 2 |
afercia
reviewed
Mar 6, 2025
| select( editorStore ).getEditedPostVisibility() | ||
| const visibility = useSelect( | ||
| ( select ) => select( editorStore ).getEditedPostVisibility(), | ||
| [] |
There was a problem hiding this comment.
Just a question: why adding the dependencies empty array?
There was a problem hiding this comment.
This is just a best practice. useSelect is similar to useEffect, dependencies need to be provided and empty array means same thing. In this case, only re-select values when component mounts or store updates.
afercia
reviewed
Mar 6, 2025
| return visibilityOptions[ visibility ]?.label; | ||
|
|
||
| return VISIBILITY_OPTIONS.find( ( option ) => option.value === visibility ) | ||
| ?.label; |
There was a problem hiding this comment.
Just a question: is find() more expensive than the previous implementation that was simpler because the options object had keys?
There was a problem hiding this comment.
Had to change visibilityOptions from an object into an array for RadioControl. There should be no noticeable difference here.
afercia
approved these changes
Mar 6, 2025
There was a problem hiding this comment.
Overall this looks to me a great simplification with a good amount of removed code.
Most importantly it removes the no longer needed confirm dialog and the need to ave the status.
Also brings in accessibility improvements and consistency with alignments and spacings.
I only left two questions, mainly for learning opportunity.
|
For history: Screenshot to illustrate how alignments and spacings are now consistent between the Post Summary > Visibility & Status and the pre-publish Visibility panel: |
|
Thanks for the review and the interesting questions, @afercia! |
This was referenced Apr 11, 2025
chriszarate
pushed a commit
to chriszarate/gutenberg
that referenced
this pull request
Jul 1, 2025
* PostVisibility: Don't immediately publish a post when changing status to 'private' * PostVisibility: Refactor to use 'RadioControl' Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: afercia <afercia@git.wordpress.org>
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.
What?
Closes #69442.
PR does the following:
RadioControlinstead of customfieldsetandPostVisibilityChoiceTesting Instructions
window.wp.data.dispatch( 'core/editor' ).autosave().Testing Instructions for Keyboard
Same.
Screenshots or screencast