Skip to content

Conversation

@xu-baolin
Copy link
Member

Fixes #101868

The Slider.label should not write to SemanticsConfiguration.label,
I check the originally introduced PR https://github.com/flutter/flutter/pull/58535/files which is more like a mistake.
Also, I test the Android native talkback, it does not read the label info out.

In addition, improve the docs about when the label will appear.

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Apr 19, 2022
@xu-baolin xu-baolin requested a review from gspencergoog April 19, 2022 07:26
Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

What is the problem with the current label?

Will information about the range labels be missing from the semantics if you do this?

Comment on lines 289 to 290
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// when the slider is active and the [SliderThemeData.showValueIndicator]
/// is satisfied.
/// when the slider is active and [SliderThemeData.showValueIndicator]
/// is true.

Copy link
Member Author

Choose a reason for hiding this comment

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

It isn't a bool property: )

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// the [SliderThemeData.showValueIndicator] is satisfied.
/// [SliderThemeData.showValueIndicator] is true.

Copy link
Contributor

Choose a reason for hiding this comment

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

If this is removed, then we no longer need to pass label to this function.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@xu-baolin
Copy link
Member Author

What is the problem with the current label?

Will information about the range labels be missing from the semantics if you do this?

As the reference issue report, the android talkback will read 'value + label + slider',such as ' 50% 50 slider '.

I have tested the native slider widget, and the talkback doesn't read labels info out. The semantics label doesn't apply here,right?

Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

Sounds good, thanks for the PR!

@fluttergithubbot fluttergithubbot merged commit 2381657 into flutter:master Apr 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 28, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 28, 2022
egramond pushed a commit to egramond/flutter that referenced this pull request May 5, 2022
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.

Accessibility is broken on the Slider widget

3 participants