Gallery block: replace gallery experimental setting with a check for use_balanceTags #34979
Conversation
glendaviesnz
added
the
[Block] Gallery
glendaviesnz
requested review from
spacedmonkey and
TimothyBJacobs
as code owners
glendaviesnz
force-pushed
the
remove/gallery-experimental-flag
branch
from
adf05d1
to
4e145c0
Compare
|
Size Change: 0 B Total Size: 1.07 MB ℹ️ View Unchanged
|
|
Works well for me 🎉 The experiment no longer appears:
The correct gallery corresponding to the value of the
This seems like the right way to go about it for now, assuming we just want to hide the experience in the background for sites with |
There was a problem hiding this comment.
Thanks for the changes @glendaviesnz
Works as advertised me.
There was a problem hiding this comment.
Just adding that this is testing well for me too. I used the following to switch the option flag on and off:
wp-env run cli wp option update use_BalanceTags 1
wp-env run cli wp option update use_BalanceTags 0
✅ The experiment flag is no longer shown at /wp-admin/admin.php?page=gutenberg-experiments
✅ Updating the option correctly switches the feature on and off
If I create an image block-based gallery with use_BalanceTags set to 0, publish, and then set use_BalanceTags to 1 and update the post containing that block, then as expected, it'll cause a block validation issue. However, if we switch the setting back to 0 and click to attempt to recover the blocks, we can get the gallery block back to a working state. So, even if a site owner switches between settings and causes a validation issue, it's good to see they can still get their content back to a working state 👍
LGTM!
|
Should this check be eventually removed when 5.9 is released (with the patch from core)? Or perhaps another check could be made if the >5.9 is being used; to bypass this check? |
@skorasaurus this has been replaced by this check, but I think this old check may still be needed by some old versions of the mobile app - is that the case @mkevins ? |
My understanding is that the current code already includes the logic to accomplish what is described in the second part of the question (i.e. that the check is "bypassed" when >= 5.9). Regarding the older mobile app versions, there is quite a long tail for app version rollout, it turns out, so there are likely still a good number of users who's apps will use the editor settings endpoint to determine whether or not to enable the v2 editor (defaulting to There are some caveats to this, since we can't control the currently cached value that mobile users have, nor the timing of when the editor initializes (there is an inherent race, since the editor settings response updates the editor props only after the first parse). This comment describes the issue pretty well and the possible permutations which can result in data loss. That comment describes scenarios with a well defined REST API response, but does not consider if the API was removed. In such cases, I believe the editor would remain in its cached state, but I'm not certain about what will happen to the content. I believe that for some users, on more recent versions of the editor, we no longer support a graceful handling of existing content, so it is possible we could increase the likelihood of data loss by removing this endpoint (for example if users have their app cached with v2 disabled, but use the app to edit v2 content generated on web). To be clear, this possibility currently exists, but could typically be mitigated by the opening of a post (even one that has no gallery) which would then update the cached flag, preventing the issues on future edits to other posts which might include a gallery with inner blocks). Afaiu, removing the API could mean that the user's cache would remain "stale".. but I have not tested this, nor do I have a sense of the impact in terms of numbers to make a risk assessment. 🤷♂️ |
|
Thanks for the prompt responses: The reason I mentioned this is: While testing my site with 5.9 RC3 and the Gutenberg plugin 12.4.0 activated; and I was still being presented with the older prerefactored version of the gallery block. ( On the other hand, is_wp_version_compatible( '5.9' ) returns false despite running 5.9 RC3, contrary to my expectations, so perhaps that will change when 5.9 is officially released? For what it's worth, I've tested this in a plain "vanilla" environment (except I had modified ( |
|
Thanks for testing this @skorasaurus. I was able to replicate the same issue with 5.9 RC3, ie. with GB plugin installed and If I set the WP version to 5.9.0 on my install though I got the new Gallery block and was able to toggle |
Do not merge before release 11.8
Description
Currently the new gallery block structure can't be used on sites that have the
use_BalanceTagsoption set due to this issue - https://core.trac.wordpress.org/ticket/54130. It is likely that a patch for this won't be available until the 5.9 WP release, so rather than delaying removal of the experimental flag until then another option would be to disable the new gallery format on sites that have this flag set.This PR replaces the experimental flag with a check for
use_BalanceTagsTesting
use_BalanceTagsoption setuse_BalanceTagsoption to a string of '1'