Fix TextButton.icon breaks focus traversal and ink effect when toggling icon #176579

bleroux · 2025-10-06T14:05:10Z

Description

This PR changes TextButton.icon to avoid building a different widget. When a different widget is created the whole subtree is recreated which leads to various issues (Focus and A11y issues for instance).
The change is similar to #175810 which fixed the exact same problem for OutlinedButton.icon.

Related Issue

Fixes TextButton.icon breaks focus traversal and ink effect when toggling icon

Tests

Adds 1 test

gemini-code-assist

Code Review

This pull request addresses an issue where TextButton.icon loses focus when its icon is toggled. The implementation refactors the TextButton.icon factory into a named constructor. This change prevents the widget from being rebuilt, which preserves its state, including focus. The _TextButtonWithIcon subclass is removed, and its padding logic is integrated into TextButton. A regression test is added to verify that focus is maintained when the icon is nullified. My review includes a suggestion to update a doc comment that has become inaccurate due to these changes.

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

bleroux · 2025-10-06T14:24:21Z

cc @chunhtai Assigning you for the review because the change is similar to #175810 that you reviewed recently.

chunhtai

LGTM, looks like it is the same for ElevatedButton and FilledButton as well. I wish we have some way to share code without exposing additional API

bleroux · 2025-10-07T13:20:33Z

@chunhtai Are there any visual diffs in the Google testing failures? Or are those failures related to tests looking up widgets by type?
My bet it is the second case. Those tests will have to be updated similarly to the flutter gallery test updated in this PR where find.byType(TextButton) returns more widgets now as widgets created by TextButton.icon are now included in the result and were not before this PR.

chunhtai · 2025-10-07T20:48:57Z

I saw a button that was previous disabled but becomes enabled, maybe some of the enabling or onpress callback is not wire up correctly?

chunhtai · 2025-10-07T20:49:54Z

before

after

chunhtai · 2025-10-07T20:59:19Z

This is their code, are we overriding the disable style somehow?

final style =
        buttonStyle ??
        TextButton.styleFrom(
          foregroundColor: theme.colors.primary,
          disabledForegroundColor: theme.colors.onSurfaceVariant,
          iconColor: theme.colors.primary,
          disabledIconColor: theme.colors.onSurfaceVariant,
          textStyle: buttonSize == ButtonSize.medium
              ? theme.textStyles.labelLarge
              : theme.textStyles.bodyMedium,
        );

return TextButton(
              onPressed: onTap != null
                  ? () {
                      onTap!();
                    }
                  : null,
              style: style,
              child: Text(label),
            );

…ng icon

bleroux · 2025-10-08T08:08:49Z

This is their code, are we overriding the disable style somehow?

Thanks for providing this code extract 🙏
I can't reproduce the problem on my side and I don't see what could lead to this change : this PR does not change the default constructor, it mainly changes the TextButton.icon implementation and the code extract does not use TextButton.icon.
It would be interesting to know how the 'buttonStyle' variable in this code snippet is initialized

bleroux · 2025-10-08T12:21:30Z

