-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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: Unify the preview dropdown between post and site editors #56921
Conversation
5ec4a35
to
8e52e2d
Compare
Size Change: -440 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
Flaky tests detected in 67b8625. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7166720819
|
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.
Overall, looks good to me! There are only two points of concern.
In the Post Editor, I noticed that the button tooltip has changed from "Preview" to "View". However, this does match the context of the dropdown menu's aria label, so I consider it an improvement.
There are no longer disabled button in template parts and patterns. In the past, we intentionally left this disabled button. Here is the background: #53913 (comment)
@t-hamano Both of these were intended in this PR. Basically, each time I find a problem, I try to lean on the "simple" approach first.
|
Ok, I've restored the disabled state. |
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.
I think we need to fix the E2E test, but it looks good to me.
* WordPress/gutenberg#56921 changed the label of the preview button from "Preview" to "View". * WordPress/gutenberg#56904 removed the setting of an `is-xxx-preview` class when using the in-page preview. Targeting the parent element (that has a static class) instead. * #86033 attempted to fix a test for another Gutenberg change, but the selector used there matches multiple elements in one of our tests and so causes it to fail. Adding `[role="tab"]` into the selector fixes that.
* WordPress/gutenberg#56921 changed the label of the preview button from "Preview" to "View". * WordPress/gutenberg#56904 removed the setting of an `is-xxx-preview` class when using the in-page preview. Targeting the parent element (that has a static class) instead. * #86033 attempted to fix a test for another Gutenberg change, but the selector used there matches multiple elements in one of our tests and so causes it to fail. Adding `[role="tab"]` into the selector fixes that.
Related #52632
What?
This PR continues the work on the great unification between post and site editors. This PR unifies the DevicePreview or PreviewDropdown component. It also brings some consistency meaning that now:
Testing instructions