Global styles: enable user global styles revisions #46667
Closed
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
ramonjd
force-pushed
the
try/user-global-styles-revisions
branch
from
475a386
to
e32c7d4
Compare
|
Size Change: +1.9 kB (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
ramonjd
added
[Type] Feature
ramonjd
force-pushed
the
try/user-global-styles-revisions
branch
2 times, most recently
from
0a8dfd1
to
777738b
Compare
There was a problem hiding this comment.
Is adding a revisions property to the global styles response a good idea? Should we rather add a new entry to _links and add a separate endpoint for revisions to avoid huge responses?
Are there any scenarios where we might want to fetch global styles (or part of it) without the revisions? If so, yeah it's probably better to have a separate endpoint for them.
Testing this a bit, it's working pretty well for under 10 revisions, but started acting funny once I added a bunch of them in a row (I didn't always reload the page between saves, not sure if that might have had any influence).
As mentioned below, only the last 10 revisions are appearing in the dropdown.
I was switching between style variations in TT3, and ended up with all the variations showing gradient backgrounds:
Also, changes to block global styles weren't reverted when going back to a previous revision.
lib/compat/wordpress-6.2/class-gutenberg-rest-global-styles-controller-6-2.php
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/global-styles/screen-revisions.js
Outdated
Show resolved
Hide resolved
Good pick up, thank you! I thought I saw that too but was hoping that it might have been also happening on trunk. Turns out hope is a dangerous thing :) |
ramonjd
force-pushed
the
try/user-global-styles-revisions
branch
from
3f1543a
to
e0753cb
Compare
It actually is occurring on trunk. Steps to reproduce:
2022-12-23.14.25.12.mp4
Issue created: #46758 |
ramonjd
changed the title
ramonjd
force-pushed
the
try/user-global-styles-revisions
branch
from
fcca8f6
to
7ac4391
Compare
ramonjd
added
[Type] Experimental
ramonjd
commented
Dec 29, 2022
| @@ -453,7 +453,7 @@ __experimentalGetTemplateForLink.shouldInvalidate = ( action ) => { | |||
| ); | |||
| }; | |||
|
|
|||
| export const __experimentalGetCurrentGlobalStylesId = | |||
| export const __experimentalGetCurrentGlobalStyles = | |||
There was a problem hiding this comment.
The reason why I'm returning the entire global style object instead of just the ID is so we can access the revisions url.
E.g.: currentGlobalStyles?._links?.[ 'version-history' ]?.[ 0 ]?.href;
There was a problem hiding this comment.
It seemed a waste not to repurpose this method since it was fetching what we wanted anyway and only returning the id.
Plus it's experimental so I didn't see much harm in changing it.
|
Some questions from the design perspective:
cc @WordPress/gutenberg-design |
…ating the full revision. The revision should overwrite everything.
- same behaviour as post editor - since revisions includes the current saved post, so we need at least two in order to switch
…yet delete global styles revisions and, because of other tests, we can't rely on the post revisions being a certain count so we try to cater for both in the test for now. Added aria label to buttons in the UI
Checking for the number of revision buttons in the revisions list.
…rentGlobalStylesId, which has been removed in this PR. Adding a reset button.
Removing secondary button styling
Updating e2e tests A sad attempt at caching the unsaved userConfig Adding an `is_latest` prop to the rest response to indicate the latest saved revision
ramonjd
force-pushed
the
try/user-global-styles-revisions
branch
from
17f9ba6
to
9975e1a
Compare
Redesigning the buttons view Rationalizing rest API return properties
ramonjd
force-pushed
the
try/user-global-styles-revisions
branch
from
9975e1a
to
408a755
Compare
|
I've been running around in circles in an attempt to come up with a reasonable workflow today, and have concerns about how to deal with unsaved changes in the editor. I'm worried that users won't know that loading a revision overwrites the styles in the editor canvas, thereby removing any unsaved changes. For example, a user might want to browse previous revisions only. @jameskoster raised this concern at the start of this PR, and suggested that a warning might be temporary approach until it's possible to load revision styles independent to the styles that show in the editor. This is what I've come up with following those lines: 2023-04-18.16.35.13.mp4I haven't committed it to this PR yet. Thinking more broadly, I have positive feelings towards an in-editor revisions view, let's say something like the style book where we could compile and load any style without affecting the main editor canvas. That would require a major refactor, but it might be useful to serve as a basis for comparing template revisions as well. I'd like to explore this option in hack branch. |
Yeah this feels like a blocker to me. I know we have the Undo affordance, but it doesn't feel intuitive in this case. What you shared above seems like a good start. The action(s) are significant enough that it warrants being placed in a modal imo. We could potentially invoke a similar UI in other flows where customisations are lost such as switching variations: #40561 |
|
Thanks folks. @andrewserong and @tellthemachines had some good ideas that I can trial as well: namely, auto-saving unsaved changes (if any) when opening the revisions panel. This would technically create a new revision I think, but I'm interested to see how it would work in practice.
This makes perfect sense too. I'll play around. |
It was all @tellthemachines' idea, I was just being the cheer squad 😄 Thanks for digging into this feature so deeply! |
3 tasks
My plan is to implement this in this PR, but I took a stab at repurposing the stylebook code to load blocks and styles independently. The main benefit is that users can open revisions, try them out and return to the editor in whatever state they left it in, that is, any unsaved changes are unaffected. There's still the need for a prompt to save unsaved changes before a revision is restored just in case. |
|
Closing this PR and tracking follow ups based on the experimental PR over in the issue: |
4 tasks
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.
Related to:
What?
Display revisions to a user's global style changes in the global styles sidebar (revisions for the
wp_global_stylescustom post type).These revisions are the ones made by the current, logged-in author. (should we show all authors' revisions?)
We're only showing maximum 100 to limit the payload.
2023-04-12.16.50.24.mp4
How?
By:
'version-history'entry to_links, mimicking the one from the posts API that containscountandhrefproperties/revisions, which also mimics the REST API's/wp/v2/posts/<parent>/revisions– that returns data fromwp_get_post_revisions()Testing Instructions
npm run test:unit:php -- --filter Gutenberg_REST_Global_Styles_Controller_Testnpm run test:e2e:playwright -- --headed site-editor/user-global-styles-revisions.spec.js