Skip to content
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

[RN Mobile] Unsupported block bottom sheet is triggered when device is rotated #13052 #25873

Merged
merged 2 commits into from
Oct 12, 2020
Merged

[RN Mobile] Unsupported block bottom sheet is triggered when device is rotated #13052 #25873

merged 2 commits into from
Oct 12, 2020

Conversation

marecar3
Copy link
Contributor

@marecar3 marecar3 commented Oct 6, 2020 •
edited
Loading

Fixes: wordpress-mobile/WordPress-Android#13052

Gutenberg Mobile PR: wordpress-mobile/gutenberg-mobile#2710

How has this been tested?

  1. Open the block editor on a post that contains an unsupported block (e.g. Audio is a block that's not yet supported on mobile at time of writing)
  2. With the editor open, rotate the device and notice the bottom sheet should not appear

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.

@marecar3 marecar3 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 Oct 6, 2020
@marecar3 marecar3 requested review from guarani and etoledom October 6, 2020 19:46
@marecar3 marecar3 self-assigned this Oct 6, 2020
@github-actions
Copy link

github-actions bot commented Oct 6, 2020 •
edited
Loading

Size Change: 0 B

Total Size: 1.19 MB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.52 kB 0 B
build/api-fetch/index.js 3.35 kB 0 B
build/autop/index.js 2.72 kB 0 B
build/blob/index.js 668 B 0 B
build/block-directory/index.js 8.55 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 130 kB 0 B
build/block-editor/style-rtl.css 11 kB 0 B
build/block-editor/style.css 11 kB 0 B
build/block-library/editor-rtl.css 8.65 kB 0 B
build/block-library/editor.css 8.65 kB 0 B
build/block-library/index.js 145 kB 0 B
build/block-library/style-rtl.css 7.66 kB 0 B
build/block-library/style.css 7.66 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-default-parser/index.js 1.78 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.6 kB 0 B
build/components/index.js 169 kB 0 B
build/components/style-rtl.css 15.5 kB 0 B
build/components/style.css 15.5 kB 0 B
build/compose/index.js 9.43 kB 0 B
build/core-data/index.js 12 kB 0 B
build/data-controls/index.js 685 B 0 B
build/data/index.js 8.6 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 4.42 kB 0 B
build/edit-navigation/index.js 10.6 kB 0 B
build/edit-navigation/style-rtl.css 868 B 0 B
build/edit-navigation/style.css 871 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.29 kB 0 B
build/edit-post/style.css 6.28 kB 0 B
build/edit-site/index.js 20.9 kB 0 B
build/edit-site/style-rtl.css 3.73 kB 0 B
build/edit-site/style.css 3.73 kB 0 B
build/edit-widgets/index.js 21.2 kB 0 B
build/edit-widgets/style-rtl.css 3.02 kB 0 B
build/edit-widgets/style.css 3.02 kB 0 B
build/editor/editor-styles-rtl.css 492 B 0 B
build/editor/editor-styles.css 493 B 0 B
build/editor/index.js 45.5 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.84 kB 0 B
build/element/index.js 4.45 kB 0 B
build/escape-html/index.js 734 B 0 B
build/format-library/index.js 7.49 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 1.74 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.54 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.39 kB 0 B
build/keycodes/index.js 1.85 kB 0 B
build/list-reusable-blocks/index.js 3.02 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.13 kB 0 B
build/notices/index.js 1.69 kB 0 B
build/nux/index.js 3.27 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.44 kB 0 B
build/primitives/index.js 1.34 kB 0 B
build/priority-queue/index.js 790 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13 kB 0 B
build/server-side-render/index.js 2.6 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.24 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.74 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@marecar3 marecar3 requested a review from mchowning October 9, 2020 20:51
@marecar3 marecar3 mentioned this pull request Oct 12, 2020
Merged
2 tasks
@marecar3 marecar3 removed their assignment Oct 12, 2020
mchowning
mchowning approved these changes Oct 12, 2020
View reviewed changes
Copy link
Contributor

@mchowning mchowning 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 both WPAndroid and WPiOS, and this is working well for me, but let's wait to hear what @guarani thinks before merging this because I know he's testing this as well.

It certainly makes it a bit confusing to have Android and iOS handle the BottomSheet differently during rotation. From what I can see it seems like Android calls the onClose function during rotation, whereas iOS does not. Just wanted to note that here for future me. 🙂

guarani
guarani reviewed Oct 12, 2020
View reviewed changes
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

I left a comment, but this is working (tested on Android emulator).

@guarani guarani self-requested a review October 12, 2020 19:49
guarani
guarani approved these changes Oct 12, 2020
View reviewed changes
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

@marecar3 and I agreed #25873 (comment) will be best left to a future PR, and fixed along with a similar issue here: #25265 (comment)

@marecar3
Copy link
Contributor Author

Tested via both WPAndroid and WPiOS, and this is working well for me, but let's wait to hear what @guarani thinks before merging this because I know he's testing this as well.

It certainly makes it a bit confusing to have Android and iOS handle the BottomSheet differently during rotation. From what I can see it seems like Android calls the onClose function during rotation, whereas iOS does not. Just wanted to note that here for future me. 🙂

Yes and it could be a reason that maybe iOS doesn't recreate the View on rotation.

@marecar3 marecar3 merged commit cfb7922 into master Oct 12, 2020
@marecar3 marecar3 deleted the rnmobile/unsupported_block_bottom_sheet_is_triggered_when_device_is_rotated branch October 12, 2020 20:26
@github-actions github-actions bot added this to the Gutenberg 9.2 milestone Oct 12, 2020
ceyhun pushed a commit that referenced this pull request Oct 13, 2020
@marecar3 @ceyhun
Fix rotation issue (#25873)
0b11e6b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mchowning mchowning mchowning approved these changes

@guarani guarani guarani approved these changes

@etoledom etoledom Awaiting requested review from etoledom

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 9.2
Development

Successfully merging this pull request may close these issues.

Unsupported block bottom sheet is triggered when device is rotated
3 participants
@marecar3 @guarani @mchowning