Global Style Revisions: ensure consistent back button behavior #55881
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
… panel. At the moment, it's using `goBack` which means the behaviour is inconsistent depending on the previous action.
ramonjd
added
[Type] Enhancement
This was referenced Nov 6, 2023
|
Size Change: +2.1 kB (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 66337ff. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6765746449
|
andrewserong
approved these changes
Nov 8, 2023
There was a problem hiding this comment.
This behaviour feels good to me when applying a revision (i.e. when applying a revision and bailing on current WIP styles) 👍
One thing I noticed is that this changes the behaviour of the close button when a user switches to the Revisions screen and then wishes to close the preview via the x button:
2023-11-08.11.37.25.mp4
This deviates a little bit from the comparable close button for the Style Book, which doesn't switch the user back to the root of the global styles interface.
From my perspective, I think I like the behaviour in this PR overall as it does maintain consistency and the idea of the Revisions screen is a little different to the Style Book — the Style Book is intended to be open while a user makes changes to other areas of global styles, whereas the Revisions screen can be thought of as a separate part of the navigation hierarchy overall, so hitting the close button and being taken back to the root of the global styles feels pretty natural to me.
Just thought I'd mention all that in case the changes there weren't intended.
LGTM! ✨
packages/edit-site/src/components/global-styles/screen-revisions/index.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/global-styles/screen-revisions/index.js
Outdated
Show resolved
Hide resolved
…ns/index.js Co-authored-by: Andrew Serong <[email protected]>
…ns/index.js Co-authored-by: Andrew Serong <[email protected]>
|
Thanks for testing this so thoroughly @andrewserong Suggestions applied 🙇🏻
Good point, and agree! |
ramonjd
deleted the
update/global-styles-revision-apply-returns-to-styles-main
branch
bph
changed the title
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?
Returns the user to the editor and main global styles panel after a global styles revision is applied.
Before
2023-11-06.13.47.20.mp4
After
2023-11-06.13.45.57.mp4
Why?
After applying a revision,
goBack()is called however the behaviour is inconsistent depending on the previous action.When applying a revision the route should switch to the global styles panel.
How?
Using
goTo( '/' ), which will return the user to the global styles main panel.Testing Instructions