Patterns: create patterns package and share the create pattern modal between post editor and site editor #53161
Conversation
glendaviesnz
added
[Type] Code Quality
|
@aaronrobertshaw I initially started off with the shared model in the reusable-blocks package, but this doesn't help with the longer term aim of deprecating that package. It didn't seem like it was worth creating a |
|
Size Change: +1.55 kB (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
|
Has broken e2e tests - will look into that. |
|
Looks good! Do we still need the |
I believe the general thinking here was we'd look to deprecate the reusable blocks package as part of a larger follow-up that would complete the renaming of reusable blocks to patterns within code etc. |
Agreed 👍
I'm not sure where it best belongs. I don't see many other similar components in the Wherever it lands, we should update the PR description to match. |
|
Can this shared modal also be used in the |
|
FYI, I think there is or was a plan to deprecate the |
|
Thanks for the feedback @kevin940726, @aaronrobertshaw, @Mamaduka
As @aaronrobertshaw suggests I think we should deprecate it down the track. Some parts of it are still used, eg. the delete options in edit-site, and I think we should limit the scope of this PR to just sharing the create modal, but in a way that allows us to then follow up with moving the remaining functionality and deprecating the reusable-blocks package. To do it all in one PR will make it too hard to fully review/test I think.
Because it used core-data entities it can't go in block-editor as that package is not allowed to have anything WP specific.
I don't think we should share it back into reusable blocks package, the idea is to locate it somewhere that will enable us to replace all existing uses of
Thanks for this, will look into this a bit more, perhaps it might be worth adding an |
glendaviesnz
requested review from
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
glendaviesnz
changed the title
|
I have moved the modal to a new |
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/client-assets.php |
|
Flaky tests detected in afae324. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5722094524
|
There was a problem hiding this comment.
Thanks for the explanations
I don't think we should share it back into reusable blocks package, the idea is to locate it somewhere that will enable us to replace all existing uses of ReusableBlocksMenuItems with PatternsMenuItems.
Let me rephrase that question then. Can we not already replace ReusableBlocksMenuItems with PatternsMenuItems in the edit-site block editor and the widgets area block editor?
I don't think those two cases expand the scope of this PR much at all (especially given the new edit-patterns package). They will also make sure the new shared components line up with existing use cases.
I have moved the modal to a new edit-patterns package which I think is going to be a better long term solution, especially if the editor package is being deprecated.
The new edit-patterns package is testing well for me so far. I think it would be good to get some wider confirmation on the approach as well.
Very nice work! ✨
| onClose(); | ||
| setTitle( '' ); | ||
| } } | ||
| overlayClassName="patterns-menu-items__convert-modal" |
There was a problem hiding this comment.
What do you think of avoiding hardcoded class names? I know this isn't the components package but it could provide freedom down the line
There was a problem hiding this comment.
Have added an optional className prop
glendaviesnz
requested review from
draganescu,
talldan,
noisysocks and
tellthemachines
as code owners
Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com>
glendaviesnz
force-pushed
the
refactor/add-pattern-buttons
branch
from
3b5fb88 to
feff32a
Compare
|
This is rebased with node update now, and ready for a final test and ✅ when someone has time, please. |
There was a problem hiding this comment.
Great PR!
✅ Implementation matches all the feedback so far
✅ Creating synced and unsynced patterns, detaching, and managing patterns works in post editor
✅ Creating synced and unsynced patterns, detaching, and managing patterns works in site editor block editor
✅ Creating synced and unsynced patterns works correctly in the site editor's browse mode patterns management screen
Thanks for the nicely organised scope of the PR, too, it made it straightforward to review even though there's lots of files 🎉
LGTM! ✨
What?
Shares a create pattern modal between the post and the site editor via a newly created
patternspackage. This new package will take over all management of synced and unsynced user created patterns to allow the eventual deprecation of thereusable-blockspackage.Why?
Currently there is duplicated code for adding patterns in post editor (imported from
reusable-blocks) and site editor (local modal component), which means any changes need to be applied in two places.How?
Adds a CreatePatternModel in a new
patternspackage.Testing Instructions
Create patternblock menu to create both a synced and an unsynced pattern and make sure both options work as expectedCreate patternblock menu to create both a synced and an unsynced pattern and make sure both options work as expected and also make sure the Detach pattern and Manage patterns options on the block overflow menu work correctlyScreenshots or screencast
patterns-share.mp4