Tweaking Material Chip a11y semantics to match buttons by nscobie · Pull Request #60141 · flutter/flutter

nscobie · 2020-06-23T23:14:30Z

Description

Material Chips generally behave similarly to buttons (excluding the plain info Chip in Flutter which is non-interactive), but are not semantically considered to be buttons for a11y purposes. This leads to subpar feedback for those interacting with Chips in a Flutter app via systems such as TalkBack or VoiceOver. This change brings the Chip a11y semantics in-line with the a11y semantics used for buttons, and matches the behavior of the reference Material components implementations for iOS.

Caveat: the reference implementation for Material components on Android considers some Chips to be "checkable" rather than "buttons". Our implementation for Chip semantics will be further refined in the future to match this behavior by making specific Chip widgets such as FilterChip and ChoiceChip checkable (see issue #60249 ), while keeping others such as InputChip and ActionChip as buttons. We should likely still move forward with this PR to least mark them as buttons for now to unblock customer: money (g3) on #58010

Related Issues

Fixes InputChip does not announce any action for a11y on iOS #58010

Tests

I added the following tests:

packages/flutter/test/material/chip_test.dart
- Chip semantics
  - tapEnabled explicitly false (ensures info chips will not be considered buttons)
  - enabled when tapEnabled and canTap (ensures chips which can be tapped, both in general and at this moment, are marked as enabled buttons)
  - disabled when tapEnabled but not canTap (ensures chips which could be tapped, but not currently, are marked as disabled buttons)

I updated the following tests to account for newly expected button semantics:

packages/flutter/test/material/chip_test.dart
- Chip semantics
  - label only
  - delete
  - with onPressed
  - with onSelected
  - disabled

Sample App for Manual Testing

Please ignore how bad this app is. :-)

// https://github.com/flutter/flutter/issues/58010

import 'package:flutter/material.dart';

void main() => runApp(MyApp());

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
//      showSemanticsDebugger: true,
      title: 'Flutter Demo 58010',
      home: Scaffold(
        appBar: AppBar(
          title: Text('Home Page 58010'),
        ),
        body: Center(
          child: ListView(
            children: [
              Text('Chips', textAlign: TextAlign.center),
              Text('(Asterisk "*" means not a button)',
                  textAlign: TextAlign.center),
              Row(
                children: [
                  Expanded(
                    flex: 1,
                    child: Column(
                      children: [
                        Text("Have callbacks"),
                        InputChip(label: Text('input'), onPressed: () {}),
                        Chip(label: Text('info*')),
                        Chip(label: Text('deletable info*'), onDeleted: () {}),
                        ChoiceChip(
                            label: Text('choice off'),
                            onSelected: (_) {},
                            selected: false),
                        ChoiceChip(
                            label: Text('choice on'),
                            onSelected: (_) {},
                            selected: true),
                        FilterChip(label: Text('filter'), onSelected: (_) {}),
                        ActionChip(label: Text('action'), onPressed: () {}),
                      ],
                    ),
                  ),
                  Expanded(
                    flex: 1,
                    child: Column(
                      children: [
                        Text("No callbacks"),
                        InputChip(label: Text('input')),
                        Chip(label: Text('info*')),
                        Chip(label: Text('deletable info*')),
                        ChoiceChip(label: Text('choice off'), selected: false),
                        ChoiceChip(label: Text('choice on'), selected: true),
                        FilterChip(label: Text('filter')),
//                      ActionChip(label: Text('action chip'), onPressed: () {}), // this one can't exists without a callback
                        Text('(ActionChip cannot exist without a callback)'),
                      ],
                    ),
                  ),
                ],
              ),
              Text('Buttons', textAlign: TextAlign.center),
              Row(
                children: [
                  Expanded(
                    flex: 1,
                    child: Column(
                      children: [
                        RaisedButton(child: Text('raised'), onPressed: () {}),
                        IconButton(
                            icon: Icon(Icons.access_alarm),
                            tooltip: 'alarm icon',
                            onPressed: () {}),
                      ],
                    ),
                  ),
                  Expanded(
                    flex: 1,
                    child: Column(
                      children: [
                        RaisedButton(child: Text('raised')),
                        IconButton(
                            icon: Icon(Icons.access_alarm),
                            tooltip: 'alarm icon'),
                      ],
                    ),
                  ),
                ],
              )
            ],
          ),
        ),
      ),
    );
  }
}

Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I signed the [CLA].
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read [Handling breaking changes].

No, no existing tests failed, so this is not a breaking change.
Note: Five of the tests under packages/flutter/test/material/chip_test.dart had to be updated since they check for exact matches to semantic trees, but I'm not actually altering anything that I would consider an "API". @Hixie said it's probably fine to not consider this a breaking change.

gaaclarke · 2020-06-24T03:44:21Z

This seems like the right property to for this:

/// For example, the [Chip] class sets this to false because it can't be /// disabled, even if no callbacks are set on it, since it is used for /// displaying information only.

Looking good =)

gaaclarke

LGTM nice work!

flutter-github-sync · 2020-06-25T00:27:57Z