[RNMobile] Fix crash when editing a component that uses RangeControl #27987

fluiddot · 2021-01-05T10:18:37Z

Description

The crash was caused because RangeCell (the native component used in RangeControl) was calling the onComplete prop but not all the components that use it pass this handler. Since not all the components require this handler I changed it to be optional.

Looks like the bug was introduced in the Full width columns PR, I checked that some components were updated but only the ones that use the UnitControl component which is the other one that uses RangeCell underneath.

How has this been tested?

Add a spacer block
Tap settings gear
Change spacing via text input

This can also be tested with other components that have a slider and an input box for editing block's settings, like:

Buttons block (Border radius setting)
Columns block (Column width setting)

Screenshots

iPhone 8 simulator

edit-spacer-block-ios.mp4

Pixel 4 emulator

edit-spacer-block-android.mp4

Types of changes

Bug fix

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

Update `master` branch from original repo.

Update master branch from original repo

Update master branch from original repo.

Update master branch from original repo

Update master branch from original repo.

Update master branch from original repo

fluiddot · 2021-01-05T10:23:57Z

packages/components/src/mobile/bottom-sheet/range-cell.native.js

+		if ( onComplete ) {
+			onComplete( nextValue );
+		}


My first attempt was to pass the handler in the Spacer block but then I realised that it would require to add it in more blocks so a better solution was to make it optional.

dratwas

Tested it on iOS and Android and everything works well. Thanks, @fluiddot for this PR!

geriux

LGTM! Thanks for fixing it!

guarani · 2021-01-05T14:54:26Z

Re-ran failing test 🤞

Update master branch from original repo

Range cell component is used in multiple components but not all of them require the onComplete handler so it should be optional.

* Release script: Update react-native-editor version to 1.44.0 * Release script: Update with changes from 'npm run core preios' * Mobile - Link settings - Fix data not being saved when closing the bottom sheet (#27997) * Set onComplete handler as optional in Range cell component (#27987) Range cell component is used in multiple components but not all of them require the onComplete handler so it should be optional. * Mobile - Link settings - Check for onClose (#28039) * Fix: Add Image style settings for hotlinked images (#28006) * Update CHANGELOG.md (#28042) * Change spaces back to tabs Co-authored-by: Gerardo Pacheco <[email protected]> Co-authored-by: Carlos Garcia <[email protected]> Co-authored-by: Enej Bajgoric <[email protected]>

* Release script: Update react-native-editor version to 1.44.0 * Release script: Update with changes from 'npm run core preios' * Mobile - Link settings - Fix data not being saved when closing the bottom sheet (#27997) * Set onComplete handler as optional in Range cell component (#27987) Range cell component is used in multiple components but not all of them require the onComplete handler so it should be optional. * Mobile - Link settings - Check for onClose (#28039) * Fix: Add Image style settings for hotlinked images (#28006) * Update CHANGELOG.md (#28042) * Change spaces back to tabs * Release script: Update react-native-editor version to 1.44.1 * Release script: Update with changes from 'npm run core preios' * [RNMobile] Fix crash in mobile paragraph blocks with custom font size (#28121) * Remove px suffix from fontSize for Android paragraph * Issue does not seem to be limited to android on latest gutenberg master * Move fontSize char removal to richtext to account for multiple block errors * Add move to top bottom when long pressing block movers (#27554) * WC: Added move to top and bottom functionality via long pressing BlockMover buttons and clicking the button on the BottomSheet Picker. * WIP: Unit tests. * WC: WIP unit tests for add move to top and bottom * WC: Moving UI tests to gutenberg with other UI tests instead of keeping at gutenberg-mobile. Adding more convenience functions to EditorPage. * WIP: Unit tests. * WC: Adding icon and title to picker for move to top/bottom. * WC: Cleaning up code. * WC: Fixing merge issue where import was changed to const. * WC: Changing all imports to requires as per jest bug. * WC: Removing before/afterAll blocks from UI tests. * WC: Removing e2e tests to issue-1191-add-move-to-top-bottom-ui-tests branch. * WC: Updating imports to fall in line with other files. Co-authored-by: Wendy Chen <[email protected]> * Fixed crash introduced from recent fontSize fix (#28209) * Updated changelog Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: Gerardo Pacheco <[email protected]> Co-authored-by: Carlos Garcia <[email protected]> Co-authored-by: Enej Bajgoric <[email protected]> Co-authored-by: illusaen <[email protected]> Co-authored-by: Wendy Chen <[email protected]>

fluiddot added 8 commits September 18, 2020 19:42

Merge pull request #3 from WordPress/master

1ea8f94

Update `master` branch from original repo.

Merge pull request #4 from WordPress/master

b3e7e51

Update master branch from original repo

Merge pull request #5 from WordPress/master

80b6df5

Update master branch from original repo.

Merge pull request #6 from WordPress/master

aadd803

Update master branch from original repo

Merge pull request #7 from WordPress/master

351a225

Update master branch from original repo.

Merge pull request #8 from WordPress/master

ff8d204

Update master branch from original repo

Merge pull request #9 from WordPress/master

8458b75

Update master branch from original repo

Merge pull request #10 from WordPress/master

77abb11

Update master branch from original repo

github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Jan 5, 2021

geriux added the Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) label Jan 5, 2021

geriux requested review from geriux and ceyhun January 5, 2021 10:22

fluiddot commented Jan 5, 2021

View reviewed changes

dratwas reviewed Jan 5, 2021

View reviewed changes

geriux approved these changes Jan 5, 2021

View reviewed changes

fluiddot added 2 commits January 5, 2021 16:15

Merge pull request #11 from WordPress/master

1b39fe3

Update master branch from original repo

Set onComplete handler as optional in Range cell component

fa94310

Range cell component is used in multiple components but not all of them require the onComplete handler so it should be optional.

gziolo merged commit 58d0ddf into WordPress:master Jan 5, 2021

github-actions bot added this to the Gutenberg 9.8 milestone Jan 5, 2021

fluiddot deleted the rnmobile/fix/range-cell-oncomplete-optional branch January 5, 2021 15:54

guarani pushed a commit that referenced this pull request Jan 5, 2021

Set onComplete handler as optional in Range cell component (#27987)

1eda213

Range cell component is used in multiple components but not all of them require the onComplete handler so it should be optional.

guarani mentioned this pull request Jan 5, 2021

Release 1.44.0 wordpress-mobile/gutenberg-mobile#2957

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RNMobile] Fix crash when editing a component that uses RangeControl #27987

[RNMobile] Fix crash when editing a component that uses RangeControl #27987

fluiddot commented Jan 5, 2021 •

edited

Loading

fluiddot Jan 5, 2021

dratwas left a comment

geriux left a comment

guarani commented Jan 5, 2021

[RNMobile] Fix crash when editing a component that uses RangeControl #27987

[RNMobile] Fix crash when editing a component that uses RangeControl #27987

Conversation

fluiddot commented Jan 5, 2021 • edited Loading

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

fluiddot Jan 5, 2021

Choose a reason for hiding this comment

dratwas left a comment

Choose a reason for hiding this comment

geriux left a comment

Choose a reason for hiding this comment

guarani commented Jan 5, 2021

fluiddot commented Jan 5, 2021 •

edited

Loading