Skip to content

BottomSheet listens to native bridge for closed modals on Android #20860

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.

Sign up for GitHub

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

Jump to bottom
Merged
cameronvoell merged 6 commits into from
Mar 25, 2020
Merged

BottomSheet listens to native bridge for closed modals on Android #20860

cameronvoell merged 6 commits into from
Mar 25, 2020

Conversation

@cameronvoell
Copy link
Member

@cameronvoell cameronvoell commented Mar 13, 2020 •
edited
Loading

Fixes: wordpress-mobile/WordPress-Android#10227

Related PRs:

GB-Mobile: wordpress-mobile/gutenberg-mobile#2011
WordPress-Android: wordpress-mobile/WordPress-Android#10383

Description

When the Gutenberg editor on Android is contained in a Fragment, there are cases on device orientation change where the native Android ReactInstanceManager will dismiss open Modals (Dialog in Android Native code) in order to avoid a crash.

This PR allows our Modal utilizing BottomSheet component to subscribe to messages from the native Android bridge that notify when a Modal is dismissed from native Android, and we'll perform the necessary React Native state updates accordingly by calling the BottomSheet onClose method. Failure to call onClose method in this instance results in non functioning buttons for opening modals, because React Native state thinks they are still open, hence this change.

How has this been tested?

Tested in WordPress Android app, as well as in the Android/iOS example apps. See steps and screenshot below.

Testing steps (same apply for opening the inserter Modal, or clicking insert Image/Video to Open a modal)

  1. Add an Image block and add an image from device or media library.
  2. While the image block is selected, press the "gear" button to open up the Image Settings.
  3. Rotate the Device.
  4. Click the gear button again, the Modal should reopen as normal.
  5. Rotate the Device again => the app should behave normally without crashing.

Screenshots

not-crashing-rotate-modal-android

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.

@github-actions
Copy link

github-actions bot commented Mar 13, 2020 •
edited
Loading

Size Change: 0 B

Total Size: 859 kB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 998 B 0 B
build/annotations/index.js 3.43 kB 0 B
build/api-fetch/index.js 3.39 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.02 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/index.js 102 kB 0 B
build/block-editor/style-rtl.css 10.9 kB 0 B
build/block-editor/style.css 10.9 kB 0 B
build/block-library/editor-rtl.css 7.24 kB 0 B
build/block-library/editor.css 7.24 kB 0 B
build/block-library/index.js 110 kB 0 B
build/block-library/style-rtl.css 7.43 kB 0 B
build/block-library/style.css 7.44 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.5 kB 0 B
build/components/index.js 191 kB 0 B
build/components/style-rtl.css 15.8 kB 0 B
build/components/style.css 15.7 kB 0 B
build/compose/index.js 6.21 kB 0 B
build/core-data/index.js 10.6 kB 0 B
build/data-controls/index.js 1.04 kB 0 B
build/data/index.js 8.25 kB 0 B
build/date/index.js 5.37 kB 0 B
build/deprecated/index.js 771 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.06 kB 0 B
build/edit-post/index.js 91.2 kB 0 B
build/edit-post/style-rtl.css 8.47 kB 0 B
build/edit-post/style.css 8.46 kB 0 B
build/edit-site/index.js 6.72 kB 0 B
build/edit-site/style-rtl.css 2.88 kB 0 B
build/edit-site/style.css 2.88 kB 0 B
build/edit-widgets/index.js 4.43 kB 0 B
build/edit-widgets/style-rtl.css 2.58 kB 0 B
build/edit-widgets/style.css 2.58 kB 0 B
build/editor/editor-styles-rtl.css 428 B 0 B
build/editor/editor-styles.css 431 B 0 B
build/editor/index.js 43.8 kB 0 B
build/editor/style-rtl.css 4 kB 0 B
build/editor/style.css 3.98 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 6.95 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 1.93 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.49 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.69 kB 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 4.84 kB 0 B
build/notices/index.js 1.57 kB 0 B
build/nux/index.js 3.01 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 781 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.4 kB 0 B
build/server-side-render/index.js 2.55 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.01 kB 0 B
build/viewport/index.js 1.61 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

@cameronvoell cameronvoell force-pushed the rnmobile/fix-android-crash-modal-open-on-rotate branch from 4773b72 to 88dbc38 Compare March 13, 2020 21:24
@cameronvoell cameronvoell force-pushed the rnmobile/fix-android-crash-modal-open-on-rotate branch from 88dbc38 to 521aff3 Compare March 13, 2020 21:28
@cameronvoell cameronvoell changed the title Bridge notifies BottomSheet of closed modals BottomSheet listens to native bridge for closed modals on Android Mar 13, 2020
@cameronvoell cameronvoell mentioned this pull request Mar 14, 2020
Merged
2 tasks
@mkevins mkevins mentioned this pull request Mar 14, 2020
Merged
1 task
@cameronvoell cameronvoell marked this pull request as ready for review March 14, 2020 02:31
mkevins
mkevins approved these changes Mar 17, 2020
View reviewed changes
Copy link
Contributor

@mkevins mkevins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested via steps here and it's working as described. Nice work Cameron! 👍

I only left a minor comment about a variable name, as a nitpick, but not a blocker. 😃

@cameronvoell cameronvoell force-pushed the rnmobile/fix-android-crash-modal-open-on-rotate branch from e9cd0da to 8a0b989 Compare March 25, 2020 01:37
@cameronvoell cameronvoell merged commit 3d676ed into master Mar 25, 2020
@cameronvoell cameronvoell deleted the rnmobile/fix-android-crash-modal-open-on-rotate branch March 25, 2020 04:31
@github-actions github-actions bot added this to the Gutenberg 7.9 milestone Mar 25, 2020
@cameronvoell cameronvoell 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 Mar 25, 2020
@geriux geriux mentioned this pull request Mar 31, 2020
Merged
6 tasks
@designsimply designsimply mentioned this pull request May 19, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mkevins mkevins mkevins approved these changes

@hypest hypest Awaiting requested review from hypest

@mchowning mchowning Awaiting requested review from mchowning

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

Gutenberg 7.9

Development

Successfully merging this pull request may close these issues.

Crash in block editor on double rotation when inserter is open

3 participants

@cameronvoell @mkevins