-
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
Align naming modals #62788
Align naming modals #62788
Conversation
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. |
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.
Nice cleanup. 👍 👍, probably just needs a code review.
Size Change: -71 B (0%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
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.
Looks good, thanks!
It seems that when the modal mounts, the focus goes to the first element. This caused the E2E test where the Tab key is pressed to move focus to the Name field to no longer pass. I fixed it so that it doesn't depend on the number of Tab key presses. |
I was mostly focussed on the modal size and auto-focussing the input in this PR. But yes, there's probably a case that the input format should be the same across. I've made a note to circle back! |
What?
Apply consistent design and behavior to naming modals in the site editor. Specifically:
size
options forModal
(most commonlysmall
).Why?
Consistency is good. Some modals (e.g. renaming a block) already exhibit these features.
Testing Instructions
In the following flows, renaming should invoke a modal with a specific
size
, and the name input should be auto-focussed:🗒️ In these scenarios all action modals will receive the given props. There may be a better way to do this, but for now it could be okay given renaming is the only action that invokes a modal.