Mobile - Gallery block - Fix bug when migrating from V1 #37889
Merged
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
…ixedHeight context, use of images prop instead of the attributes
|
Size Change: +10 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
mkevins
reviewed
Jan 12, 2022
There was a problem hiding this comment.
Hi Gerardo 👋 😄
One of the changes is to use images from the props within gallery.native.js. Before it was getting it from attributes but in some cases when saving after migrating it would be undefined other than that it would always return [].
Great work tracking this down so quickly and finding a solution!
I tested this via iPhone 11, physical device (both before and after the fix was applied) and confirmed that this fix works as described. 👍
Regarding the fixedHeight, I think it makes sense to fix it as you have, and that it is not necessary to add an additional deprecation (since it isn't making changes to the block format, but rather fixing a bug the migration code of an already existing format).
One other point I noticed, commenting here for posterity, is that the crash seemed to appear even when I didn't save the page, but just when leaving the editor. I very briefly saw the error, but it goes away quickly as the editor is being closed. Not sure if that will be a useful clue in the future or not 🤷♂️ .
mkevins
approved these changes
Jan 12, 2022
Thanks for confirming and thanks for testing/approving! |
2 tasks
mkevins
pushed a commit
that referenced
this pull request
Jan 17, 2022
…ixedHeight context, use of images prop instead of the attributes (#37889)
mkevins
added a commit
that referenced
this pull request
Jan 19, 2022
* Release script: Update react-native-editor version to 1.69.0 * Release script: Update with changes from 'npm run core preios' * Update native editor changelog for v1.69.0 * Release script: Update react-native-editor version to 1.69.1 * Release script: Update with changes from 'npm run core preios' * Mobile - Gallery block - Fix issue migrating old format and missing fixedHeight context, use of images prop instead of the attributes (#37889) * [Mobile] - RichText - Use parsed font size values when comparing new changes (#37951) * Mobile - RichText - Use parsed font size values when comparing new changes to avoid not matching values that generate a render loop * Mobile - E2E - Move typography test to full tests instead of canary * Mobile - RichText - Avoid using the fontSize value from the props if there's a font size set from the styles * Mobile - E2E - Remove test and move some test cases into the current integration tests * Mobile - RichTest - Update tests * Fix LinkPicker freeze when virtual keyboard is hidden (#37782) * Fix LinkPicker freeze when virtual keyboard is hidden When a devices virtual keyboard is hidden, e.g. when a hardware keyboard is connected, dismissing the `LinkPicker` resulted in the application freezing. The freeze likely originates from an unconsumed `LayoutAnimation` registered during resizing of the `BottomSheet`. To address this issue, we now avoid registering a `LayoutAnimation` whenever the changes to the header are sub-pixel values, which can result in the `LayoutAnimation` going unconsumed. https://git.io/J9q2G Long-term, we should likely consider refactoring the `BottomSheet` to holistically avoid stacking `LayoutAnimations`: https://git.io/J9q2l * Support unique BottomSheet header testID This allows individual tests to pass a unique, top-level testID for the BottomSheet and have it propagate to the header as well, which may make querying easier. Co-authored-by: Carlos Garcia <fluiddot@gmail.com> * Fix indentation issue in bottom sheet component * Add change log entry * Add pull request reference to change log entry Co-authored-by: Carlos Garcia <fluiddot@gmail.com> * Update release notes * Add back missing dep in package-lock Co-authored-by: Gerardo Pacheco <gerardo.pacheco@automattic.com> Co-authored-by: David Calhoun <438664+dcalhoun@users.noreply.github.com> Co-authored-by: Carlos Garcia <fluiddot@gmail.com>
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.
Description
There's a current bug on iOS only when saving an old version of the Gallery block it would generate a crash.
One of the changes is to use
imagesfrom the props withingallery.native.js. Before it was getting it fromattributesbut in some cases when saving after migrating it would beundefinedother than that it would always return[].The second change is updating the migration to include
fixedHeightwhich is used within theImageblock to show the size correctly. Without this, the image block wouldn't show the image and the expected size (When migrating). Here I'm not sure if we can update the latest version of the migration or if we should create a new one 🤔 what do you think @mkevins?How has this been tested?
UpdateScreenshots
Types of changes
Bug fix
Checklist:
*.native.jsfiles for terms that need renaming or removal).