Skip to content

Reland "Fix Slider semantics node size" with some changes #184168

Merged
auto-submit[bot] merged 3 commits into
flutter:masterfrom
hannah-hyj:slider-thumb-size-
Mar 30, 2026
Merged

Reland "Fix Slider semantics node size" with some changes #184168
auto-submit[bot] merged 3 commits into
flutter:masterfrom
hannah-hyj:slider-thumb-size-

Conversation

@hannah-hyj

@hannah-hyj hannah-hyj commented Mar 25, 2026

Copy link
Copy Markdown
Member

This is a reland of a old PR #115285 with some changes:

  1. the node has onFocusAction
  2. update tests to reflect the semantics tree changes.
  3. the semantics and paint() reuse the _calcThumbCenter function. in the formly reverted PR Fix Slider semantics node size #115285 the semantics only get the value of the thumbcenter after it's updated in paint() , I think it's what caused the g3 test failures in g3 and caused the revert.

The semantics tree changes was caused because the semantics node was higher than the overlay before, and after the PR, the semantics node is formed as a leaf node and not merged with the overlay node, the extra overlay node won't change user experience but it will change some test result,

fixes #114225

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

@github-actions github-actions Bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) labels Mar 25, 2026
@hannah-hyj hannah-hyj added the CICD Run CI/CD label Mar 25, 2026

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

Copy link
Copy Markdown
Contributor

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 refactors the Slider widget's accessibility semantics by moving semantic properties from the Semantics widget wrapping the FocusableActionDetector directly into the _RenderSlider's describeSemanticsConfiguration and assembleSemanticsNode methods. This includes adding an onDidGainAccessibilityFocus callback and a new onFocusAction to handle accessibility focus. Test files have been updated to reflect these changes. A critical issue was identified in packages/flutter_test/test/controller_test.dart where sliderFinder.evaluate().single is incorrectly used to access a value property, which would cause a compilation error. The suggestion is to use tester.semantics.find(sliderFinder).value to correctly check the SemanticsNode's value.

Comment thread packages/flutter_test/test/controller_test.dart Outdated
@github-actions github-actions Bot removed the CICD Run CI/CD label Mar 25, 2026
@hannah-hyj hannah-hyj added the CICD Run CI/CD label Mar 25, 2026
@github-actions github-actions Bot removed the CICD Run CI/CD label Mar 25, 2026
@hannah-hyj hannah-hyj added the CICD Run CI/CD label Mar 25, 2026
@hannah-hyj hannah-hyj requested review from chunhtai and removed request for chunhtai March 25, 2026 23:00
@github-actions github-actions Bot removed the CICD Run CI/CD label Mar 26, 2026
@hannah-hyj hannah-hyj changed the title Reland "Fix Slider semantics node size" Reland "Fix Slider semantics node size" with some changes Mar 26, 2026
Update slider.dart

_thumbCenter

Update packages/flutter_test/test/controller_test.dart

tests

Update slider_test.dart

1

Update slider.dart
@hannah-hyj hannah-hyj added the CICD Run CI/CD label Mar 26, 2026
@github-actions github-actions Bot removed the CICD Run CI/CD label Mar 26, 2026
@hannah-hyj hannah-hyj added the CICD Run CI/CD label Mar 26, 2026
@hannah-hyj hannah-hyj requested a review from chunhtai March 26, 2026 19:59
@hannah-hyj hannah-hyj added CICD Run CI/CD and removed CICD Run CI/CD labels Mar 26, 2026
Iterable<SemanticsNode> children,
) {
node.rect = Rect.fromCenter(
center: _semanticThumbCenter,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We will also need to make sure the thumb's render object itself is big enough, not just the semantics node.

@hannah-hyj hannah-hyj Mar 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Do you mean we need to make the thumb size 48*48?

I don't think that's necessary, because in non-a11y mode,
the whole slider is interactive and you can start dragging the thumb even if you didn't clicked on the thumb. so the interactive is definitely larger than 48*48

@hannah-hyj hannah-hyj requested a review from chunhtai March 27, 2026 20:46

@chunhtai chunhtai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@hannah-hyj hannah-hyj added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 30, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue Mar 30, 2026
Merged via the queue into flutter:master with commit 53257ab Mar 30, 2026
80 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 30, 2026
ahmedsameha1 pushed a commit to ahmedsameha1/flutter that referenced this pull request Apr 14, 2026
…4168)

This is a reland of a old PR
flutter#115285 with some changes:
1. the node has onFocusAction 
2. update tests to reflect the semantics tree changes.  
3. the `semantics` and `paint()` reuse the _calcThumbCenter function. in
the formly reverted PR flutter#115285
the semantics only get the value of the thumbcenter after it's updated
in paint() , I think it's what caused the g3 test failures in g3 and
caused the revert.


The semantics tree changes was caused because the semantics node was
higher than the overlay before, and after the PR, the semantics node is
formed as a leaf node and not merged with the overlay node, the extra
overlay node won't change user experience but it will change some test
result,



fixes flutter#114225

## Pre-launch Checklist

- [ ] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [ ] I read the [AI contribution guidelines] and understand my
responsibilities, or I am not using AI tools.
- [ ] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [ ] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [ ] I signed the [CLA].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[AI contribution guidelines]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines
[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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: tests "flutter test", flutter_test, or one of our tests CICD Run CI/CD 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.

Slider and RangeSlider thumb accessibility issues

2 participants