Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testing Color Contrast in AlertDialog #148657

Open
anasqadrei opened this issue May 20, 2024 · 6 comments
Open

Testing Color Contrast in AlertDialog #148657

anasqadrei opened this issue May 20, 2024 · 6 comments
Labels
a: tests "flutter test", flutter_test, or one of our tests found in release: 3.22 Found to occur in 3.22 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P3 Issues that are less important to the Flutter project team-framework Owned by Framework team triaged-framework Triaged by Framework team

Comments

@anasqadrei
Copy link

Steps to reproduce

I'm building a complex app with asynchronous code, providers, external libraries, and so on. It works well and passes tests in most cases, except for this one. I created a minimal code example to demonstrate the issue. In my complex app, for some reason, I need to run two pumps for the AlertDialog test to pass. However, only the color contrast test fails. I'm unsure why it fails, as it shouldn't fail regardless of the number of pumps I run.

Expected results

Test should pass

Actual results

Test fails

Code sample

lib/main.dart
import 'package:flutter/material.dart';

void main() => runApp(const AlertDialogExampleApp());

class AlertDialogExampleApp extends StatelessWidget {
  const AlertDialogExampleApp({super.key});

  @override
  Widget build(BuildContext context) {
    return const MaterialApp(
      home: Scaffold(
        body: Center(
          child: DialogExample(),
        ),
      ),
    );
  }
}

class DialogExample extends StatelessWidget {
  const DialogExample({super.key});

  @override
  Widget build(BuildContext context) {
    return TextButton(
      child: const Text('Show Dialog'),
      onPressed: () => showDialog<String>(
        context: context,
        builder: (BuildContext context) => AlertDialog(
          title: const Text('Title'),
          content: const Text('Alert dialog description'),
          actions: <Widget>[
            TextButton(
              onPressed: () => Navigator.pop(context, 'OK'),
              child: const Text('OK'),
            ),
          ],
        ),
      ),
    );
  }
}
test/widget_test.dart
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';

import 'package:flutter_dialog_color_contrast/main.dart';

void main() {
  testWidgets('Test AlertDialog Color Contrast', (tester) async {
    final SemanticsHandle semanticsHandle = tester.ensureSemantics();
    await tester.pumpWidget(const AlertDialogExampleApp());

    // Before showing dialog. Test passes
    expect(find.text('Show Dialog'), findsOneWidget);
    expect(find.byType(AlertDialog), findsNothing);
    expect(find.text('Alert dialog description'), findsNothing);
    expect(find.text('OK'), findsNothing);

    // Tap to show AlertDialog
    await tester.tap(find.text('Show Dialog'));

    // In order for my complex app to pass tests, it requires two pumps as below
    await tester.pump();
    // This 2nd pump causes the color contrast test failure. Test will pass if commented
    await tester.pump(Durations.short1);

    // After showing dialog. Test passes
    expect(find.byType(AlertDialog), findsOneWidget);
    expect(find.text('Alert dialog description'), findsOneWidget);
    expect(find.text('OK'), findsOneWidget);

    // Color contrast test fails
    await expectLater(tester, meetsGuideline(textContrastGuideline));

    semanticsHandle.dispose();
  });
}

Screenshots or Video

No response

Logs

Logs
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: Text contrast should follow WCAG guidelines
  Actual: <Instance of 'WidgetTester'>
   Which: SemanticsNode#10(Rect.fromLTRB(24.0, 72.0, 366.0, 92.0), label: "Alert dialog
description", textDirection: ltr):
          Expected contrast ratio of at least 4.5 but found 3.57 for a font size of 14.0.
          The computed colors was:
          light - Color(0xffcecad0), dark - Color(0xff676568)
          See also: https://www.w3.org/TR/UNDERSTANDING-WCAG20/visual-audio-contrast-contrast.html
          SemanticsNode#11(Rect.fromLTRB(302.0, 116.0, 366.0, 164.0), actions: [tap], flags:
[isButton, hasEnabledState, isEnabled, isFocusable], label: "OK", textDirection: ltr):
          Expected contrast ratio of at least 4.5 but found 2.27 for a font size of 14.0.
          The computed colors was:
          light - Color(0xffcecad0), dark - Color(0xff8c7fa9)
          See also: https://www.w3.org/TR/UNDERSTANDING-WCAG20/visual-audio-contrast-contrast.html

When the exception was thrown, this was the stack:
#0      fail (package:matcher/src/expect/expect.dart:149:31)
#1      _expect.<anonymous closure> (package:matcher/src/expect/expect.dart:125:9)
<asynchronous suspension>
<asynchronous suspension>
#8      expectLater.<anonymous closure> (package:flutter_test/src/widget_tester.dart:512:24)
<asynchronous suspension>
#9      main.<anonymous closure> (file:///Users/anasqaderi/Coding/play/flutter_dialog_color_contrast/test/widget_test.dart:31:5)
<asynchronous suspension>
#10     testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:183:15)
<asynchronous suspension>
#11     TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1017:5)
<asynchronous suspension>
<asynchronous suspension>
(elided 7 frames from dart:async and package:stack_trace)

The test description was:
  Test AlertDialog Color Contrast
════════════════════════════════════════════════════════════════════════════════════════════════════

Flutter Doctor output

Doctor output
[✓] Flutter (Channel stable, 3.19.6, on macOS 14.4.1 23E224 darwin-arm64, locale en-AU)
    • Flutter version 3.19.6 on channel stable at /Users/anasqaderi/Coding/exe/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 54e66469a9 (5 weeks ago), 2024-04-17 13:08:03 -0700
    • Engine revision c4cd48e186
    • Dart version 3.3.4
    • DevTools version 2.31.1

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
    • Android SDK at /Users/anasqaderi/Library/Android/sdk
    • Platform android-34, build-tools 34.0.0
    • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b829.9-10027231)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 15.4)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 15F31d
    • CocoaPods version 1.14.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2022.3)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b829.9-10027231)

[✓] VS Code (version 1.89.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.88.0

[✓] Connected device (4 available)
    • Anas iPhone 15 Pro Max (mobile) • 00008130-001079A10A98001C • ios            • iOS 17.4.1 21E236
    • Anas iPhone XS Max (mobile)     • 00008020-000E6CCA1104002E • ios            • iOS 17.5 21F79
    • macOS (desktop)                 • macos                     • darwin-arm64   • macOS 14.4.1 23E224 darwin-arm64
    • Chrome (web)                    • chrome                    • web-javascript • Google Chrome 124.0.6367.209

[✓] Network resources
    • All expected network resources are available.

• No issues found!
@danagbemava-nc danagbemava-nc added the in triage Presently being triaged by the triage team label May 20, 2024
@danagbemava-nc
Copy link
Member

Reproducible using the code sample provided above.

flutter doctor -v
[✓] Flutter (Channel stable, 3.22.0, on macOS 14.4.1 23E224 darwin-arm64, locale en-GB)
    • Flutter version 3.22.0 on channel stable at /Users/nexus/dev/sdks/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 5dcb86f68f (11 days ago), 2024-05-09 07:39:20 -0500
    • Engine revision f6344b75dc
    • Dart version 3.4.0
    • DevTools version 2.34.3

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
    • Android SDK at /Users/nexus/Library/Android/sdk
    • Platform android-34, build-tools 34.0.0
    • Java binary at: /Users/nexus/Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.7+0-17.0.7b1000.6-10550314)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 15.3)
    • Xcode at /Applications/Xcode-15.3.0.app/Contents/Developer
    • Build 15E204a
    • CocoaPods version 1.15.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2023.1)
    • Android Studio at /Users/nexus/Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.7+0-17.0.7b1000.6-10550314)

[✓] IntelliJ IDEA Ultimate Edition (version 2023.2.5)
    • IntelliJ at /Users/nexus/Applications/IntelliJ IDEA Ultimate.app
    • Flutter plugin version 77.2.2
    • Dart plugin version 232.10286

