-
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
Update preferences organization #56481
Conversation
Size Change: +528 B (0%) Total Size: 1.72 MB
ℹ️ View Unchanged
|
Flaky tests detected in 0ad51fd. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6971768839
|
This only updates the preferences in the Post Editor. Should we do the same/similar in the Site Editor? |
Of course! |
Two work in progress additions that should be accounted for when ready: |
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.
Thanks, @mtias!
I think the new a11y tab makes sense, and the changes are straightforward.
85206af
to
fee46cf
Compare
@mtias, I've removed the second "Type" label, as PR should only have one. I think the failing unit test is related to the changes here and needs to be fixed. |
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'll add some updates to enable distraction free and top toolbar to work in tandem.
5082611
to
6587730
Compare
I am wrangling the tests here ... |
An error in rendering |
Yes! I zeroed in on that too @Mamaduka since the test runner errors out with undefined reads on what those check components do. But what does this branch do wrong is a mistery :D |
BTW, this test feels like e2e, so rewrite in Playwright and let’s delete the Jest one. |
- Add "Appeareance" and "Accessibility" panels. - Update sections across the panels (blocks->inserter). - Improve copy for clarity and consistency.
6587730
to
39390ab
Compare
… the user sees on screen
test( 'Enable pre-publish flow is visible on desktop ', async ( { | ||
page, | ||
} ) => { | ||
await page.click( |
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.
These page.click
calls seem to be discouraged in the Playwright Docs (although I didn't see any reasoning in the docs, the note was added as a follow-up to some deprecations).
Does it still work if you use page.locator.click
instead?
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 to me, and the updated tests seem to be passing. 👍
Closes #56510.
This seeks to improve the organization of the preferences modal.