[RNMobile] Ignore column width attribute when empty #29015
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
The editor was crashing when attempting to render a column block which contained an empty width attribute (i.e `<!-- wp:column {"width":""} -->`.
The exception occurred when accessing a property of an undefined object, so adding a default object (`|| {}`) fixes this by allowing the property to be accessed.
guarani
force-pushed
the
rnmobile/ignore-empty-column-width-attribute
branch
from
81b570a to
10eba54
Compare
guarani
added
Mobile App - i.e. Android or iOS
|
Size Change: 0 B Total Size: 1.38 MB ℹ️ View Unchanged
|
2 tasks
lukewalczak
requested changes
Feb 16, 2021
There was a problem hiding this comment.
Hello @guarani, thanks for fixing the crash! I've spotted one more bug which is related to missing slider value and sliders are not displayed in Columns settings:
I think that correction should assume a fallback value (50) in the line:
value={ getWidths( innerWidths )[ index ] || 50 }
Could you please check that and handle it?
|
👋 Since this is blocking the release I'm going to handle over it until Paul comes online. |
I'm afraid that doesn't fix it, I'm testing different cases and looks like there're other parts that might be affecting, I'll keep investigating. For example if you switch to HTML Mode and then back to Visual mode, the sliders are not shown again. |
|
The correction The problem was related to a missing dependency for memoizing I added that prop as a dependency in @lukewalczak let me know if this fixed the issue you spotted, thanks! |
lukewalczak
approved these changes
Feb 16, 2021
There was a problem hiding this comment.
The problem was related to a missing dependency for memoizing useSelect, the prop editorSidebarOpened was returning always false in some cases because the isSelect prop, that is used for calculating that value (line 476), wasn't being updated.
Checked the file history and it looks like the missing isSelected is the result of an oversight from the previous PR. Good catch! Thanks for the fix and explanation.
@lukewalczak let me know if this fixed the issue you spotted, thanks!
It works properly again 🎉
Yeah I think so too, Thanks for the review! |
fluiddot
changed the title
fluiddot
pushed a commit
that referenced
this pull request
Feb 16, 2021
|
Thank you @lukewalczak and @fluiddot for getting this merged! |
dcalhoun
pushed a commit
that referenced
this pull request
Feb 16, 2021
* Ignore empty width attribute in columns
The editor was crashing when attempting to render a column block which contained an empty width attribute (i.e `<!-- wp:column {"width":""} -->`.
The exception occurred when accessing a property of an undefined object, so adding a default object (`|| {}`) fixes this by allowing the property to be accessed.
* Update react-native-editor changelog
* Add missing dependency to useSelect in columns block
Co-authored-by: Carlos Garcia <[email protected]>
3 tasks
3 tasks
fluiddot
added a commit
that referenced
this pull request
Feb 17, 2021
* Release script: Update react-native-editor version to 1.46.1 * Release script: Update with changes from 'npm run core preios' * Update react-native-editor CHANGELOG * [RNMobile] Add capability to bridge to show/hide audio block (#28952) * [RNMobile] Ignore column width attribute when empty (#29015) Co-authored-by: Ceyhun Ozugur <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]>
fluiddot
added a commit
that referenced
this pull request
Feb 17, 2021
* Release script: Update react-native-editor version to 1.46.1 * Release script: Update with changes from 'npm run core preios' * Update react-native-editor CHANGELOG * [RNMobile] Add capability to bridge to show/hide audio block (#28952) * [RNMobile] Ignore column width attribute when empty (#29015) * Update react-native-editor CHANGELOG * Fixes minor changelog alignement issue * Add Stories bridge methods for iOS (#29083) Add missing bridge methods for the Stories block. The absence of these threw an error, even though Stories block isn't fully supported for iOS just yet. Co-authored-by: Ceyhun Ozugur <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: Antonis Lilis <[email protected]> Co-authored-by: David Calhoun <[email protected]>
fluiddot
added a commit
that referenced
this pull request
Feb 18, 2021
* Release script: Update react-native-editor version to 1.46.1 * Release script: Update with changes from 'npm run core preios' * Update react-native-editor CHANGELOG * [RNMobile] Add capability to bridge to show/hide audio block (#28952) * [RNMobile] Ignore column width attribute when empty (#29015) * Release script: Update react-native-editor version to 1.46.2 * Release script: Update with changes from 'npm run core preios' * Enable mediaFilesBlockReplaceSync RN bridge function only on Android * Add missing arguments in RN bridge function on iOS * Change mediaFiles from string to a dictionary (#28766) Co-authored-by: Ceyhun Ozugur <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: Brandon Titus <[email protected]>
dcalhoun
pushed a commit
that referenced
this pull request
Feb 18, 2021
fluiddot
added a commit
that referenced
this pull request
Feb 19, 2021
* Release script: Update react-native-editor version to 1.46.1 * Release script: Update with changes from 'npm run core preios' * Update react-native-editor CHANGELOG * [RNMobile] Add capability to bridge to show/hide audio block (#28952) * [RNMobile] Ignore column width attribute when empty (#29015) Co-authored-by: Ceyhun Ozugur <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> # Conflicts: # packages/react-native-editor/CHANGELOG.md
fluiddot
added a commit
that referenced
this pull request
Feb 19, 2021
* Release script: Update react-native-editor version to 1.46.1 * Release script: Update with changes from 'npm run core preios' * Update react-native-editor CHANGELOG * [RNMobile] Add capability to bridge to show/hide audio block (#28952) * [RNMobile] Ignore column width attribute when empty (#29015) * Release script: Update react-native-editor version to 1.46.2 * Release script: Update with changes from 'npm run core preios' * Enable mediaFilesBlockReplaceSync RN bridge function only on Android * Add missing arguments in RN bridge function on iOS * Change mediaFiles from string to a dictionary (#28766) Co-authored-by: Ceyhun Ozugur <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: Brandon Titus <[email protected]>
dcalhoun
added a commit
that referenced
this pull request
Feb 19, 2021
* Release script: Update react-native-editor version to 1.47.0 * Release script: Update with changes from 'npm run core preios' * [RNMobile] Ignore column width attribute when empty (#29015) * Ignore empty width attribute in columns The editor was crashing when attempting to render a column block which contained an empty width attribute (i.e `<!-- wp:column {"width":""} -->`. The exception occurred when accessing a property of an undefined object, so adding a default object (`|| {}`) fixes this by allowing the property to be accessed. * Update react-native-editor changelog * Add missing dependency to useSelect in columns block Co-authored-by: Carlos Garcia <[email protected]> * Update editor changelog * Mobile - RichText - Restore onSelectionChange when its focused (#29074) * Adds null check before function call * [RNMobile] Merge 1.46.1 beta fix release to 1.47.0 (#29044) * Release script: Update react-native-editor version to 1.46.1 * Release script: Update with changes from 'npm run core preios' * Update react-native-editor CHANGELOG * [RNMobile] Add capability to bridge to show/hide audio block (#28952) * [RNMobile] Ignore column width attribute when empty (#29015) * Update react-native-editor CHANGELOG * Fixes minor changelog alignement issue * Add Stories bridge methods for iOS (#29083) Add missing bridge methods for the Stories block. The absence of these threw an error, even though Stories block isn't fully supported for iOS just yet. Co-authored-by: Ceyhun Ozugur <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: Antonis Lilis <[email protected]> Co-authored-by: David Calhoun <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: Carlos Garcia <[email protected]> Co-authored-by: David Calhoun <[email protected]> Co-authored-by: Gerardo Pacheco <[email protected]> Co-authored-by: Ceyhun Ozugur <[email protected]>
3 tasks
fluiddot
added a commit
that referenced
this pull request
Mar 10, 2021
* Release script: Update react-native-editor version to 1.47.0 * Release script: Update with changes from 'npm run core preios' * [RNMobile] Ignore column width attribute when empty (#29015) * Ignore empty width attribute in columns The editor was crashing when attempting to render a column block which contained an empty width attribute (i.e `<!-- wp:column {"width":""} -->`. The exception occurred when accessing a property of an undefined object, so adding a default object (`|| {}`) fixes this by allowing the property to be accessed. * Update react-native-editor changelog * Add missing dependency to useSelect in columns block Co-authored-by: Carlos Garcia <[email protected]> * Update editor changelog * Mobile - RichText - Restore onSelectionChange when its focused (#29074) * Adds null check before function call * [RNMobile] Merge 1.46.1 beta fix release to 1.47.0 (#29044) * Release script: Update react-native-editor version to 1.46.1 * Release script: Update with changes from 'npm run core preios' * Update react-native-editor CHANGELOG * [RNMobile] Add capability to bridge to show/hide audio block (#28952) * [RNMobile] Ignore column width attribute when empty (#29015) * Update react-native-editor CHANGELOG * Fixes minor changelog alignement issue * Add Stories bridge methods for iOS (#29083) Add missing bridge methods for the Stories block. The absence of these threw an error, even though Stories block isn't fully supported for iOS just yet. Co-authored-by: Ceyhun Ozugur <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: Antonis Lilis <[email protected]> Co-authored-by: David Calhoun <[email protected]> * Release script: Update react-native-editor version to 1.47.1 * Release script: Update with changes from 'npm run core preios' * Change the maximum items per page of reusable block fetch Co-authored-by: Antonis Lilis <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: David Calhoun <[email protected]> Co-authored-by: Gerardo Pacheco <[email protected]> Co-authored-by: Ceyhun Ozugur <[email protected]>
fluiddot
added a commit
that referenced
this pull request
Mar 12, 2021
* Release script: Update react-native-editor version to 1.47.0 * Release script: Update with changes from 'npm run core preios' * [RNMobile] Ignore column width attribute when empty (#29015) * Ignore empty width attribute in columns The editor was crashing when attempting to render a column block which contained an empty width attribute (i.e `<!-- wp:column {"width":""} -->`. The exception occurred when accessing a property of an undefined object, so adding a default object (`|| {}`) fixes this by allowing the property to be accessed. * Update react-native-editor changelog * Add missing dependency to useSelect in columns block Co-authored-by: Carlos Garcia <[email protected]> * Update editor changelog * Mobile - RichText - Restore onSelectionChange when its focused (#29074) * Adds null check before function call * [RNMobile] Merge 1.46.1 beta fix release to 1.47.0 (#29044) * Release script: Update react-native-editor version to 1.46.1 * Release script: Update with changes from 'npm run core preios' * Update react-native-editor CHANGELOG * [RNMobile] Add capability to bridge to show/hide audio block (#28952) * [RNMobile] Ignore column width attribute when empty (#29015) * Update react-native-editor CHANGELOG * Fixes minor changelog alignement issue * Add Stories bridge methods for iOS (#29083) Add missing bridge methods for the Stories block. The absence of these threw an error, even though Stories block isn't fully supported for iOS just yet. Co-authored-by: Ceyhun Ozugur <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: Antonis Lilis <[email protected]> Co-authored-by: David Calhoun <[email protected]> * Release script: Update react-native-editor version to 1.48.0 * Release script: Update with changes from 'npm run core preios' * [Mobile] - Fix splitting/merging of Paragraph and Heading (#29502) * Wip: Mobile RichText - Updating old value after splitting * Mobile - Fix splitting/merging issues and keyboard jumpiness on Android * Mobile - RichText - Add isIOS check for componentDidUpdate and use blockEditorStore * Mobile - RichText - Prevent onTextUpdate on Android * Update changelog * Changelog - fix typo * Revert "[Mobile] - Fix splitting/merging of Paragraph and Heading (#29502)" This reverts commit a14915f. * Revert "Rich text: keep block ID on split (#28505)" This reverts commit 4b9d13f. * Release script: Update react-native-editor version to 1.47.1 * Release script: Update with changes from 'npm run core preios' * Change the maximum items per page of reusable block fetch * Release script: Update react-native-editor version to 1.48.1 * Release script: Update with changes from 'npm run core preios' * Change the maximum items per page of reusable block fetch * Add replace block content by clientID * Add item to release notes about `replaceBlock` method * Revert "Revert "Rich text: keep block ID on split (#28505)"" This reverts commit 956cdfc. Co-authored-by: Antonis Lilis <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: David Calhoun <[email protected]> Co-authored-by: Gerardo Pacheco <[email protected]> Co-authored-by: Ceyhun Ozugur <[email protected]> Co-authored-by: Enej Bajgoric <[email protected]> Co-authored-by: Brandon Titus <[email protected]>
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
Fixes #29017
The editor was crashing when attempting to render a column block which contained an empty width attribute (i.e
<!-- wp:column {"width":""} -->.The exception occurred when accessing a property of an undefined object, so adding a default object (
|| {}) fixes this by allowing the property to be accessed.How has this been tested?
I tested by replacing the editor contents with the HTML snippet that caused the error:
I then smoke tested the Columns block, adding columns and changing their widths and looked there were no regressions.
Types of changes
Bug fix
Checklist: