-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Cover Block: Fix regressions #36406
Cover Block: Fix regressions #36406
Conversation
Size Change: +736 B (0%) Total Size: 1.09 MB
ℹ️ View Unchanged
|
dd3bb71
to
ee4aee0
Compare
Hi, @WunderBart I see block validation errors when testing this branch with the TT2 theme. Screenshot |
I think this is due to this change |
…ndefaulting the backgroundType attribute
@WunderBart I pushed a change that adds a check for |
…tribute defaults and setting in useEffect
@WunderBart I noticed when trying to sort the fixtures that the removal of the default for |
Just retesting this after @glendaviesnz's change in 65b5290 The cover block works as expected. Fixture tests are passing for me, and also pasting in deprecated Cover Block code migrates correctly. Transforming from an Image > Cover Block adds a default dimRatio of Creating a Cover Block with media, i.e., an image or video, sets the dimRatio to With a color background, it sets the dimRatio to |
Oh, this looks much better now, thanks! It does seem to cover all the testing scenarios and we don't need to regenerate the fixtures 💪 @ramonjd thanks for testing! It seems we need another approval to unlock this PR though 🙏 |
@glendaviesnz one small issue that I've noticed - the title color is not properly set initially when setting a video as a Cover background. Once the opacity slider is moved, it seems to be detecting and seting the correct value though: Screen.Recording.2021-11-16.at.13.32.34.movShould we leave this PR as is and address this issue in a follow-up PR? Let me know what you think! |
@WunderBart lets make this a separate issue - #36549 - it seems related to the third party I will get someone to give us a final review and sign off on this today. |
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.
* Set default dim to 50% for media and 100% for solids Co-authored-by: Glen Davies <glen.davies@a8c.com>
This PR was cherry picked into the GB 11.9.1 point release in c51a153 |
* Set default dim to 50% for media and 100% for solids Co-authored-by: Glen Davies <glen.davies@a8c.com>
Description
This PR fixes the following Cover block regressions:
Fixes situations where Cover blocks published pre #35065 lost their background dim.
Fixes situations where an Image transformed into Cover gets a 100% opaque overlay by default.
Updated tests:
Updated failing Cover block fixtures(see Cover Block: Fix regressions #36406 (comment))How has this been tested?
The following scenarios for the Cover block should be verified:
git checkout v11.7.1
,git checkout trunk
,git checkout fix/cover-block-regression
,Types of changes
Bug fix
Checklist:
*.native.js
files for terms that need renaming or removal).