Global styles revisions: add pagination #56799
Merged
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
12 tasks
|
Size Change: +644 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in cc20a31. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7282037588
|
ramonjd
added
Global Styles
ramonjd
force-pushed
the
try/global-styles-revisions-pagination
branch
from
0a671f7
to
0eef736
Compare
ramonjd
force-pushed
the
try/global-styles-revisions-pagination
branch
from
0a61895
to
c11233f
Compare
ramonjd
commented
Dec 14, 2023
packages/edit-site/src/components/global-styles/screen-revisions/index.js
Show resolved
Hide resolved
ramonjd
force-pushed
the
try/global-styles-revisions-pagination
branch
from
5d90667
to
5556a41
Compare
apeatling
reviewed
Dec 18, 2023
There was a problem hiding this comment.
Few thoughts here so far.
I think we should keep the button on each revision as "Apply", and as the primary color button. Definitely keep the spacing and sizing the same a other buttons and in alignment:
Others may want it as a secondary button or a link, but I think this will get us moving forward for now..
Also, the pagination I think should always stick to the bottom of the window, and not creep up if there are a smaller number of revisions:
Finally, let's make the pagination look like Joen's example here, so no outlines (include the extra arrows):
ramonjd
force-pushed
the
try/global-styles-revisions-pagination
branch
from
b54c599
to
d4be421
Compare
ramonjd
changed the title
|
Thanks for the review @apeatling I think I've addressed the points above. |
apeatling
reviewed
Dec 19, 2023
There was a problem hiding this comment.
Visually this is looking way better, very nice!
I ran into a couple of bugs that I don't remember being present previously.
- A couple of my revisions don't have an Apply button.
- The pagination numbering is not changing
Screen.Recording.2023-12-18.at.8.02.13.PM.mov
I'll double check, but I suspect they won't have an apply button because they match the styles already loaded in the editor. I think in a previous commit I had some text in these items e.g.,
Hmm, I can't replicate this (yet). I have tried with 14 Revisions — the same number as that in your screencast — and 20+. |
I think we need something there, it felt broken without it. |
|
Happy to debug the pagination issue tomorrow. I did try a fresh build of the branch but it was the same. I will try nuking it tomorrow to see if that fixes it. |
I think a disabled Apply button, along with the text below in a smaller font "These changes match the current editor styles" would do it? |
Agree that we need something. I'll throw up a commit 👍🏻 |
|
I think it works without the button.
|
|
Without button looks great 👍 The pagination issue is a Safari problem, the data looks correct but the 
|
Holy smokes! Good catch. I'll fix that. |
|
I've never seen this before. The component state and HTML are updating fine, but it's not rendering in the browser 🙃 When I toggle a style or open the inspector it updates. This smells like another Safari bug. 
|
ramonjd
force-pushed
the
try/global-styles-revisions-pagination
branch
from
00710e7
to
b8a000e
Compare
ramonjd
commented
Dec 20, 2023
| @@ -101,11 +101,11 @@ | |||
| } | |||
|
|
|||
| .edit-site-sidebar-fixed-bottom-slot { | |||
| position: sticky; | |||
| position: absolute; | |||
There was a problem hiding this comment.
I had to change this to get around the overflow-x scroll in Safari.
The overflow is actually on
.interface-complementary-area.edit-site-global-styles-sidebar
but I decided on a more local approach to avoid side-effects.
Fudge the button
Creating an internal state for revisions to avoid refreshing
Shifting "Apply" button to the single item so that we don't have to deal with it being next to the pagination row.
Restyle apply button Reinstate fixed bottom slot
added pagination tests
ramonjd
force-pushed
the
try/global-styles-revisions-pagination
branch
from
8edc3ca
to
cc20a31
Compare
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.
Resolves #53621
Part of:
What?
Now that we have a revisions API in Core Data, let's add paginated results to global styles revisions!
Uploading 2023-12-19 15.40.44.mp4…
Why?
Good question. Because currently, there is a hard limit of 100 results for global styles revisions coming straight from the REST response.
Paginated results not only reduces the amount of space revisions take up in the sidebar, but allows users to access any number of revisions.
How?
per_pageandpagevalues touseGlobalStylesRevisions()to get paginated results.Testing Instructions
Requires a block theme like 2024.
In the Site Editor:
Because I've abstracted the pagination component from the Patterns page, check that there are no regressions between trunk and this PR:
Testing Instructions for Keyboard
It should be possible to tab down the revision items and access the pagination navigation buttons.
TODO
shouldShowRevisions