Global styles revisions: highlight currently-loaded revision #50725
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
|
Size Change: -2 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
ramonjd
changed the title
ramonjd
added
Global Styles
Ensure the latest revision is highlighted even when there is no selected id Add useEffect dependencies Updating tests Align text to left for long strings
ramonjd
force-pushed
the
fix/global-revision-highlighted-state
branch
from
b8183d5 to
b604b97
Compare
andrewserong
approved these changes
May 18, 2023
There was a problem hiding this comment.
Good catch @ramonjd, and nice fix — I like the switch of referring to the revision from current to selected, too, that makes things a bit clearer 👍
✅ Confirmed that on trunk, some of the time the second or third revision would be erroneously highlighted in the global styles revisions view
✅ With this PR applied, when opening the global styles revisions, the correct item is highlighted now
✅ Updated Unsaved text is displayed for unsaved revisions:
Just left an optional comment about possibly using an EMPTY_ARRAY const for a couple of empty arrays. But with or without that, this LGTM! ✨
| } = select( coreStore ); | ||
| const dirtyEntityRecords = __experimentalGetDirtyEntityRecords(); | ||
| const _currentUser = getCurrentUser(); | ||
| const _isDirty = dirtyEntityRecords.length > 0; | ||
| const globalStylesRevisions = | ||
| getCurrentThemeGlobalStylesRevisions() || []; | ||
| const _authors = getUsers( SITE_EDITOR_AUTHORS_QUERY ); | ||
| const _authors = getUsers( SITE_EDITOR_AUTHORS_QUERY ) || []; |
There was a problem hiding this comment.
Tiny nit (and it might not matter too much in this context): should we use a stable reference of an empty array, instead of creating a new empty array? I mostly noticed it because authors is used in the useMemo dependency array.
It might be worth using EMPTY_ARRAY two lines up, too?
|
Thanks for the quick review @andrewserong I'll make those changes, they sound immensely reasonable. I appreciate it! |
|
Flaky tests detected in 07f805d. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5010746255
|
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?
Ensure that the revision being shown in the revisions panel is the one that is selected.
Why?
This fixes a bug whereby the revisions panel was displaying the wrong, or no, currently-selected revision in the list.
How?
Checking for the currently-selected element, if there's none, we use the first one.
Testing Instructions
Create and save several global styles changes in the site editor.
When opening the revisions panel, ensure that the revision being displayed is the currently selected one. It should be blue.
Screenshots or screencast