Skip to content

Conversation

@bleroux
Copy link
Contributor

@bleroux bleroux commented Oct 27, 2025

Description

This PR changes FilledButton.icon and FilledButton.tonalIcon 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

TextButton.icon breaks focus traversal and ink effect when toggling icon
Voiceover focus traversal breaks if a button's state changes to include an icon

Tests

  • Adds 4 tests

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Oct 27, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request modifies the FilledButton widget to avoid rebuilding the whole subtree when the icon changes, resolving focus traversal and VoiceOver issues. The changes involve refactoring the FilledButton.icon and FilledButton.tonalIcon constructors to use a single _FilledButtonWithIconChild widget instead of creating a new widget each time the icon changes. The tests are expanded to ensure that focus and semantics are preserved when toggling the icon. I have added review comments for a high severity issue.

@bleroux bleroux force-pushed the fix_FilledButton.icon_breaks_focus_and_semantics branch from 2fe655c to 72d2013 Compare October 27, 2025 19:52
@bleroux bleroux requested a review from chunhtai October 28, 2025 23:31
@chunhtai chunhtai requested review from hannah-hyj and removed request for chunhtai October 30, 2025 16:41
Copy link
Member

@hannah-hyj hannah-hyj left a comment

Choose a reason for hiding this comment

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

This looks good to me

@bleroux bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 31, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Oct 31, 2025
Merged via the queue into flutter:master with commit 46733c6 Oct 31, 2025
78 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 31, 2025
@bleroux bleroux deleted the fix_FilledButton.icon_breaks_focus_and_semantics branch October 31, 2025 14:59
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 31, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 31, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 31, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 31, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 31, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 1, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 1, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 1, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 2, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 2, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 2, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 3, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Nov 4, 2025
flutter/flutter@6f8abdd...027f2e4

2025-10-31 [email protected] Roll Dart SDK from bb45c4186fb2 to db168d9e7471 (1 revision) (flutter/flutter#177839)
2025-10-31 [email protected] Hide "waiting for customer" issues from text input triage (flutter/flutter#177524)
2025-10-31 [email protected] Making the multiple_windows example app demonstrate dialogs of dialogs (flutter/flutter#177786)
2025-10-31 [email protected] Fix ElevatedButton.icon breaks focus traversal and VoiceOver when toggling icon (flutter/flutter#177579)
2025-10-31 [email protected] Fix FilledButton.icon and FilledButton.tonalIcon break focus traversal and VoiceOver (flutter/flutter#177593)
2025-10-31 [email protected] Fix cubic subdivision estimation using correct Wang’s formula (flutter/flutter#177758)
2025-10-31 [email protected] Roll Skia from ccbd7697791f to 2cf9a1923078 (1 revision) (flutter/flutter#177832)
2025-10-31 [email protected] Roll Fuchsia Linux SDK from ksXeDDo2yYBXJ4uEu... to O-OoG6j4wHXd1ThNM... (flutter/flutter#177831)
2025-10-31 [email protected] Roll Skia from 1532fabb4b7d to ccbd7697791f (4 revisions) (flutter/flutter#177828)
2025-10-31 [email protected] Roll Dart SDK from cf24b43cb643 to bb45c4186fb2 (2 revisions) (flutter/flutter#177824)
2025-10-31 [email protected] Roll Skia from 825d5c854302 to 1532fabb4b7d (3 revisions) (flutter/flutter#177808)
2025-10-30 [email protected] Roll Dart SDK from da663596bf6d to cf24b43cb643 (2 revisions) (flutter/flutter#177798)
2025-10-30 [email protected] Update the path used by the download_fuchsia_sdk.py script to //third_party/fuchsia-sdk (flutter/flutter#177794)
2025-10-30 [email protected] Roll Skia from 6af53143b120 to 825d5c854302 (2 revisions) (flutter/flutter#177788)
2025-10-30 [email protected] Fixes RuntimeEffect when used with ImageFilter.compose and gaussian blur (flutter/flutter#177687)
2025-10-30 [email protected] Adds cache extent type to two_dimentional_viewport (flutter/flutter#177411)
2025-10-30 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Update .ci.yaml in flutter/flutter to use 15.5 (#177669)" (flutter/flutter#177793)
2025-10-30 [email protected] Marks Windows windowing_test to be flaky (flutter/flutter#177716)
2025-10-30 [email protected] Fixes `SettingsChannelTest` flake (flutter/flutter#177061)
2025-10-30 [email protected] Implements uniform-by-name for web (flutter/flutter#176980)
2025-10-30 [email protected] Update .ci.yaml in flutter/flutter to use 15.5 (flutter/flutter#177669)
2025-10-30 [email protected] [Impeller] Fall back to OpenGL ES on older Adreno GPUs (flutter/flutter#177747)
2025-10-30 [email protected] [VPAT] Update a11y assessment app and guideline tests (flutter/flutter#177690)
2025-10-30 [email protected] Roll Skia from 18457971c30f to 6af53143b120 (4 revisions) (flutter/flutter#177778)
2025-10-30 [email protected] Roll Packages from 41c6b3d to 1a7075b (2 revisions) (flutter/flutter#177777)
2025-10-30 [email protected] Roll Dart SDK from 4785d5971d64 to da663596bf6d (1 revision) (flutter/flutter#177772)

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
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
…l and VoiceOver (flutter#177593)

## Description

This PR changes `FilledButton.icon` and `FilledButton.tonalIcon` 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

[TextButton.icon breaks focus traversal and ink effect when toggling
icon](flutter#173944)
[Voiceover focus traversal breaks if a button's state changes to include
an icon](flutter#175810)

## Tests

- Adds 4 tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants