Skip to content

Set highContrast value from AccessibilityFeatures into MediaQueryData#48811

Merged
fluttergithubbot merged 3 commits intoflutter:masterfrom
miquelbeltran:mb-highcontrast-mediaquerydata
Feb 26, 2020
Merged

Set highContrast value from AccessibilityFeatures into MediaQueryData#48811
fluttergithubbot merged 3 commits intoflutter:masterfrom
miquelbeltran:mb-highcontrast-mediaquerydata

Conversation

@miquelbeltran
Copy link
Member

@miquelbeltran miquelbeltran commented Jan 14, 2020 •
edited by goderbauer
Loading

Description

Part of the efforts to fix the issue #48418 split in a new PR as suggested by @Piinks in #48486

The highContrast property is going to be added to the class AccessibilityFeatures in the PR: flutter/engine#15343

Here, I set it to the MediaQueryData class so it can be used by Flutter developers by querying: MediaQuery.of(context).highContrast

Design document: https://flutter.dev/go/cupertino-increase-contrast

Related Issues

Blocked by: flutter/engine#15343

Fixes: #48418

Related to: #48486

Tests

Tests are covered in #48486

Checklist

Checklist covered in #48486

Breaking Change

Change info covered in #48486

@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Jan 14, 2020

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@fluttergithubbot fluttergithubbot added the framework flutter/packages/flutter repository. See also f: labels. label Jan 14, 2020
@miquelbeltran miquelbeltran changed the title Set highContrast value from AccessibilityFeatures Set highContrast value from AccessibilityFeatures into MediaQueryData Jan 14, 2020
@goderbauer goderbauer requested a review from Piinks January 15, 2020 22:41
@Piinks
Copy link
Contributor

Piinks commented Jan 15, 2020

This is waiting on a few changes to land first.

@Piinks Piinks added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) platform-ios iOS applications specifically a: tests "flutter test", flutter_test, or one of our tests labels Feb 21, 2020
@goderbauer
Copy link
Member

goderbauer commented Feb 24, 2020

Looks like the blocking engine PR was submitted (flutter/engine#15343). Is this one ready to move forward?

Also, is it possible to add a test for this?

@miquelbeltran
Copy link
Member Author

miquelbeltran commented Feb 25, 2020

Hi @goderbauer

I looked into the possibility to add tests covering this code change, but I didn't see it done for the other MediaQueryData parameters (like disableAnimations or boldText), maybe you can point me to one, or I can write a new test case on verifying that MediaQueryData is created correctly using window.accessibilityFeatures.

@Piinks
Copy link
Contributor

Piinks commented Feb 25, 2020

Hey @miquelbeltran, This PR will need to be updated with the latest from master.
Regarding the tests, I made a similar change (#33999) that may be helpful. Tests like

flutter/packages/flutter/test/widgets/media_query_test.dart

Line 74 in 1ec2a43

testWidgets('MediaQueryData.copyWith defaults to source', (WidgetTester tester) async {

should provide the appropriate coverage for this change. Several of the tests in that file make sure the property works as intended with the expected values. Let me know if that is helpful in creating tests for this. :)

@miquelbeltran
Copy link
Member Author

miquelbeltran commented Feb 25, 2020

Thanks for the hint @Piinks! I added the highContrast field to the existing tests, which was missing.

@Piinks
Copy link
Contributor

Piinks commented Feb 25, 2020

While you are here, can you re-apply the change to the FakeAccessibilityFeatures class? I removed them so the engine change could land: https://github.com/flutter/flutter/pull/51130/files

This has been a tricky change to land in a non-breaking way, and while you were at it you made it easier for others that will make changes like this in the future. Thanks for this. It's just about ready. :)

Piinks
Piinks approved these changes Feb 26, 2020
View reviewed changes
Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

Flutter_LGTM
Thank you again for all of your work on this!

@fluttergithubbot fluttergithubbot merged commit 7ff3a50 into flutter:master Feb 26, 2020
@miquelbeltran miquelbeltran deleted the mb-highcontrast-mediaquerydata branch February 27, 2020 07:15
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Piinks Piinks Piinks approved these changes

Assignees

No one assigned

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. platform-ios iOS applications specifically

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

MediaQuery highContrast is always false

6 participants

@miquelbeltran @fluttergithubbot @Piinks @goderbauer @jmagman @googlebot