Skip to content

Increase tap-target of primary action on unsupported blocks#25209

Merged
marecar3 merged 6 commits intomasterfrom
rnmobile/2567_Increase-tap-target-of-primary-action-on-unsupported-blocks
Sep 15, 2020
Merged

Increase tap-target of primary action on unsupported blocks#25209
marecar3 merged 6 commits intomasterfrom
rnmobile/2567_Increase-tap-target-of-primary-action-on-unsupported-blocks

Conversation

@marecar3
Copy link
Copy Markdown
Contributor

@marecar3 marecar3 commented Sep 10, 2020

Description

Based on the issue: wordpress-mobile/gutenberg-mobile#2567
this PR introduces an increase of tap-target of primary action on unsupported blocks

gutenberg-mobile PR: wordpress-mobile/gutenberg-mobile#2608

How has this been tested?

  1. Open Gutenberg mobile editor
  2. Add unsupported block
  3. Try to click on unsupported block surface
  4. The options sheet should be presented

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 Sep 10, 2020
@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 10, 2020

Size Change: +3 B (0%)

Total Size: 1.2 MB

Filename Size Change
build/components/index.js 202 kB +3 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.67 kB 0 B
build/api-fetch/index.js 3.41 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 8.52 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 128 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.67 kB 0 B
build/block-library/editor.css 8.67 kB 0 B
build/block-library/index.js 139 kB 0 B
build/block-library/style-rtl.css 7.59 kB 0 B
build/block-library/style.css 7.59 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.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.8 kB 0 B
build/components/style-rtl.css 15.5 kB 0 B
build/components/style.css 15.4 kB 0 B
build/compose/index.js 9.67 kB 0 B
build/core-data/index.js 12.2 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.55 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.48 kB 0 B
build/edit-navigation/index.js 10.7 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 305 kB 0 B
build/edit-post/style-rtl.css 6.24 kB 0 B
build/edit-post/style.css 6.22 kB 0 B
build/edit-site/index.js 19.3 kB 0 B
build/edit-site/style-rtl.css 3.13 kB 0 B
build/edit-site/style.css 3.13 kB 0 B
build/edit-widgets/index.js 12.2 kB 0 B
build/edit-widgets/style-rtl.css 2.55 kB 0 B
build/edit-widgets/style.css 2.55 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.3 kB 0 B
build/editor/style-rtl.css 3.8 kB 0 B
build/editor/style.css 3.8 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.71 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 2.13 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 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.31 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 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.56 kB 0 B
build/primitives/index.js 1.41 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.9 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

Copy link
Copy Markdown
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Thank you for this PR @marecar3 !

Code looks perfect. Nice job! ✨

About the behaviour, I think we are missing something. From the original ticket, @iamthomasbishop mentions this:

With this change, I think we will want to change the logic so that you first have to select the block before the tap target becomes "active", which follows some other similar blocks like the Latest Posts block, and most importantly, the select-first interaction gives the user the other block controls such as movers and the overflow (•••) menu.

What I see on the tests, is that pressing over the target area will open the bottom sheet directly, even if the block is not selected. What we would want instead is:

  • When the block is not selected, pressing in the target area (or anywhere over the block) Will select the block.
  • When the block is already selected, pressing in the target area will open the Bottom Sheet.

@marecar3
Copy link
Copy Markdown
Contributor Author

Thank you for this PR @marecar3 !

Code looks perfect. Nice job! ✨

About the behaviour, I think we are missing something. From the original ticket, @iamthomasbishop mentions this:

With this change, I think we will want to change the logic so that you first have to select the block before the tap target becomes "active", which follows some other similar blocks like the Latest Posts block, and most importantly, the select-first interaction gives the user the other block controls such as movers and the overflow (•••) menu.

What I see on the tests, is that pressing over the target area will open the bottom sheet directly, even if the block is not selected. What we would want instead is:

  • When the block is not selected, pressing in the target area (or anywhere over the block) Will select the block.
  • When the block is already selected, pressing in the target area will open the Bottom Sheet.

Ok, it's clear. Thanks for bringing this up. I will fix it.

@marecar3
Copy link
Copy Markdown
Contributor Author

Hey @etoledom 👋
Could you do another round of testing? thanks.

cc @guarani if you want to also give a try.

@etoledom etoledom self-requested a review September 15, 2020 10:56
Copy link
Copy Markdown
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Thank you for the update @marecar3 . Working great now! 🎉

@marecar3
Copy link
Copy Markdown
Contributor Author

Thank you for the update @marecar3 . Working great now! 🎉

Thanks for review!

@marecar3 marecar3 merged commit c2019e3 into master Sep 15, 2020
@marecar3 marecar3 deleted the rnmobile/2567_Increase-tap-target-of-primary-action-on-unsupported-blocks branch September 15, 2020 15:12
@github-actions github-actions bot added this to the Gutenberg 9.1 milestone Sep 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Development

Successfully merging this pull request may close these issues.

2 participants