Skip to content

[VPAT] Fix contrast and reflow in three a11y_assessments demos - #13409

Merged
auto-submit[bot] merged 2 commits into
flutter:mainfrom
flutter-zl:vpat-a11y-assessments-colors-layout
Nov 2, 2026
Merged

auto-submit[bot] merged 2 commits into
flutter:mainfrom
flutter-zl:vpat-a11y-assessments-colors-layout

Conversation

@flutter-zl

Copy link
Copy Markdown
Contributor

Problem

Three a11y_assessments demos fail basic accessibility checks. The badge label is white on Colors.green, a 2.8:1 contrast ratio, and the drawer header is white on Colors.blue, 3.1:1, where small text needs 4.5:1. The expansion tile page lays its tiles out in a Column, so at a 320x256 viewport the second and third tiles overflow and cannot be reached.

Fix

The badge drops its hardcoded colors for the theme defaults, error on onError, and the drawer header uses the theme's primary and onPrimary pair. Both pairs meet 4.5:1 in all four of the app's themes. The expansion tile body becomes a ListView, so the page scrolls instead of overflowing. Each change has a regression test that failed before it. Only the test app changes, so no package version or CHANGELOG update is needed.

Demo

Run the test app on web. Check the badge and drawer header colors with the DevTools color picker, and the expansion tile page in device mode at 320x256.

@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Oct 23, 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 updates accessibility assessment use cases and tests by removing custom colors from the badge, refactoring the drawer header to use theme-based colors, and changing the expansion tile body to a scrollable ListView. It also introduces new tests for text contrast ratios and viewport reachability, along with utility functions for contrast calculation. Feedback highlights a compilation error in expansion_tile_test.dart due to the non-existent tester.view.reset method, and suggests avoiding force-unwrapping potentially null text style properties in test_utils.dart.

testWidgets('expansion tiles stay reachable at a 320x256 viewport', (WidgetTester tester) async {
tester.view.physicalSize = const Size(320, 256);
tester.view.devicePixelRatio = 1.0;
addTearDown(tester.view.reset);

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.

high

The TestFlutterView class (accessed via tester.view) does not have a general reset() method. Calling tester.view.reset will result in a compilation error. Instead, you should explicitly reset the modified properties (physicalSize and devicePixelRatio) using resetPhysicalSize() and resetDevicePixelRatio().

Suggested change
addTearDown(tester.view.reset);
addTearDown(() {
tester.view.resetPhysicalSize();
tester.view.resetDevicePixelRatio();
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

TestFlutterView.reset() exists (flutter_test/lib/src/window.dart) and resets physicalSize, devicePixelRatio and the other view overrides in one call. It is the pattern framework tests use with addTearDown(tester.view.reset), and this test compiles and passes on CI. Keeping it as is.

Comment on lines +43 to +46
final RichText richText = tester.widget<RichText>(
find.descendant(of: text, matching: find.byType(RichText)),
);
return richText.text.style!.color!;

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.

medium

Using the force-unwrap operator ! on richText.text.style and style.color can lead to unhandled null pointer exceptions during tests if the text style or color is not explicitly set on the widget. It is safer to use null-aware operators and throw a descriptive StateError if the color cannot be resolved, which makes debugging test failures much easier.

  final RichText richText = tester.widget<RichText>(
    find.descendant(of: text, matching: find.byType(RichText)),
  );
  final Color? color = richText.text.style?.color;
  if (color == null) {
    throw StateError('The text style or color is null for the found RichText widget.');
  }
  return color;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a test helper. If the style or color is null the test fails right here with a stack trace naming paintedTextColor, which is the behavior we want, so the explicit StateError adds code without changing the signal. For the Material text these tests read, RichText.text.style is the merged effective style and the color is always resolved.

@github-actions github-actions Bot added p: material_ui triage-design Should be looked at in design triage labels Oct 23, 2026
@flutter-zl
flutter-zl requested a review from mdebbar October 23, 2026 21:36
@mdebbar
mdebbar requested a review from chunhtai October 28, 2026 04:00
@flutter-zl
flutter-zl requested review from hannah-hyj and removed request for chunhtai and mdebbar October 29, 2026 21:27

@hannah-hyj hannah-hyj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@flutter-zl flutter-zl added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 2, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 2, 2026
@auto-submit

auto-submit Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/packages/13409, because Pull request flutter/packages/13409 is not in a mergeable state.

@flutter-zl flutter-zl added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 2, 2026
@auto-submit
auto-submit Bot merged commit cc93a31 into flutter:main Nov 2, 2026
13 checks passed
RamyAkin pushed a commit to RamyAkin/flutter that referenced this pull request Nov 3, 2026
flutter/packages@3f34a6f...1df8447

2026-11-03 [email protected] [webview_flutter] Convert iOS impl
example app to Swift (flutter/packages#13538)
2026-11-03 [email protected] [material_ui]
Temporarily exclude material_ui from non-Wasm web unit tests
(flutter/packages#13537)
2026-11-02 [email protected] [material_ui]
Removes list semantics in some a11y_assessment samples
(flutter/packages#13257)
2026-11-02 [email protected] [material_ui] Use named uniforms
instead of setFloat in InkSparkle (flutter/packages#13445)
2026-11-02 [email protected] [VPAT] Fix contrast and reflow in
three a11y_assessments demos (flutter/packages#13409)
2026-11-02 [email protected]
[two_dimensional_scrollables] Fix pinned cells hit test
(flutter/packages#13376)
2026-11-02 [email protected] [tool] Let the OS pick a port for
chromedriver and wait for exit (flutter/packages#13355)
2026-11-02 [email protected] [vector_graphics] Draw <text> under an RTL ambient
direction (flutter/packages#13408)
2026-11-02 [email protected] [VPAT] Keep a11y_assessments
snackbars open until closed (flutter/packages#13451)
2026-11-02 [email protected] [pigeon] Fix race in event channel
handler removal integration test (flutter/packages#13528)

If this roll has caused a breakage, revert this CL and set the roller
to dry run mode using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected] on the revert to ensure that a
human
is aware of the problem.

To file a bug in Flutter:
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App CICD Run CI/CD p: material_ui triage-design Should be looked at in design triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants