InkWell can show focusColor optional on touch device by xu-baolin · Pull Request #70442 · flutter/flutter

xu-baolin · 2020-11-13T08:00:54Z

Description

Currently, the focus highlight can only be shown when [FocusManager.highlightMode] is [FocusHighlightMode.traditional],
Sometimes, showing the focus highlight is useful for the touch device widget, such as highlight the selected item of DropdownButton, so I add a property alwaysShowFocusColor of InkWell to make it optinal.

I also improve some documentations about focusColor and [FocusManager.highlightMode] behavior.

Related Issues

Fixes #70294

Tests

I added the following tests:

See files.

gspencergoog · 2020-11-13T21:08:34Z

Hi @xu-baolin,

Thanks for the PR, but I'd rather not add this parameter to InkWell, I think it's redundant. You can already do what you want by setting FocusManager.instance.highlightStrategy to FocusHighlightStrategy.alwaysTraditional, which will enable focus highlights to be shown even on a device in touch mode.

xu-baolin · 2020-11-13T21:33:48Z

But this doesn't seem to work for this issue scenario. I just want to let some widget response instead of making all of them respond focus change.

HansMuller · 2020-11-20T21:47:10Z

I'd prefer to avoid adding one-off flags like alwaysShowFocusColor.

It's not possible to use the FocusManager, since it's a singleton and can't be used to override highlightStrategy within a subtree (i.e. for just one DropdownButton's menu).

I think what's really needed is a way to specify how menu item highlights are computed for the selected/focused/etc states. I think we could do that without hijacking the focus manager's highlightMode property, by exposing the per-item InkWell overlayColor property in DropdownButton and by folding in the MaterialState.selected state, so overlayColor (MaterialStateProperty<Color>)values could select a color based on each item's disabled, hovered, pressed, focused, and selected state.

xu-baolin · 2020-11-21T07:22:45Z

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

    switch (FocusManager.instance.highlightMode) {
      case FocusHighlightMode.touch:
        showFocus = false;
+        showSelect = true;


For touch devices, do not display the focus color, try to display the selected color(if set by overlayColor) instead.

HansMuller · 2020-11-23T19:33:37Z

Why not "expose the per-item InkWell overlayColor property in DropdownButton" so that the color of menu items for all states could be specified with DropdownButton.itemOverlayColor?

xu-baolin · 2020-11-24T05:47:28Z

Why not "expose the per-item InkWell overlayColor property in DropdownButton" so that the color of menu items for all states could be specified with DropdownButton.itemOverlayColor?

Done.

HansMuller · 2020-11-24T17:10:20Z

How would a developer override the default overlayColor you've added?

xu-baolin · 2020-11-25T00:27:51Z

How would a developer override the default overlayColor you've added?

Override the ThemeData for the subtree, the same as before this change.

xu-baolin · 2020-12-04T06:00:10Z

Hi guys, Any thoughts about this patch?

flutter-dashboard · 2020-12-24T06:02:19Z

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

HansMuller · 2021-01-20T00:17:43Z

I appreciate the work you've put into this PR, and I apologize for only being a "drive by" reviewer.

I think we're going to need to sort out a general solution per #70442 (comment), for all components that can take the focus when they're used in desktop applications. I'm going to close this PR to give everyone a chance to think about how best to solve (and explain) a general solution.

Sorry about the long delay and for the effort you've put into this.

Fix #70294 ### Description The bug is about the previously selected DropdownMenuItem isn't highlighted on mobile while it works on Web and desktop. The reason is that `FocusManager.instance.highlightMode` has fallen to `FocusHighlightMode.touch` on mobile by default (or touch device without mouse in general): https://github.com/flutter/flutter/blob/6f220d3ec0948a4def1622de805fabda9855338d/packages/flutter/lib/src/material/dropdown.dart#L172-L175 https://github.com/flutter/flutter/blob/7d56be4c8de814022357633179a816b51ffca4b8/packages/flutter/lib/src/widgets/focus_manager.dart#L2335-L2353 https://github.com/flutter/flutter/blob/7d56be4c8de814022357633179a816b51ffca4b8/packages/flutter/lib/src/widgets/focus_manager.dart#L1741-L1748 We actually can work around this by setting `FocusManager.instance.highlightStrategy` to `FocusHighlightStrategy.alwaysTraditional`, see #70442 (comment). Nevertheless, this approach will may affect the entire scope of the app, potentially resulting in unforeseen behavior for other widgets. In this PR, I propose a straightforward solution that utilizes an ancestor widget (Ink) to manage the color state corresponding to `InkWell.overlayColor`. ### Demo the fix https://github.com/user-attachments/assets/b6476732-716c-4f39-ab82-7ab5accb7182 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md Signed-off-by: huycozy <[email protected]>

InkWell can show focusColor optinal on touch device

cbaba89

flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Nov 13, 2020

google-cla bot added the cla: yes label Nov 13, 2020

xu-baolin mentioned this pull request Nov 13, 2020

Automatic focus highlight mode for FocusManager #37825

Merged

9 tasks

xu-baolin added 2 commits November 13, 2020 18:06

fix some typos

94e2209

fix analyze

864c224

gspencergoog self-requested a review November 13, 2020 21:04

HansMuller changed the title ~~InkWell can show focusColor optinal on touch device~~ InkWell can show focusColor optional on touch device Nov 19, 2020

for code review suggestion

9d444fe

xu-baolin commented Nov 21, 2020

View reviewed changes

xu-baolin added 5 commits November 21, 2020 15:35

for code review suggestion

63cefe3

for code review suggestion

655c169

for code review suggestion

841f7fd

for code review suggestion

df5cae1

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

4e7bacd

for code review suggestions

93d04de

Piinks added the f: focus Focus traversal, gaining or losing focus label Nov 25, 2020

xu-baolin requested a review from HansMuller December 24, 2020 13:29

HansMuller closed this Jan 20, 2021

huycozy mentioned this pull request Apr 26, 2025

Fix Selected DropdownMenuItem isn't highlighted on mobile #167874

Merged

9 tasks

huycozy mentioned this pull request Jun 12, 2025

focusColor does not work in InkWell on mobile platforms (Android, iOS) #157817

Open

Conversation

xu-baolin commented Nov 13, 2020

Description

Related Issues

Tests

Uh oh!

gspencergoog commented Nov 13, 2020

Uh oh!

xu-baolin commented Nov 13, 2020

Uh oh!

HansMuller commented Nov 20, 2020

Uh oh!

xu-baolin Nov 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

HansMuller commented Nov 23, 2020

Uh oh!

xu-baolin commented Nov 24, 2020

Uh oh!

HansMuller commented Nov 24, 2020

Uh oh!

xu-baolin commented Nov 25, 2020

Uh oh!

xu-baolin commented Dec 4, 2020

Uh oh!

flutter-dashboard bot commented Dec 24, 2020

Uh oh!

HansMuller commented Jan 20, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

xu-baolin Nov 21, 2020 •

edited

Loading