Cover: Explicitly set isUserOverlayColor to false when media is updated #65105
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewserong
added
[Type] Bug
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
This was referenced Sep 6, 2024
|
Size Change: +25 B (0%) Total Size: 1.78 MB
ℹ️ View Unchanged
|
Mamaduka
approved these changes
Sep 6, 2024
There was a problem hiding this comment.
Thank you, @andrewserong! This fixes both issues 🎉
A tiny observation: When testing Cover blocks that were inserted before the fix, the overlay color stayed the same. I guess this is expected, since isUserOverlayColor: true and we have no way of knowing if it was set by a user or machine.
Cover Variations for testing
add_filter( 'get_block_type_variations', function( $variations, $block_type ) {
if ( 'core/cover' !== $block_type->name ) {
return $variations;
}
$variations[] = array(
'name' => 'cover-locked',
'title' => 'Cover Block Variation Locked',
'description' => 'Cover block variation with locked settings.',
'category' => 'media',
'icon' => 'smiley',
'scope' => array( 'inserter' ),
'active' => array( 'className' ),
'attributes' => array(
'templateLock' => 'contentOnly',
'useFeaturedImage' => true,
'dimRatio' => 0,
'backgroundType' => "image",
'overlayColor' => "",
'isUserOverlayColor' => true,
'contentPosition' => "center center",
'isDark' => false,
'className' => "is-cover-locked",
'tagName' => "div",
'textColor' => "white",
'layout' => array(
'type' => "constrained",
'contentSize' => ""
),
),
'innerBlocks' => array(
array(
'name' => 'core/heading',
'attributes' => array(
'level' => 1,
'placeholder' => "When reloading the page, the templateLock 'contentOnly' is lost.",
),
),
),
);
return $variations;
}, 10, 2 );
|
Thanks for the review! Since this implements the approach that @ajlende recommended, I'll merge this in now. Happy to follow up on Monday if there's any further feedback, but I think this approach should work pretty well.
Yes, unfortunately that is a limitation of the approach here. But at least for newly added Cover blocks we'll have the correct behaviour now 🙂 |
andrewserong
deleted the
try/explicitly-set-cover-block-is-user-overlay-color-to-false-on-media-selection
branch
This was referenced Sep 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Fixes #64702, fixes #64480
Update the logic in the Cover block to explicitly set
isUserOverlayColortofalsewhen the media is changed. Also, update the deprecation that setsisUserOverlayColortotrueto only do so if the attribute is not already set.Between these two changes, the issue linked issues above should be resolved. The main goal of this PR is to ensure that the Cover block's auto overlay color behaviour (where an overlay color is set when an image is added) continues to work even after a post or page reload.
Kudos: @ajlende for the suggestion of this fix in #65077 (comment)
Why?
Ensure that the Cover block's auto overlay color behaviour continues to work after reloading the editor, if the user has never manually set the overlay color for the Cover block.
Prior to this change, the deprecation was firing too frequently. Because the deprecation didn't check to see if
isUserOverlayColorwas already set, it would fire even when it didn't need to. Additionally, without settingisUserOverlayColorexplicitly tofalse, the deprecation would run on Cover blocks where a user had never manually interacted with the overlay color.How?
isUserOverlayColortofalseif it isn't already set totrue.isUserOverlayColorto only do so if the attribute has never been set to a value at all (isundefined).Testing Instructions
Follow the reproduction steps in #64702. Basically:
Also, for extra testing, see that the sample code in #64480 works correctly with this PR applied.
Screenshots or screencast
This screengrab demos the auto color overlay holding up beyond reloading the post editor. It only sticks once a user sets an overlay color deliberatly:
cover-block.mp4