-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Try: Add synced patterns to theme on save #675
Conversation
@mikachan Great job on this! Initial testing works well. I know it's a work in progress, but I wondered why the Inserter was set to 'no' in the pattern header. Once saved and exported, the pattern still doesn't come from the theme/patterns folder; it remains in the database. I guess there is logic behind that. For me, if I'm developing a theme, I wouldn't want anything left in the database after saving and exporting the patterns. I think there might be some thinking behind synced patterns, but as a developer creating patterns as part of the theme, I'd want them not to be restricted and synced. I'd prefer the theme to be distributed to users who could then decide to generate their own synced patterns from the ones provided by the theme, if that makes sense. Also, I think the heredoc approach might be more readable for generating the header. This allows you to include the variables directly within the string without needing to concatenate multiple parts. Again, I know this is a work in progress, so I guess any image-related details (e.g., downloaded to the theme assets/images and changing the reference to get_theme_file_uri('xxxx'), etc.) haven't been addressed yet. I'd love to contribute to this with my own pull request if it helps. Great job 🥳 |
In hindsight, I guess the reasoning for not removing them from the database is to preserve synced patterns that have already been placed throughout the site. It would be really useful if there was a flag saved within postmeta indicating whether a pattern was already synced. This could then be used to avoid removing the pattern when the theme is saved and the patterns are exported. |
It looks like the sync status is saved in postmeta under
|
# Conflicts: # includes/class-create-block-theme-api.php
Co-Authored-By: Elliott Richmond <[email protected]>
Co-Authored-By: Elliott Richmond <[email protected]>
Co-Authored-By: Elliott Richmond <[email protected]>
I've updated this PR with the changes from @eirichmond's PR (#693), refactored a couple of things and added some error handling. I think this is ready for review 🤞 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is testing pretty well for me, nice work! I was able to save custom patterns and it exported with the theme.
Thanks for the feedback @jffng and @colorful-tones! This should be ready for another review. |
I'm confused about something in my testing. I created a synced pattern, then assigned it to a template, and then went into CBT and checked only the 'Save Synced Patterns' thinking that the template would also be saved, because of this overall statement: "Any synced patterns created in the Editor will be moved to the theme. Note that this will delete all synced patterns from the Editor and any references in templates will be made relative to the theme." However, after hitting the 'Save Changes' from CBT I only see my new pattern in the patterns directory and the template has no changes at all, which makes me think the statement is a bit misleading. I imagine I would want to check the "Save Template Changes" box too, but the "any references in templates will be made relative to the theme." next to the "Save Synced Patterns" implied to me that my template changes would be saved too. 🤔 Also, after saving patterns to the theme when I go back to the template it states: "Block has been deleted or is unavailable." |
Thanks @colorful-tones! I've pushed an update that will update any templates that reference the pattern and save those changes to the theme, regardless of whether the "Save Template Changes" option is checked. I think that matches up with the expectations in the "Save Sycned Patterns" description. This should also fix the "Block has been deleted or is unavailable." issue. |
@mikachan I did a bunch of testing and everything is working. Nice work! I think this functionality will help folks with creating and saving their patterns. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I think this functionality will be super helpful.
Thanks so much for reviewing, @colorful-tones! |
This PR explores the most minimal way we can introduce some pattern management into the plugin by allowing custom patterns to be saved to the theme filesystem on save, allowing them to be exported with the theme.
Part of #287.
Testing Instructions