-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Update theme block patterns to current shape of used blocks #1483
Update theme block patterns to current shape of used blocks #1483
Conversation
I've done some testing, and overall this works great! A lot of the notices are now gone. I did notice that there was one notice still present in all of the themes.
Are we able to fix this one as well? I couldn't seem to find where there was a button with "Visit" as the text. It also seems that the retrieved vs. generated text is exactly the same. Is the difference in the JSON being read? |
Ah, after re-reading your notes, I think that the remaining notice may be what you are calling out here. I'm going to commit everything here to Core now as it's a big improvement. |
What happens if the theme is updated before WordPress 5.8? Wouldn’t this make the inserted blocks invalid? |
Hmm, good question. Let me give this a test. |
@ocean90 I've tested with the updates in this PR and the latest point release for 5.5, 5.6, and 5.7. using Twenty Twenty and Twenty Twenty-One. It seems that the patterns still work and no notices are output when using Twenty Twenty. But the notices eliminated in 5.8 after the changes in this PR are present in these older versions when using Twenty Twenty-One. In 5.5, there are instances where the Console output
After inserting the patterns, saving as draft and refreshing the page, the blocks render correctly. But I was not able to get them to display correctly by using resolve to convert to HTML or blocks (this is actually broken entirely in 5.5 when interacting with the Portfolio pattern). |
Can't these changed be targeted for 5.8 only and so on? I don't know if themes work differently but I guess every release has it's branch, no? My changes were made for 5.8 and tested with the GB plugin disabled. Also noting that the parsing of patterns, which produced the logs, is introduced in 5.8. |
They can, but you have to check the current version by yourself and conditionally register the different types of patterns. For example Twenty Twenty-One supports at least WordPress 5.3 which means the theme can be installed on any WordPress version between 5.3 and the current. |
It's not clear to me yet how themes get updated. For example if I am in a WP 5.6 and the above changes are not included in |
Themes, which also includes the default themes, are updated via the Theme Directory (Twenty Twenty-One) just like plugins. So while their code lives in the same repo as WordPress core their releases are independent from WordPress core. |
I think there are a few options here. One option is to leave the patterns how they are when they were first added. While there would be a bunch of console notices, it's reasonable to expect that the block editor will be backwards compatible with old block formats. The same cannot be said for newer block formats being valid in old versions of WordPress Core. Another option would be to have a way for the default themes to load different block patterns based on the version of WordPress being used. This could be nice, as it would allow the addition of block patterns utilizing new blocks added in later WordPress releases while still fully supporting patterns that were added for earlier versions of WordPress. However, this would mean a potentially ever-growing list of block patterns. @ocean90 what are your thoughts on these approaches? |
A third option that just came to me would be to prevent patterns from showing in older versions of WordPress when the block syntax is updated. So based on my testing above, for example, the patterns resulting in invalid blocks would not be shown in WordPress 5.5. This would solve the issue, but over time, older versions of WordPress would have fewer and fewer theme defined block patterns available. This may become less of an issue though as more and more patterns are added to the pattern library on .org. |
I don't think we should leave them as-is, since leaving warnings that "it's okay to ignore" is not a good precedent to set (in the latest version, at least). My preference is for the second option - duplicate the patterns and keep the older syntax behind a version check. I think this only needs to happen for the broken patterns (the columns), the ones that work but throw console notices in older WP are probably fine, and would cut down on that “ever growing list” 🙂
I also saw that “Resolve” and “Convert to blocks” didn’t work on the columns here, but “Attempt block recovery” worked just fine on all 3 invalid patterns in WP 5.5. I don’t think that’s an argument for keeping the new code, but just an observation.
This is an issue we'll need to solve for the pattern directory too. right now we're just assuming all patterns will be supported by anyone requesting them. I think we’ll end up with some kind of version check in the API. |
That's quite a problem here 😄 . I'll leave this for more experienced folks in themes and core to decide what will be done for now, but here are my observations. This PR has surfaced a couple of things:
I guess it wouldn't be a good practice to add more complexity by adding an extra version check for patterns in themes, as they actually work. On the other hand this version check should be built and handled for pattern directory. In addition patterns are going to be a focus for 5.9 too and its handling might change. My suggestions would be:
|
Trac ticket: https://core.trac.wordpress.org/ticket/53617
There have been many enhancements and added features regarding block patterns which require their parsing in browser's idle time, during editor's loading.
If we have deprecated versions of blocks in the patterns, they produce some info log messages in console. While this is not something that causes any problem, it can be a bit overwhelming if someone uses the dev tools.
Some details about the changes
The diff is kind of rough in some themes:
2021 - some of the deprecations :
Most deprecations for the other themes where about the extra div in
Group
block and theborderRadius
attribute inButton
block.Testing instructions
Notes
There is still one conversion coming from core patterns fetched by the pattern/directory. If you disable core patterns you'll see no conversion log messages.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.