All checks are green now but this is because the Google testing check did not run automatically after I pushed a new commit.
This new commit sets the default clipBehavior similarly to what was done previously (my goal is to have as little differences as possible compare to before to narrow down the Google testing failure, I don't think this change will help for the Google test failure which was reported).

@Piinks This is a case where the Google testing does not run automatically after a new commit (maybe it will in an indeterminate delay?). The problem here is that someone might jump in and merge this PR, then we will have to revert it later.

Piinks · 2025-10-08T15:52:52Z

Filed #176721 and reached out to the infra team. I will rebase here to try once more.

bleroux · 2025-10-10T16:43:02Z

@chunhtai We looked again at the change, the screenshot and the code extract and we have no clue. If you can share more of the code extract we might understand how it can happen.

chunhtai · 2025-10-23T21:48:12Z

It seems to be a timing issue that the internal test is relies on frame perfect timing of a screen shot.

chunhtai · 2025-10-23T21:56:41Z

cl up cl/823197890, this won't fixed the google test failure, but will at least make scuba triaging less confusing

chunhtai

LGTM

…hen toggling icon (flutter/flutter#176579)

flutter/flutter@7cf0dc1...df72035 2025-10-29 [email protected] Roll Packages from c8ba0cc to 41c6b3d (3 revisions) (flutter/flutter#177725) 2025-10-29 [email protected] [Android] Remove unecessary `spy` in `FlutterActivityAndFragmentDelegateTest` (flutter/flutter#177120) 2025-10-29 [email protected] Add discussion of federated plugin documentation (flutter/flutter#177659) 2025-10-29 [email protected] [web] Deprecate --pwa-strategy (flutter/flutter#177613) 2025-10-29 [email protected] Roll Skia from 53b8b802bbc4 to 0a0c9f8c704f (1 revision) (flutter/flutter#177701) 2025-10-29 [email protected] Add `Navigator.popUntilWithResult` (flutter/flutter#169341) 2025-10-29 [email protected] Replace deprecated `withOpacity` with `withValues` in `text_style.dart` (flutter/flutter#177537) 2025-10-29 [email protected] Replace deprecated withOpacity in `radio.1.dart` example (flutter/flutter#177606) 2025-10-29 [email protected] Roll Skia from 4408d8ea88b0 to 53b8b802bbc4 (2 revisions) (flutter/flutter#177698) 2025-10-29 [email protected] Remove generated file from template manifest (flutter/flutter#177034) 2025-10-29 [email protected] Roll Skia from e582a5594b6f to 4408d8ea88b0 (5 revisions) (flutter/flutter#177691) 2025-10-28 [email protected] Roll Fuchsia Linux SDK from ir6J2isKAYa1jNLyJ... to 3EF6k6lqXPWDwrdyj... (flutter/flutter#177682) 2025-10-28 [email protected] [Gradle 9] Fix Engine Deps (flutter/flutter#177623) 2025-10-28 [email protected] Replace deprecated `withOpacity` in `focus_scope.0.dart‎` example (flutter/flutter#177542) 2025-10-28 [email protected] Fix EditableText _justResumed is not accurate (flutter/flutter#177658) 2025-10-28 [email protected] Replace deprecated `withOpacity` in `interactive_viewer.builder.0.dart` (flutter/flutter#177541) 2025-10-28 [email protected] Fix TextButton.icon breaks focus traversal and ink effect when toggling icon (flutter/flutter#176579) 2025-10-28 [email protected] [Android 16] Update Engine `ci.yaml` to test against Java 21 (flutter/flutter#177677) 2025-10-28 [email protected] Replace deprecated `withOpacity` in `interactive_viewer.constrained.0.dart` (flutter/flutter#177540) 2025-10-28 [email protected] Replace opacity from random color in navigation bar test (flutter/flutter#177490) 2025-10-28 [email protected] Roll Skia from e4d3d8f31aef to e582a5594b6f (6 revisions) (flutter/flutter#177679) 2025-10-28 [email protected] Workaround for lag when dragging window titlebar on Windows (flutter/flutter#177597) 2025-10-28 [email protected] Roll Packages from bbf96a0 to c8ba0cc (2 revisions) (flutter/flutter#177672) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…ng icon (flutter#176579) ## Description This PR changes `TextButton.icon` to avoid building a different widget. When a different widget is created the whole subtree is recreated which leads to various issues (Focus and A11y issues for instance). The change is similar to flutter#175810 which fixed the exact same problem for `OutlinedButton.icon`. ## Related Issue Fixes [TextButton.icon breaks focus traversal and ink effect when toggling icon](flutter#173944) ## Tests - Adds 1 test --------- Co-authored-by: chunhtai <[email protected]>

github-actions bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Oct 6, 2025

gemini-code-assist bot reviewed Oct 6, 2025

View reviewed changes

packages/flutter/lib/src/material/text_button.dart Show resolved Hide resolved

bleroux force-pushed the fix_TextButton.icon_breaks_focus branch from 89921ce to d46cf10 Compare October 6, 2025 14:15

bleroux requested a review from chunhtai October 6, 2025 14:23

bleroux force-pushed the fix_TextButton.icon_breaks_focus branch from d46cf10 to d5f104b Compare October 6, 2025 18:19

chunhtai approved these changes Oct 6, 2025

View reviewed changes

Fix TextButton.icon breaks focus traversal and ink effect when toggli…

0daeb2b

…ng icon

Set default clipBehavior similarly to previous code

16363ef

bleroux force-pushed the fix_TextButton.icon_breaks_focus branch from d5f104b to 16363ef Compare October 8, 2025 08:27

Piinks mentioned this pull request Oct 8, 2025

Google testing not being scheduled #176721

Closed

4 tasks

QuncCccccc mentioned this pull request Oct 9, 2025

Cleanup OutlinedButton.icon documentation and implementation #176630

Merged

Piinks added this to Google Testing Queue Oct 10, 2025

bleroux mentioned this pull request Oct 17, 2025

Using a named constructor (such as .icon) on a MaterialButton makes it unfindable in tests #177089

Open

Piinks requested a review from chunhtai October 22, 2025 18:20

Merge branch 'master' into fix_TextButton.icon_breaks_focus

c0a3663

chunhtai approved these changes Oct 28, 2025

View reviewed changes

chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 28, 2025

auto-submit bot added this pull request to the merge queue Oct 28, 2025

Merged via the queue into flutter:master with commit c059c9b Oct 28, 2025
156 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 28, 2025

github-project-automation bot moved this to Done in Google Testing Queue Oct 28, 2025

bleroux deleted the fix_TextButton.icon_breaks_focus branch October 28, 2025 23:29

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 29, 2025

c059c9b7d Fix TextButton.icon breaks focus traversal and ink effect w…

1be0c59

…hen toggling icon (flutter/flutter#176579)

engine-flutter-autoroll mentioned this pull request Oct 29, 2025

Roll Flutter from 7cf0dc1be72e to 41eae526d73d (19 revisions) flutter/packages#10318

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 29, 2025

c059c9b7d Fix TextButton.icon breaks focus traversal and ink effect w…

254e666

…hen toggling icon (flutter/flutter#176579)

engine-flutter-autoroll mentioned this pull request Oct 29, 2025

Roll Flutter from 7cf0dc1be72e to c06559584290 (20 revisions) flutter/packages#10319

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 29, 2025

c059c9b7d Fix TextButton.icon breaks focus traversal and ink effect w…

ef110c8

…hen toggling icon (flutter/flutter#176579)

engine-flutter-autoroll mentioned this pull request Oct 29, 2025

Roll Flutter from 7cf0dc1be72e to 83ce1845ecc2 (22 revisions) flutter/packages#10321

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 29, 2025

c059c9b7d Fix TextButton.icon breaks focus traversal and ink effect w…

d6f2375

…hen toggling icon (flutter/flutter#176579)

engine-flutter-autoroll mentioned this pull request Oct 29, 2025

Roll Flutter from 7cf0dc1be72e to df72035ba8c5 (23 revisions) flutter/packages#10322

Merged

Fix TextButton.icon breaks focus traversal and ink effect when toggling icon #176579

Fix TextButton.icon breaks focus traversal and ink effect when toggling icon #176579

Uh oh!

Conversation

bleroux commented Oct 6, 2025

Description

Related Issue

Tests

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

bleroux commented Oct 6, 2025

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

bleroux commented Oct 7, 2025

Uh oh!

chunhtai commented Oct 7, 2025

Uh oh!

chunhtai commented Oct 7, 2025

Uh oh!

chunhtai commented Oct 7, 2025

Uh oh!

bleroux commented Oct 8, 2025

Uh oh!

bleroux commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Piinks commented Oct 8, 2025

Uh oh!

bleroux commented Oct 10, 2025

Uh oh!

chunhtai commented Oct 23, 2025

Uh oh!

chunhtai commented Oct 23, 2025

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bleroux commented Oct 8, 2025 •

edited

Loading