Add outer gesture detector to increase mini-fab size to 48x48 #17921

jonahwilliams · 2018-05-25T21:27:02Z

This is one potential solution to the issue of certain hit targets being too small. The gesture detector in the material button takes it's size from the parent constraints, there isn't a way to make the hit test area larger without also increasing the size of material. By adding an outer gesture detector on padding we increase the hit test area without needing to modify Material or InkResponse.

For the mini-fab, by adding 4.0 of padding on all sides we increase the hit test region to 48x48.

Fixes #16625

jonahwilliams · 2018-05-25T21:29:36Z

Note: I confirmed that this works locally by increasing the padding to an absurdly large size so my fingers could hit it without pressing the button.

jonahwilliams · 2018-05-29T21:35:00Z

@Hixie & @goderbauer thoughts on this approach?

goderbauer · 2018-05-29T22:09:24Z

packages/flutter/lib/src/material/button.dart


+  /// Padding to increase the size of the gesture detector which doesn't
+  /// increase the visible material of the button.
+  final EdgeInsets slopPadding;


(My non-native english skills might be failing me here), but why is this called slopPadding?

Also, should we document that this actually increases the layout size of the FAB? i.e. without extra work(e.g. using a stack or similar) you can't place things in the slopPadding.

I named it this to relate it to hitSlop, but we could just call it outerPadding or externalPadding to be more literal

goderbauer · 2018-05-29T22:12:30Z

packages/flutter/lib/src/material/button.dart

      ),
    );
+
+    if (widget.slopPadding != null) {


This will need at least two tests: One that verifies that the bounds of the semantics node are actually increased by slop and another one to verify that a tap dispatched to the (for testing purposes unreasonably large) "slop" area is actually handled by the onPressed handler.

We don't specifically have tests for the RawMaterialButton, do you know if this is intentional? Otherwise happy to add some

I'm pretty sure the lack of testing is not intentional :)

Added two basic tests to check that hit detection is expanded and semantics are expanded

goderbauer · 2018-05-29T22:13:43Z

This looks like the approach I've discussed with @Hixie before.

goderbauer · 2018-05-29T22:18:02Z

packages/flutter/lib/src/material/floating_action_button.dart

      onHighlightChanged: _handleHighlightChanged,
      elevation: _highlight ? widget.highlightElevation : widget.elevation,
      constraints: widget._sizeConstraints,
+      slopPadding: widget.mini ? const EdgeInsets.all(4.0) : null,


Might be worth documenting this side effect on the mini property.

jonahwilliams · 2018-05-30T19:38:42Z

Placing excludeSemantics: true on the outerPadding gesture detector seems to get the behavior we want, with the semantic bounds become the entire area.

goderbauer

LGTM

0c35a34 Roll src/third_party/skia f5132a05c893..4baa7326ccfb (1 commits) (flutter#18003) f07712b Roll src/third_party/skia 5f56cb1d3b4f..f5132a05c893 (6 commits) (flutter#17999) 103c9c7 Bumped up the timeout for testAccessibilityFocusOnTextSemanticsProducesCorrectIosViews (flutter#17988) a1fdff6 Roll src/third_party/skia 81ef385c1fcd..5f56cb1d3b4f (14 commits) (flutter#17991) 22479ca Roll Dart to 726d3c772554924f62db0b9e0d4c280dbbddc824 (flutter#17993) 494a63c Trial PR to enable null safety unfork in the Dart SDK. (flutter#17818) 887efcb Roll fuchsia/sdk/core/linux-amd64 from eapIV... to 3h-X9... (flutter#17987) 50ae2b9 Set SkSL asset manager in RunConfiguration ctor (flutter#17948) 5f437fb Started ignoring remote keyboard notifications. (flutter#17981) 027eff8 Reenable flutter scenic test to identify crashes and follow up on fixes. (flutter#17979) 992a55c Update buildroot (flutter#17978) d5587df Roll src/third_party/skia 78debd6f6d83..81ef385c1fcd (1 commits) (flutter#17976) aa00d50 [web] Don't allow empty initial route (flutter#17936) 3b0e415 Roll fuchsia/sdk/core/mac-amd64 from 9O3Ef... to arZdZ... (flutter#17975) 66af9ea Roll src/third_party/skia 981d590e8eba..78debd6f6d83 (5 commits) (flutter#17972) 732bf22 Manual roll of Dart to 03429b20cd67f85d65cc589b529ab8c1a4780912...a53d336b9fd4bbb415d2f1e3f4c653aa107f31c7 (flutter#17971) cad81c7 Roll src/third_party/skia 1ae3e75a0b4c..981d590e8eba (1 commits) (flutter#17968) eed05dd Add initial unit tests for the android embedding (flutter#17921) 168a65f Roll src/third_party/dart 2e438d1baffc..a53d336b9fd4 (4 commits) (flutter#17967) 742adb8 Roll src/third_party/skia 97cfb05aabe4..1ae3e75a0b4c (2 commits) (flutter#17966) 805ef7c Roll fuchsia/sdk/core/mac-amd64 from 2CE6x... to 9O3Ef... (flutter#17963) 11c6a18 Roll src/third_party/skia c12aad9485a9..97cfb05aabe4 (3 commits) (flutter#17957) 4e29e57 Roll fuchsia/sdk/core/mac-amd64 from 9-v-E... to 2CE6x... (flutter#17955) 4f3b929 Roll src/third_party/dart 216e3df4526c..2e438d1baffc (7 commits) (flutter#17950) 4f888d6 Change the repo fetch script used in integration tests (flutter#17943) 3999ef9 Roll src/third_party/skia 1e21d14f2b8b..c12aad9485a9 (20 commits) (flutter#17942) d01de3b Roll src/third_party/dart a69cb6d700f5..216e3df4526c (16 commits) (flutter#17945) 2589d07 Fix accessibility focus loss when first focusing on text field (flutter#17803) 3af2b1a Roll fuchsia/sdk/core/linux-amd64 from _dAFU... to G4HpJ... (flutter#17938) d132ac5 [web] Fix exception when getting boxes for rich text range (flutter#17933) cade0e9 [web] Batch systemFontChange messages (flutter#17885)

0c35a34 Roll src/third_party/skia f5132a05c893..4baa7326ccfb (1 commits) (#18003) f07712b Roll src/third_party/skia 5f56cb1d3b4f..f5132a05c893 (6 commits) (#17999) 103c9c7 Bumped up the timeout for testAccessibilityFocusOnTextSemanticsProducesCorrectIosViews (#17988) a1fdff6 Roll src/third_party/skia 81ef385c1fcd..5f56cb1d3b4f (14 commits) (#17991) 22479ca Roll Dart to 726d3c772554924f62db0b9e0d4c280dbbddc824 (#17993) 494a63c Trial PR to enable null safety unfork in the Dart SDK. (#17818) 887efcb Roll fuchsia/sdk/core/linux-amd64 from eapIV... to 3h-X9... (#17987) 50ae2b9 Set SkSL asset manager in RunConfiguration ctor (#17948) 5f437fb Started ignoring remote keyboard notifications. (#17981) 027eff8 Reenable flutter scenic test to identify crashes and follow up on fixes. (#17979) 992a55c Update buildroot (#17978) d5587df Roll src/third_party/skia 78debd6f6d83..81ef385c1fcd (1 commits) (#17976) aa00d50 [web] Don't allow empty initial route (#17936) 3b0e415 Roll fuchsia/sdk/core/mac-amd64 from 9O3Ef... to arZdZ... (#17975) 66af9ea Roll src/third_party/skia 981d590e8eba..78debd6f6d83 (5 commits) (#17972) 732bf22 Manual roll of Dart to 03429b20cd67f85d65cc589b529ab8c1a4780912...a53d336b9fd4bbb415d2f1e3f4c653aa107f31c7 (#17971) cad81c7 Roll src/third_party/skia 1ae3e75a0b4c..981d590e8eba (1 commits) (#17968) eed05dd Add initial unit tests for the android embedding (#17921) 168a65f Roll src/third_party/dart 2e438d1baffc..a53d336b9fd4 (4 commits) (#17967) 742adb8 Roll src/third_party/skia 97cfb05aabe4..1ae3e75a0b4c (2 commits) (#17966) 805ef7c Roll fuchsia/sdk/core/mac-amd64 from 2CE6x... to 9O3Ef... (#17963) 11c6a18 Roll src/third_party/skia c12aad9485a9..97cfb05aabe4 (3 commits) (#17957) 4e29e57 Roll fuchsia/sdk/core/mac-amd64 from 9-v-E... to 2CE6x... (#17955) 4f3b929 Roll src/third_party/dart 216e3df4526c..2e438d1baffc (7 commits) (#17950) 4f888d6 Change the repo fetch script used in integration tests (#17943) 3999ef9 Roll src/third_party/skia 1e21d14f2b8b..c12aad9485a9 (20 commits) (#17942) d01de3b Roll src/third_party/dart a69cb6d700f5..216e3df4526c (16 commits) (#17945) 2589d07 Fix accessibility focus loss when first focusing on text field (#17803) 3af2b1a Roll fuchsia/sdk/core/linux-amd64 from _dAFU... to G4HpJ... (#17938) d132ac5 [web] Fix exception when getting boxes for rich text range (#17933) cade0e9 [web] Batch systemFontChange messages (#17885)

add outer gesture detector to increase mini-fab size to 48x48

90a5884

googlebot added the cla: yes label May 25, 2018

goderbauer reviewed May 29, 2018

View reviewed changes

Jonah Williams added 3 commits May 29, 2018 17:04

rename slopPadding to outerPadding, add doc to mini prop on fab

4aa3346

Merge branch 'master' of github.com:flutter/flutter into a11y_16625

7f48378

Exclude padding hit detector from semantics and add tests

7519864

goderbauer approved these changes May 30, 2018

View reviewed changes

jonahwilliams merged commit 6fc7199 into flutter:master May 30, 2018

jonahwilliams deleted the a11y_16625 branch May 30, 2018 21:54

christopherfujino mentioned this pull request Apr 28, 2020

manual engine roll (4bcfae82c7c1 -> 0c35a3417) #55891

Merged

github-actions bot locked as resolved and limited conversation to collaborators Aug 10, 2021

Add outer gesture detector to increase mini-fab size to 48x48 #17921

Add outer gesture detector to increase mini-fab size to 48x48 #17921

Uh oh!

Conversation

jonahwilliams commented May 25, 2018

Uh oh!

jonahwilliams commented May 25, 2018

Uh oh!

jonahwilliams commented May 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

goderbauer commented May 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented May 30, 2018

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants