Repository navigation
Use a separately focusable semantics node for the chip delete icon - #48740
Conversation
fluttergithubbot
added
d: examples
| child: IconTheme( | ||
| data: theme.iconTheme.copyWith( | ||
| color: widget.deleteIconColor ?? chipTheme.deleteIconColor, | ||
| return Semantics( |
There was a problem hiding this comment.
I wonder if this change should be moved inside the _wrapWithTooltip helper
There was a problem hiding this comment.
Good question. I chose not to because there is already a top level Semantics node for the whole chip, and the only other use case of _wrapTooltip is for the onPressed, and that tooltip message gets read from the top level Semantics node
HansMuller
left a comment
HansMuller
left a comment
There was a problem hiding this comment.
This seems OK but it might need to be a little more buttony.
| data: theme.iconTheme.copyWith( | ||
| color: widget.deleteIconColor ?? chipTheme.deleteIconColor, | ||
| return Semantics( | ||
| container: true, |
There was a problem hiding this comment.
Shouldn't this also specify button: true?
| key: deleteIconKey, | ||
| behavior: HitTestBehavior.opaque, | ||
| onTap: widget.isEnabled | ||
| ? () { |
There was a problem hiding this comment.
This was indented incorrectly before, you could fix it (2 spaces) now
| label: 'test', | ||
| textDirection: TextDirection.ltr, | ||
| children: <TestSemantics>[ | ||
| TestSemantics( |
There was a problem hiding this comment.
Shouldn't this look more like a button semantics test? Like https://github.com/flutter/flutter/blob/master/packages/flutter/test/material/flat_button_test.dart#L490
There was a problem hiding this comment.
Updated to use button flag that reflects the change made in the actual Semantics
6 participants
Description
Previously, only the entire chip was focusable by the semantics voiceover reader. Now, the delete icon is a separate semantics node. This also aids in the a11y experience for chips that have both a primary action and a delete action.
Related Issues
closes #48739
Tests
I added the following tests:
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.