Global Styles: Save and Delete functionality for user variations #46952
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
ribaricplusplus
added
the
Global Styles
|
Size Change: +3.52 kB (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 975326c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/3886573790
|
ribaricplusplus
changed the title
ribaricplusplus
force-pushed
the
ribaricplusplus
branch
from
c3c2a1b
to
51fbfd9
Compare
ribaricplusplus
force-pushed
the
ribaricplusplus
branch
from
b8171fc
to
15c9e21
Compare
ribaricplusplus
requested review from
spacedmonkey,
noahtallen and
nerrad
as code owners
|
@jasmussen @youknowriad @ellatrix Can I have some help finding someone to review this? |
|
This is an impressive PR. Great work. It's going to take some time for me to review it properly. That said, first I think I want to mention is that if you're able to split it into smaller PRs that can ship independently, I think that's better for reviews and iterations. For instance the "wp-env" change is clearly something that need to be extracted out of this PR into a dedicated PR. |
|
I also see that there are some changes to the php side (endpoints I assume), is it possible to extract these to their own PR? In other words, does it make sense for these changes to happen independently or not? |
Thank you for looking into it! I will try to split the things I can into other PRs. The wp-env thing definitely can go to an independent PR.
Unfortunately I don't think it makes sense splitting the PHP endpoints into a separate PR, because the JavaScript relies on new endpoints to work. |
Yeah, but I mean can we land the endpoints first and then the JS changes? |
Yep, we could do that. Should I split it that way? |
Yeah I think so, especially if these endpoints changes could make sense regardless of whether we land the JS or not. |
|
Alright. I will do that and ping you when it's done. Thanks a lot for your help. |
This was referenced Jan 11, 2023
|
@youknowriad I split this PR into 3:
I'm closing this PR in favor of those 3. |
|
Thanks @ribaricplusplus -- apologies for the delay in getting back to you. Had this to get to today and, upon catching up, I see you've already gotten direction! |
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?
Partially addresses #45371. Makes it possible to save and delete custom user variations. Adds a warning that is shown if a user has modified Global Styles and then tries to switch either to a theme variation or a custom user variation, because if the user switches, they would lose the unsaved changes.
This PR also makes a small tweak to
wp-envto make it possible to run xdebug for php unit tests. Just run them usingnpm run test:unit:php:debug@carolinan @annezazu Can you help me find someone to review this? I don't know who to ping.
Testing Instructions
The PR includes unit tests and e2e tests. For manual testing, see the screencasts for a demonstration of all testable functionality.
Screenshots or screencast
Creating and deleting variations
Video1.mp4
Switching between variations
Video2.mp4
Keeping the selected custom variation in sync with current user global styles
Video3.mp4
Display a warning if unsaved changes are about to be lost while switching
Video4.mp4