Allow import/export patterns as JSON files #54337
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
kevin940726
added
[Type] Feature
|
Size Change: +581 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
kevin940726
commented
Sep 11, 2023
| @@ -252,6 +268,12 @@ function GridItem( { categoryId, item, ...props } ) { | |||
| : __( 'Duplicate' ) | |||
| } | |||
| /> | |||
| { item.type === USER_PATTERNS && ( | |||
| <MenuItem onClick={ () => exportAsJSON() }> | |||
There was a problem hiding this comment.
Question: Should we close the dropdown menu after it's pressed? I feel like that might be doing too much. I could use some accessibility feedback here 🤔 .
There was a problem hiding this comment.
I'm no a11y expert so I'll defer to others here. One point to note though is that the same dropdown is closed when we rename a pattern etc so it should probably be consistent one way or the other.
There was a problem hiding this comment.
Thanks for working on this @kevin940726 👍
Tests nicely for me!
✅ Verified that exported patterns match between wp-admin and site editor
✅ Importing patterns works well
The only issue I spotted was a minor implementation detail. We should probably leverage private apis to export createPatternFromFile from the patterns package.
The __experimental prefix from all the other actions etc are being removed in #54338.
| @@ -252,6 +268,12 @@ function GridItem( { categoryId, item, ...props } ) { | |||
| : __( 'Duplicate' ) | |||
| } | |||
| /> | |||
| { item.type === USER_PATTERNS && ( | |||
| <MenuItem onClick={ () => exportAsJSON() }> | |||
There was a problem hiding this comment.
I'm no a11y expert so I'll defer to others here. One point to note though is that the same dropdown is closed when we rename a pattern etc so it should probably be consistent one way or the other.
kevin940726
force-pushed
the
add/import-export-patterns-from-json
branch
2 times, most recently
from
5ffa599
to
079f4e5
Compare
|
@SaxonF Just to confirm do you think this is a good first approach for adding import/export features to patterns? |
kevin940726
force-pushed
the
add/import-export-patterns-from-json
branch
from
079f4e5
to
33dbc93
Compare
There was a problem hiding this comment.
Thanks for updating this PR @kevin940726 👍
There are a couple of issues after the rebase that break the intended functionality.
While testing I also wondered if we should be exporting the pattern's categories too.
I was on a category other than "All patterns" or "Uncategorized" (the one belonging to the pattern I just tested exporting). Importing after that doesn't give much indication of where your imported pattern went, just the snackbar notice that it was imported.
The category export/import could be left to a follow-up, if it is in fact desired. What do you think?
I also ran into another edge case. If you try to import the same pattern file multiple times nothing happens. If you then delete the imported pattern and try again the import still fails without error. Refreshing the page and trying again the pattern import works.
We should also move the Import menu option to the bottom as it's the least likely to be what the user wants.
Given we are short on time before the beta for 6.4 is cut, this could be pushed to 6.5 if needed.
Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>
kevin940726
force-pushed
the
add/import-export-patterns-from-json
branch
from
a50918d
to
0dbb84b
Compare
Thanks for spotting them!
This is fixed by always navigating to the
Yeah, I'm not sure if we want that in the aspect of product or UX. We can discuss it in a follow-up if needed 👍 .
Ahh right. This is a bug and it's now fixed 👍 .
Moved to the bottom 👍 .
Agreed 👍 |
Hmm, on another thought, I think we could instead auto-apply the current category when importing. That might be what the users expect anyway 🤔. |
Do you think we have enough time to make this change and still land this for 6.4? Could applying the current category be a follow-up? |
I think it's only a few lines change and I can do it by today. |
Done in 9a625d8. |
aaronrobertshaw
approved these changes
Sep 20, 2023
There was a problem hiding this comment.
Thanks for the iterations @kevin940726 👍
Done in 9a625d8.
This works well for existing user-created pattern categories but doesn't work if the user has a theme pattern category selected e.g. Featured etc.
Everything else is testing well for me. I think we should merge this and iterate on the theme pattern category issue in a follow-up, what do you think?
Screen.Recording.2023-09-20.at.6.10.14.pm.mp4
This was referenced Sep 28, 2023
This was referenced Oct 11, 2023
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?
Close #53288. Allow import/export patterns as JSON files in the site editor's pattern screen.
Why?
To match wp-admin functionality.
How?
The implementation is quite straightforward. Just add some menu items to the corresponding dropdown menus.
Testing Instructions
/wp-admin/edit.php?post_type=wp_block..jsonfile.Imported "title" from JSON..Testing Instructions for Keyboard
Same as above.
Screenshots or screencast
Kapture.2023-09-11.at.11.23.12.mp4