[RNMobile][Embed block] Fix inline preview cut-off when editing URL #35321
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
fluiddot
added
[Type] Bug
|
Size Change: +86 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
Merged
2 tasks
fluiddot
changed the title
fluiddot
changed the title
jd-alexander
reviewed
Oct 5, 2021
jd-alexander
approved these changes
Oct 5, 2021
There was a problem hiding this comment.
@fluiddot I tested the changes here and I noticed the cut-off issue on develop and I can confirm that switching to this branch and doing the tests resulted in the expected styles being applied. I left a minor comment/inquiry but it's not a blocker. LGTM 🚢
Thanks @jd-alexander for the review ❤️ ! I finally applied your suggestion in this commit so I'd appreciate it if you could take a quick look before merging 🙇 . Additionally, I changed the value used when ignoring the previous classname from an empty string to |
No prob 🙇🏾
Looking good!
Ah, that makes sense! 👍🏾 |
7 tasks
5 tasks
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.
Gutenberg Mobile PR: wordpress-mobile/gutenberg-mobile#4072
Description
This PR changes the calculation of attributes when handling an incoming preview after editing the URL of an embed block. Previously, the attributes were obtained from the
getMergedAttributesfunction, however, this function could return the classname from the previous embed, which depending on the new embed could lead to rendering issues, as the cut-off mentioned in wordpress-mobile/gutenberg-mobile#4059.These rendering issues are caused when the previous embed adds the aspect ratio classes to the classname attribute, then after setting a different URL, the new embed expects to have that attribute empty. In the following code references, you can see the flow of how the classname is calculated when handling an incoming preview:
gutenberg/packages/block-library/src/embed/edit.native.js
Line 184 in c357fc7
gutenberg/packages/block-library/src/embed/edit.native.js
Lines 137 to 143 in c357fc7
gutenberg/packages/block-library/src/embed/util.js
Lines 243 to 260 in c357fc7
gutenberg/packages/block-library/src/embed/util.js
Lines 284 to 288 in c357fc7
gutenberg/packages/block-library/src/embed/util.js
Lines 177 to 227 in c357fc7
The calculation is now done in a similar way as the
getMergedAttributesfunction but passing an empty classname ingetAttributesFromPreview. This way we assure that the classname attribute set to the block doesn't preserve the value from the previous embed.How has this been tested?
NOTE: This can be also reproduced by using a Twitter embed in step 5, although the rendering issue is an unexpected extra padding at the top instead of cut-off.
Screenshots
embed-block-cutoff-issue-before.mp4
embed-block-cutoff-issue-after.mp4
Types of changes
Bug fix
Checklist:
*.native.jsfiles for terms that need renaming or removal).