[✓] VS Code (version 1.89.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.88.0

[✓] Connected device (6 available)
    • Pixel 7 (mobile)                • adb-28291FDH2001SA-5Lv71w._adb-tls-connect._tcp. • android-arm64  • Android 14 (API 34)
    • Nexus (mobile)                  • 00008020-001875E83A38002E                        • ios            • iOS 17.4.1 21E236
    • Dean’s iPad (mobile)            • 00008103-000825C811E3401E                        • ios            • iOS 17.4.1 21E236
    • macOS (desktop)                 • macos                                            • darwin-arm64   • macOS 14.4.1 23E224 darwin-arm64
    • Mac Designed for iPad (desktop) • mac-designed-for-ipad                            • darwin         • macOS 14.4.1 23E224 darwin-arm64
    • Chrome (web)                    • chrome                                           • web-javascript • Google Chrome 124.0.6367.208

[✓] Network resources
    • All expected network resources are available.

• No issues found!
[!] Flutter (Channel master, 3.22.0-36.0.pre.54, on macOS 14.4.1 23E224 darwin-arm64, locale en-GB)
    • Flutter version 3.22.0-36.0.pre.54 on channel master at /Users/nexus/dev/sdks/flutters
    ! Warning: `flutter` on your path resolves to /Users/nexus/dev/sdks/flutter/bin/flutter, which is not inside your current Flutter SDK checkout at /Users/nexus/dev/sdks/flutters. Consider adding /Users/nexus/dev/sdks/flutters/bin to the front of your path.
    ! Warning: `dart` on your path resolves to /Users/nexus/dev/sdks/flutter/bin/dart, which is not inside your current Flutter SDK checkout at /Users/nexus/dev/sdks/flutters. Consider adding /Users/nexus/dev/sdks/flutters/bin to the front of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 414d923872 (11 hours ago), 2024-05-19 21:58:34 -0400
    • Engine revision c6fecf65fb
    • Dart version 3.5.0 (build 3.5.0-169.0.dev)
    • DevTools version 2.36.0-dev.10
    • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades.

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
    • Android SDK at /Users/nexus/Library/Android/sdk
    • Platform android-34, build-tools 34.0.0
    • Java binary at: /Users/nexus/Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.7+0-17.0.7b1000.6-10550314)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 15.3)
    • Xcode at /Applications/Xcode-15.3.0.app/Contents/Developer
    • Build 15E204a
    • CocoaPods version 1.15.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2023.1)
    • Android Studio at /Users/nexus/Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.7+0-17.0.7b1000.6-10550314)

[✓] IntelliJ IDEA Ultimate Edition (version 2023.2.5)
    • IntelliJ at /Users/nexus/Applications/IntelliJ IDEA Ultimate.app
    • Flutter plugin version 77.2.2
    • Dart plugin version 232.10286

