Make WordPress Core

Opened 3 years ago

Closed 4 months ago

#53617 closed defect (bug) (wontfix)

Update theme block patterns to current shape of used blocks

Reported by: ntsekouras's profile ntsekouras Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.8
Component: Bundled Theme Keywords: has-patch needs-refresh close
Focuses: Cc:

Description

There have been many enhancements and added features regarding block patterns which require their parsing in browsers 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.

Of course it's a good practice to keep patterns up to date and it might even be an opportunity to explore/tweak patterns when some blocks change their shape (this is not happening often though).

This should be included in 5.8.

Change History (29)

This ticket was mentioned in PR #1483 on WordPress/wordpress-develop by ntsekouras.


3 years ago
#1

  • Keywords has-patch added

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 :

  • Image blocks with align left, right or center needed a div wrapper and the appropriate classes
  • A Group block with the extra div wp-block-groupinner-container
  • Column blocks with a number in width like <!-- wp:column {"width":80}. The current shape has width as a string, so it should be <!-- wp:column {"width":"80%"}

Most deprecations for the other themes where about the extra div in Group block and the borderRadius attribute in Button block.

## Testing instructions

  1. Verify that every pattern's design is not affected and can be inserted with no problems as before
  2. Verify that there are no logs produced from deprecated block conversions

## 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.

#2 @desrosj
3 years ago

  • Milestone changed from Awaiting Review to 5.8

It looks like this one was overlooked. Adding to the 5.8 milestone to test and consider.

desrosj commented on PR #1483:


3 years ago
#3

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.

Block successfully updated for `core/button` ( 
Object { name: "core/button", icon: {…}, keywords: (1) […], attributes: {…}, providesContext: {}, usesContext: [], supports: {…}, styles: (2) […], save: save(), apiVersion: 2, … }
 ).

New content generated by `save` function:

<div class="wp-block-button is-style-outline"><a class="wp-block-button__link has-text-color has-background no-border-radius" style="background-color:#000000;color:#ffffff">Visit</a></div>

Content retrieved from post body:

<div class="wp-block-button is-style-outline"><a class="wp-block-button__link has-text-color has-background no-border-radius" style="background-color:#000000;color:#ffffff">Visit</a></div> parser.js:599:11

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?

desrosj commented on PR #1483:


3 years ago
#4

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.

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.

#5 @desrosj
3 years ago

  • Component changed from Themes to Bundled Theme
  • Keywords commit added
  • Owner set to desrosj
  • Status changed from new to reviewing
  • Version set to trunk

The PR looks good and fixes a large majority of the console notices that are happening in 5.8, so I'm going to commit that.

ocean90 commented on PR #1483:


3 years ago
#6

What happens if the theme is updated before WordPress 5.8? Won’t this make the inserted blocks invalid?

#7 @desrosj
3 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 51372:

Bundled Themes: Update block patterns to match the latest versions of core/* blocks.

When using the post editor with a bundled theme active, there will be a number of console.info notices printed to the browser’s console.

These notices are caused by block patterns containing outdated and deprecated versions of core/* blocks. Before rendering the blocks, they need to be updated, and this process outputs information to the console.

Props ntsekouras.
Fixes #53617.

desrosj commented on PR #1483:


3 years ago
#8

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.

desrosj commented on PR #1483:


3 years ago
#9

@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 core/column block does fail validation and outputs this warning:

<details>
<summary>Console output</summary>

Block validation: Block validation failed for `core/media-text` ( 
Object { name: "core/media-text", icon: {…}, keywords: (2) […], attributes: {…}, providesContext: {}, usesContext: [], supports: {…}, styles: [], save: media_text_save_save(_ref), category: "media", … }
 ).

Content generated by `save` function:

<div class="wp-block-media-text alignwide is-stacked-on-mobile is-style-twentytwentyone-border"><figure class="wp-block-media-text__media"><img src="http://five-five.wordpress.test/wp-content/themes/twentytwentyone/assets/images/playing-in-the-sand.jpg" alt="“Playing in the Sand” by Berthe Morisot" class="wp-image-1752"/></figure><div class="wp-block-media-text__content"></div></div>

Content retrieved from post body:

<div class="wp-block-media-text alignwide is-stacked-on-mobile is-style-twentytwentyone-border"><figure class="wp-block-media-text__media"><img src="http://five-five.wordpress.test/wp-content/themes/twentytwentyone/assets/images/playing-in-the-sand.jpg" alt="&#8220;Playing in the Sand&#8221; by Berthe Morisot" class="wp-image-1752 size-full"/></figure><div class="wp-block-media-text__content"></div></div> 
BlockPattern@http://five-five.wordpress.test/wp-includes/js/dist/block-editor.js?ver=2e2da9c7130cd4f50dca6a4fe997e882:25186:17
BlockPatternList@http://five-five.wordpress.test/wp-includes/js/dist/block-editor.js?ver=2e2da9c7130cd4f50dca6a4fe997e882:25226:23
div
InserterPanel@http://five-five.wordpress.test/wp-includes/js/dist/block-editor.js?ver=2e2da9c7130cd4f50dca6a4fe997e882:24885:15
BlockPatternsPerCategories@http://five-five.wordpress.test/wp-includes/js/dist/block-editor.js?ver=2e2da9c7130cd4f50dca6a4fe997e882:25295:18
BlockPatternsTabs@http://five-five.wordpress.test/wp-includes/js/dist/block-editor.js?ver=2e2da9c7130cd4f50dca6a4fe997e882:25351:18
div
div
TabPanel@http://five-five.wordpress.test/wp-includes/js/dist/components.js?ver=989f6de80dd9e531142dcfdea0309e37:45159:19
InserterTabs@http://five-five.wordpress.test/wp-includes/js/dist/block-editor.js?ver=2e2da9c7130cd4f50dca6a4fe997e882:25624:18
div
div
div
InserterMenu@http://five-five.wordpress.test/wp-includes/js/dist/block-editor.js?ver=2e2da9c7130cd4f50dca6a4fe997e882:25677:22
InserterLibrary@http://five-five.wordpress.test/wp-includes/js/dist/block-editor.js?ver=2e2da9c7130cd4f50dca6a4fe997e882:38163:22
div
div
444/FocusManaged<@http://five-five.wordpress.test/wp-includes/js/dist/edit-post.js?ver=2ddfbfc078dd7ea40fece8d763fa80f6:7920:18
div
FocusReturn@http://five-five.wordpress.test/wp-includes/js/dist/components.js?ver=989f6de80dd9e531142dcfdea0309e37:12057:106
WithFocusReturn(Component)
div
_class@http://five-five.wordpress.test/wp-includes/js/dist/components.js?ver=989f6de80dd9e531142dcfdea0309e37:11905:104
_class@http://five-five.wordpress.test/wp-includes/js/dist/edit-post.js?ver=2ddfbfc078dd7ea40fece8d763fa80f6:7900:46
div
_class@http://five-five.wordpress.test/wp-includes/js/dist/components.js?ver=989f6de80dd9e531142dcfdea0309e37:6494:104
div
PopoverWrapper@http://five-five.wordpress.test/wp-includes/js/dist/edit-post.js?ver=2ddfbfc078dd7ea40fece8d763fa80f6:7924:17
div
div
div
InterfaceSkeleton@http://five-five.wordpress.test/wp-includes/js/dist/edit-post.js?ver=2ddfbfc078dd7ea40fece8d763fa80f6:4157:16
div
this.wp.components</navigate_regions</<@http://five-five.wordpress.test/wp-includes/js/dist/components.js?ver=989f6de80dd9e531142dcfdea0309e37:45918:26
div
FocusReturnProvider@http://five-five.wordpress.test/wp-includes/js/dist/components.js?ver=989f6de80dd9e531142dcfdea0309e37:15696:102
Layout@http://five-five.wordpress.test/wp-includes/js/dist/edit-post.js?ver=2ddfbfc078dd7ea40fece8d763fa80f6:8010:79
ErrorBoundary@http://five-five.wordpress.test/wp-includes/js/dist/editor.js?ver=a62559f50dd2b6b353cf91faa01b654e:7820:46
BlockEditorProvider@http://five-five.wordpress.test/wp-includes/js/dist/block-editor.js?ver=2e2da9c7130cd4f50dca6a4fe997e882:23803:18
this.wp.blockEditor</withRegistryProvider</<@http://five-five.wordpress.test/wp-includes/js/dist/block-editor.js?ver=2e2da9c7130cd4f50dca6a4fe997e882:23541:31
WithRegistryProvider(BlockEditorProvider)
BlockContextProvider@http://five-five.wordpress.test/wp-includes/js/dist/block-editor.js?ver=2e2da9c7130cd4f50dca6a4fe997e882:16595:15
EntityProvider@http://five-five.wordpress.test/wp-includes/js/dist/core-data.js?ver=3554010244a974993efe1958d3e1426a:4413:14
EntityProvider@http://five-five.wordpress.test/wp-includes/js/dist/core-data.js?ver=3554010244a974993efe1958d3e1426a:4413:14
EditorProvider@http://five-five.wordpress.test/wp-includes/js/dist/editor.js?ver=a62559f50dd2b6b353cf91faa01b654e:13786:46
withDispatch/</<@http://five-five.wordpress.test/wp-includes/js/dist/data.js?ver=c81752ab00c3fbcf956387ec3ac1e162:3619:48
withSelect/</<@http://five-five.wordpress.test/wp-includes/js/dist/data.js?ver=c81752ab00c3fbcf956387ec3ac1e162:3443:33
_class2@http://five-five.wordpress.test/wp-includes/js/dist/compose.js?ver=8a43b90ecf2c5c35b7352e8182a3ce48:2714:48
443/withRegistryProvider</<@http://five-five.wordpress.test/wp-includes/js/dist/editor.js?ver=a62559f50dd2b6b353cf91faa01b654e:13274:33
WithRegistryProvider(WithSelect(WithDispatch(EditorProvider)))
div
DropZoneProvider@http://five-five.wordpress.test/wp-includes/js/dist/components.js?ver=989f6de80dd9e531142dcfdea0309e37:36320:46
slot_fill_provider_SlotFillProvider@http://five-five.wordpress.test/wp-includes/js/dist/components.js?ver=989f6de80dd9e531142dcfdea0309e37:11594:18
SlotFillProvider@http://five-five.wordpress.test/wp-includes/js/dist/components.js?ver=989f6de80dd9e531142dcfdea0309e37:11650:46
Editor@http://five-five.wordpress.test/wp-includes/js/dist/edit-post.js?ver=2ddfbfc078dd7ea40fece8d763fa80f6:8288:46
withDispatch/</<@http://five-five.wordpress.test/wp-includes/js/dist/data.js?ver=c81752ab00c3fbcf956387ec3ac1e162:3619:48
withSelect/</<@http://five-five.wordpress.test/wp-includes/js/dist/data.js?ver=c81752ab00c3fbcf956387ec3ac1e162:3443:33
_class2@http://five-five.wordpress.test/wp-includes/js/dist/compose.js?ver=8a43b90ecf2c5c35b7352e8182a3ce48:2714:48 react_devtools_backend.js:2560:23
    overrideMethod moz-extension://5adabd99-7ad9-8a4b-9836-2702c47081a5/build/react_devtools_backend.js:2560
    log http://five-five.wordpress.test/wp-includes/js/dist/blocks.js?ver=a04998bde4e146842a4163ebe9a9e4af:8846
    createBlockWithFallback http://five-five.wordpress.test/wp-includes/js/dist/blocks.js?ver=a04998bde4e146842a4163ebe9a9e4af:10748
    createBlockWithFallback http://five-five.wordpress.test/wp-includes/js/dist/blocks.js?ver=a04998bde4e146842a4163ebe9a9e4af:10745
    createParse http://five-five.wordpress.test/wp-includes/js/dist/blocks.js?ver=a04998bde4e146842a4163ebe9a9e4af:10807
    createParse http://five-five.wordpress.test/wp-includes/js/dist/blocks.js?ver=a04998bde4e146842a4163ebe9a9e4af:10806
    blocks http://five-five.wordpress.test/wp-includes/js/dist/block-editor.js?ver=2e2da9c7130cd4f50dca6a4fe997e882:25191
    mountMemo http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:15807
    useMemo http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:16029
    useMemo http://five-five.wordpress.test/wp-includes/js/dist/vendor/react.js?ver=16.9.0:1680
    BlockPattern http://five-five.wordpress.test/wp-includes/js/dist/block-editor.js?ver=2e2da9c7130cd4f50dca6a4fe997e882:25190
    renderWithHooks http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:15246
    mountIndeterminateComponent http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:17480
    beginWork$1 http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:18624
    beginWork$$1 http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:23331
    performUnitOfWork http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:22346
    workLoopSync http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:22323
    renderRoot http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:22016
    runRootCallback http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:21692
    flushSyncCallbackQueueImpl http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:11491
    unstable_runWithPriority http://five-five.wordpress.test/wp-includes/js/dist/vendor/react.js?ver=16.9.0:2820
    runWithPriority$2 http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:11443
    flushSyncCallbackQueueImpl http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:11487
    flushSyncCallbackQueue http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:11476
    scheduleUpdateOnFiber http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:21569
    dispatchAction http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:15954
    append http://five-five.wordpress.test/wp-includes/js/dist/compose.js?ver=8a43b90ecf2c5c35b7352e8182a3ce48:3733
    runWaitingList http://five-five.wordpress.test/wp-includes/js/dist/priority-queue.js?ver=e9443030125d73a24aeee83a51ebd259:226
    (Async: requestIdleCallback handler)
    runWaitingList http://five-five.wordpress.test/wp-includes/js/dist/priority-queue.js?ver=e9443030125d73a24aeee83a51ebd259:230
    (Async: requestIdleCallback handler)
    runWaitingList http://five-five.wordpress.test/wp-includes/js/dist/priority-queue.js?ver=e9443030125d73a24aeee83a51ebd259:230
    (Async: requestIdleCallback handler)
    add http://five-five.wordpress.test/wp-includes/js/dist/priority-queue.js?ver=e9443030125d73a24aeee83a51ebd259:251
    useAsyncList http://five-five.wordpress.test/wp-includes/js/dist/compose.js?ver=8a43b90ecf2c5c35b7352e8182a3ce48:3741
    commitHookEffectList http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:20124
    commitPassiveHookEffects http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:20154
    callCallback http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:341
    invokeGuardedCallbackDev http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:391
    invokeGuardedCallback http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:448
    flushPassiveEffectsImpl http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:23006
    unstable_runWithPriority http://five-five.wordpress.test/wp-includes/js/dist/vendor/react.js?ver=16.9.0:2820
    runWithPriority$2 http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:11443
    flushPassiveEffects http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:22979
    renderRoot http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:21939
    runRootCallback http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:21692
    flushSyncCallbackQueueImpl http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:11491
    unstable_runWithPriority http://five-five.wordpress.test/wp-includes/js/dist/vendor/react.js?ver=16.9.0:2820
    runWithPriority$2 http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:11443
    flushSyncCallbackQueueImpl http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:11487
    flushSyncCallbackQueue http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:11476
    discreteUpdates$1 http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:21815
    discreteUpdates http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:2357
    dispatchDiscreteEvent http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:6104
    addEventBubbleListener http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:5463
    trapEventForPluginEventSystem http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:6098
    trapBubbledEvent http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:6042
    listenTo http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:6280
    ensureListeningTo http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:8806
    setInitialDOMProperties http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:8874
    setInitialProperties http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:9056
    finalizeInitialChildren http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:10080
    completeWork http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:19243
    completeUnitOfWork http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:22382
    performUnitOfWork http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:22356
    workLoopSync http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:22323
    renderRoot http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:22016
    runRootCallback http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:21692
    flushSyncCallbackQueueImpl http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:11491
    unstable_runWithPriority http://five-five.wordpress.test/wp-includes/js/dist/vendor/react.js?ver=16.9.0:2820
    runWithPriority$2 http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:11443
    flushSyncCallbackQueueImpl http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:11487
    flushSyncCallbackQueue http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:11476
    scheduleUpdateOnFiber http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:21569
    dispatchAction http://five-five.wordpress.test/wp-includes/js/dist/vendor/react-dom.js?ver=16.9.0:15954
    onStoreChange http://five-five.wordpress.test/wp-includes/js/dist/data.js?ver=c81752ab00c3fbcf956387ec3ac1e162:3355
    unsubscribe http://five-five.wordpress.test/wp-includes/js/dist/data.js?ver=c81752ab00c3fbcf956387ec3ac1e162:3371
    globalListener http://five-five.wordpress.test/wp-includes/js/dist/data.js?ver=c81752ab00c3fbcf956387ec3ac1e162:2418
    globalListener http://five-five.wordpress.test/wp-includes/js/dist/data.js?ver=c81752ab00c3fbcf956387ec3ac1e162:2417
    subscribe http://five-five.wordpress.test/wp-includes/js/dist/data.js?ver=c81752ab00c3fbcf956387ec3ac1e162:2024

</details>

https://i0.wp.com/user-images.githubusercontent.com/359867/124810680-9dde9880-df2f-11eb-88cf-118bf1331a85.png

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).

#10 @desrosj
3 years ago

  • Keywords commit removed

Holding off on backporting to consider scenarios where themes are installed on older versions of WordPress where newer block syntax may be invalid in older versions of WordPress.

ntsekouras commented on PR #1483:


3 years ago
#11

What happens if the theme is updated before WordPress 5.8? Wouldn’t this make the inserted blocks invalid?

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?

ocean90 commented on PR #1483:


3 years ago
#12

Can't these changed be targeted for 5.8 only and so on?

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.

ntsekouras commented on PR #1483:


3 years ago
#13

They can, but you have to check the current version by yourself and conditionally register the different types of patterns.

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 5.6 branch and therefore are not in a minor release of 5.6, will I see an update for the theme, based on the [They can, but you have to check the current version by yourself and conditionally register the different types of patterns. above line you shared]?

ocean90 commented on PR #1483:


3 years ago
#14

It's not clear to me yet how themes get updated.

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.

desrosj commented on PR #1483:


3 years ago
#15

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.

@ocean90 what are your thoughts on these approaches?

desrosj commented on PR #1483:


3 years ago
#16

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.

#17 @desrosj
3 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

This was never reopened for backporting consideration.

Last edited 3 years ago by desrosj (previous) (diff)

ryelle commented on PR #1483:


3 years ago
#18

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” 🙂

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).

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 may become less of an issue though as more and more patterns are added to the pattern library on .org.

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.

ntsekouras commented on PR #1483:


3 years ago
#19

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:

  1. The need for themes with patterns, to have in mind the WP version they are compatible for. The main reason for this PR was to eliminate the number of logs produced in dev tools, which it's harmless in practice, but can be annoying for possible observers.
  2. Patterns in pattern directory could produce errors if they are built with a newer version of WP.

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.

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:

  1. Keep the changes that are valid for all WP versions, revert the rest and bear with the logs for now. No extra version check for themes.
  2. Implement version check in pattern directory

#20 @desrosj
3 years ago

  • Keywords fixed-major removed

I've been doing some more investigation here today, and this is a lot more involved than I anticipated.

It seems that the older themes that had block patterns added this release cycle in [51012,51033,51043,51045,51103,51106] all produce block validation errors in WordPress 5.5, even after reverting [51372]. This means the initial block patterns introduced do not work on WP 5.5.

WordPress 5.6 has more mixed results. For example, Twenty Twenty-One has no issues after [51372]. I have not tested Twenty Fifteen and earlier yet, but Twenty Twenty through Twenty Sixteen all have validation errors after [51372] but did not before.

I have not yet tested WordPress 5.7.

With this in mind, I am tempted to revert [51372] in trunk in favor of exploring a better solution that provides better backwards compatibility and aiming to release a new version of default themes shortly after 5.8 to address these issues.

@ryelle what do you think?

#21 @desrosj
3 years ago

  • Keywords needs-refresh added
  • Milestone changed from 5.8 to 5.8.1

Alright, I've done some more thorough analysis. Here's a spreadsheet of what I've found.

Without [51372]:

  • All bundled themes (11 total) work in WP 5.8.
  • All bundled themes (11 total) work in WP 5.7.
  • All but Twenty Thirteen (10 total) work in WP 5.6.
  • Twenty Sixteen through Twenty Twenty-One (5 total) work in 5.5.

Including [51372] in this release:

  • All bundled themes (11 total) work in WP 5.8.
  • Only 6 work in WP 5.7: Twenty Twenty-One, Twenty Fifteen, Twenty Twelve, Twenty Ten Twenty Nineteen, and Twenty Thirteen.
  • Only 5 work in WP 5.6: Twenty Twenty-One, Twenty Nineteen, Twenty Fifteen, Twenty Twelve, Twenty ten.
  • Only 1 works in WP 5.5: Twenty Nineteen.

Note: Twenty Nineteeen, Twenty Thirteen, and Twenty Ten are unchanged by [51372].

With these findings, I'm going to revert [51372], and move this ticket to 5.8.1. Even though there are a bunch of notices being printed to the console with the current state of patterns in themes, they work in 97% of combinations from WP 5.6 through 5.8, and 84% of combinations since WP 5.5. Including the changes proposed here would decrease support to 66% and 52% respectively.

#22 @desrosj
3 years ago

In 51434:

Bundled Themes: Revert the [51372] update to block patterns in bundled themes.

Upon further examination, this change was not great for backwards compatibility, resulting in block validation errors when running on older versions of WordPress.

While there are currently many console.info() notices caused by older format block syntax being updated to the current version included in WordPress 5.8, the blocks do not break.

Block patterns do not currently have a versioning mechanism, or a means to indicate which versions of WordPress are supported.

See #53617.

This ticket was mentioned in Slack in #core-editor by desrosj. View the logs.


3 years ago

#24 @desrosj
3 years ago

  • Milestone changed from 5.8.1 to Future Release

I'm going to punt this to Future Release. Unfortunately, it seems that this cannot be reliably solved with the current state of blocks.

For the time being, the notices caused by updating block syntax have been combined into a collapsed group using console.groupCollapsed() (see Gutenberg-54163).

Gutenberg-21703 discusses some related concepts that would help address this problem.

#25 @desrosj
3 years ago

  • Owner desrosj deleted
  • Status changed from reopened to assigned

Not going to be able to work on this near term. Removing myself as owner so other interested contributors do not hesitate to take this on.

#26 @poena
19 months ago

I am wondering if this should remain open or if we should close it as wont fix?

This cannot be reliably solved with the current state of blocks.

I believe this statement is still accurate?

I also believe there was a related discussion about removing patterns from the themes that are not bundled, and only keeping the patterns, up to date, in the pattern directory? But I do not recall where this discussion took place.

Last edited 19 months ago by poena (previous) (diff)

#27 @karmatosed
4 months ago

  • Keywords close added

I am wondering if this should remain open or if we should close it as wont fix?

My belief is this is the right path for now. I am going to note that with the keyword and we can always come back to things. Thank you everyone.

#28 @ntsekouras
4 months ago

Yes, I think we can close this one. We have added a flag in parser to skip these logs for a while now: https://github.com/WordPress/gutenberg/pull/39404

#29 @karmatosed
4 months ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

Thank you @ntsekouras I am going to close this then. I appreciate the work everyone has done on this.

Note: See TracTickets for help on using tickets.