[✓] VS Code (version 1.89.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.88.0

[✓] Connected device (6 available)
    • Pixel 7 (mobile)                • adb-28291FDH2001SA-5Lv71w._adb-tls-connect._tcp. • android-arm64  • Android 14 (API 34)
    • Nexus (mobile)                  • 00008020-001875E83A38002E                        • ios            • iOS 17.4.1 21E236
    • Dean’s iPad (mobile)            • 00008103-000825C811E3401E                        • ios            • iOS 17.4.1 21E236
    • macOS (desktop)                 • macos                                            • darwin-arm64   • macOS 14.4.1 23E224 darwin-arm64
    • Mac Designed for iPad (desktop) • mac-designed-for-ipad                            • darwin         • macOS 14.4.1 23E224 darwin-arm64
    • Chrome (web)                    • chrome                                           • web-javascript • Google Chrome 124.0.6367.208

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 1 category.

@danagbemava-nc danagbemava-nc added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on team-framework Owned by Framework team found in release: 3.22 Found to occur in 3.22 and removed in triage Presently being triaged by the triage team labels May 20, 2024
@LimaneGaya
Copy link

It works with await tester.pump(Durations.short2);
Maybe Durations.short1 is not enough to display properly

@bleroux
Copy link
Contributor

bleroux commented May 20, 2024

Once showDialog is called, the dialog is displayed using a FadeTransition which animates the dialog opacity. The default duration of the animation is 150ms, this probably explains why at 50ms (Durations.short1) the color contrast test fails and why it succeeds at 100ms (Durations.short2).

@anasqadrei Any reason for not relying on a longer duration? It would make sense to wait for the dialog to be fully visible before checking color contrast.

@anasqadrei
Copy link
Author

In my original test, I am testing for semantics too. After some investigation, I found out that pump(Durations.short1) was needed for expect(find.bySemanticsLabel('OK'), findsOneWidget) test to pass.

Consider the below lines after tapping show dialog button. I ran these tests individually (running one section and commenting the rest)

    // Passes
    await tester.pump();
    expect(find.text('OK'), findsOneWidget);

    // Passes
    await tester.pump(Durations.short1);
    expect(find.text('OK'), findsOneWidget);

    // Fails
    await tester.pump();
    expect(find.bySemanticsLabel('OK'), findsOneWidget);

    // Fails
    await tester.pump(Durations.short1);
    expect(find.bySemanticsLabel('OK'), findsOneWidget);

    // Passes
    await tester.pump();
    await tester.pump(Durations.short1);
    expect(find.bySemanticsLabel('OK'), findsOneWidget);

    // Passes
    await tester.pump();
    await expectLater(tester, meetsGuideline(textContrastGuideline));

    // Passes
    await tester.pump(Durations.short1);
    await expectLater(tester, meetsGuideline(textContrastGuideline));

    // Passes
    await tester.pump(Durations.short3);
    await expectLater(tester, meetsGuideline(textContrastGuideline));

    // Fails
    await tester.pump();
    await tester.pump(Durations.short1);
    await expectLater(tester, meetsGuideline(textContrastGuideline));

    // Passes
    await tester.pump();
    await tester.pump(Durations.short3);
    await expectLater(tester, meetsGuideline(textContrastGuideline));

I'm a bit puzzled here

Question 1: Why did find.text('OK') pass without waiting for animation to finish?
Question 2: Why did find.bySemanticsLabel('OK') need two pumps(one without duration and one with) to pass?
Question 3: Why did textContrastGuideline pass using one pump(regardless of duration)?
Question 4: Shouldn't we use only one pump(either with duration or without) for all three tests(text, semantics label & colour contrast) to pass?

@goderbauer
Copy link
Member

To debug this you can print the semantics tree with debugDumpSemanticsTree and the widget tree with debugDumpApp to see what state the app is in after each pump.

@goderbauer goderbauer added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label May 21, 2024
@anasqadrei
Copy link
Author

To narrow down the problem, I'm wondering why finding a text takes one pump while finding its corresponding semantics label takes two. If text is pumped then its semantics label should go with it, shouldn't it? Why is there an inconsistency?

  1. If I run one pump
await tester.pump();
debugDumpApp();
debugDumpSemanticsTree();
expect(find.text('Alert dialog description'), findsOneWidget); // Passes. Shouldn't it fail because it NEEDS-PAINT
expect(find.bySemanticsLabel('Alert dialog description'), findsOneWidget); // Fails

I get this snippet from debugDumpApp. It shows that text "Alert dialog description" is there and it's under a semantics container with a NEEDS-PAINT

│  └Semantics(container: true, properties: SemanticsProperties, renderObject: RenderSemanticsAnnotations#64e30 relayoutBoundary=up9 NEEDS-PAINT)
│   └Text("Alert dialog description", dependencies: [DefaultSelectionStyle, DefaultTextStyle, MediaQuery])
│    └RichText(softWrap: wrapping at box width, maxLines: unlimited, text: "Alert dialog description", dependencies: [Directionality, _LocalizationsScope-[GlobalKey#e4e16]], renderObject: RenderParagraph#b28cd relayoutBoundary=up10 NEEDS-PAINT)

and I get this from debugDumpSemanticsTree that doesn't show the label "Alert dialog description" anywhere

SemanticsNode#0
 │ Rect.fromLTRB(0.0, 0.0, 2400.0, 1800.0)
 │
 └─SemanticsNode#1
   │ Rect.fromLTRB(0.0, 0.0, 800.0, 600.0) scaled by 3.0x
   │ textDirection: ltr
   │
   ├─SemanticsNode#6
   │   Rect.fromLTRB(0.0, 0.0, 800.0, 600.0)
   │   sortKey: OrdinalSortKey#83bb4(order: 0.0)
   │
   └─SemanticsNode#5
       Rect.fromLTRB(0.0, 0.0, 800.0, 600.0)
       actions: dismiss, tap
       label: "Dismiss"
       textDirection: ltr
       sortKey: OrdinalSortKey#c7b52(order: 1.0)
  1. If I run two pumps
await tester.pump();
await tester.pump(Durations.short1);
debugDumpApp();
debugDumpSemanticsTree();
expect(find.text('Alert dialog description'), findsOneWidget); // Passes
expect(find.bySemanticsLabel('Alert dialog description'), findsOneWidget); // Passes

I get this snippet from debugDumpApp. It's similar to above but without NEEDS-PAINT

│  └Semantics(container: true, properties: SemanticsProperties, renderObject: RenderSemanticsAnnotations#b6ba5 relayoutBoundary=up9)
│   └Text("Alert dialog description", dependencies: [DefaultSelectionStyle, DefaultTextStyle, MediaQuery])
│    └RichText(softWrap: wrapping at box width, maxLines: unlimited, text: "Alert dialog description", dependencies: [Directionality, _LocalizationsScope-[GlobalKey#06191]], renderObject: RenderParagraph#78d80 relayoutBoundary=up10)

and I get this from debugDumpSemanticsTree showing the label "Alert dialog description"

SemanticsNode#0
 │ Rect.fromLTRB(0.0, 0.0, 2400.0, 1800.0)
 │
 └─SemanticsNode#1
   │ Rect.fromLTRB(0.0, 0.0, 800.0, 600.0) scaled by 3.0x
   │ textDirection: ltr
   │
   ├─SemanticsNode#6
   │ │ Rect.fromLTRB(0.0, 0.0, 800.0, 600.0)
   │ │ sortKey: OrdinalSortKey#e5c66(order: 0.0)
   │ │
   │ └─SemanticsNode#7
   │   │ Rect.fromLTRB(0.0, 0.0, 800.0, 600.0)
   │   │ flags: scopesRoute
   │   │
   │   └─SemanticsNode#8
   │     │ Rect.fromLTRB(205.0, 206.0, 595.0, 394.0)
   │     │ flags: scopesRoute, namesRoute
   │     │ label: "Alert"
   │     │ textDirection: ltr
   │     │ elevation: 6.0
   │     │
   │     ├─SemanticsNode#9
   │     │   Rect.fromLTRB(24.0, 24.0, 366.0, 56.0)
   │     │   label: "Title"
   │     │   textDirection: ltr
   │     │
   │     ├─SemanticsNode#10
   │     │   Rect.fromLTRB(24.0, 72.0, 366.0, 92.0)
   │     │   label: "Alert dialog description"
   │     │   textDirection: ltr
   │     │
   │     └─SemanticsNode#11
   │         Rect.fromLTRB(302.0, 116.0, 366.0, 164.0)
   │         actions: tap
   │         flags: isButton, hasEnabledState, isEnabled, isFocusable
   │         label: "OK"
   │         textDirection: ltr
   │
   └─SemanticsNode#5
       Rect.fromLTRB(0.0, 0.0, 800.0, 600.0)
       actions: dismiss, tap
       label: "Dismiss"
       textDirection: ltr
       sortKey: OrdinalSortKey#160d8(order: 1.0)

From the debugs I did above, it seems showing an AlertDialog takes few steps which is okay but there are inconsistencies with text, their semantics labels and colour contrast.

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label May 24, 2024
@goderbauer goderbauer added P3 Issues that are less important to the Flutter project triaged-framework Triaged by Framework team labels May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: tests "flutter test", flutter_test, or one of our tests found in release: 3.22 Found to occur in 3.22 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P3 Issues that are less important to the Flutter project team-framework Owned by Framework team triaged-framework Triaged by Framework team
Projects
None yet
Development

No branches or pull requests

5 participants