A11y high contrast #34276
A11y high contrast #34276
Conversation
flutter-dashboard
bot
added
the
platform-web
yjbanov
left a comment
yjbanov
left a comment
There was a problem hiding this comment.
Just a bunch of nitpicks, suggestions, and pointers. None of this is blocking, so LGTM.
|
|
||
| /// Reference to css media query that indicates whether high contrast is on. | ||
| final html.MediaQueryList _highContrastMediaQuery = | ||
| html.window.matchMedia(_highContrastMediaQueryString); |
There was a problem hiding this comment.
Does this need to be cleaned up in some way when the instance of HighContrastSupport is no longer needed? For example, when hot-restart is invoked we will reinitialize the state of the Dart program. Would that create a dangling MediaQueryList from a previous iteration of the app?
Here's an example of a class that performs hot restart clean-up:
|
|
||
| /// Updates [_highContrast] and invokes [onHighContrastModeChanged] | ||
| /// callback if [_highContrast] changed. | ||
| void _updateHighContrast(bool value) { |
There was a problem hiding this comment.
Can we simplify the logic here by calling initializeAccessibilityFeatures?
| void dispose() { | ||
| _removeBrightnessMediaQueryListener(); | ||
| _disconnectFontSizeObserver(); | ||
| HighContrastSupport.instance.removeListener(_updateHighContrast); |
There was a problem hiding this comment.
Oh, I see that this covers the hot-restart case. This is unusual because typically our singletons manage their own lifecycle w.r.t. to hot-restart, but in this case the lifecycle of HighContrastSupport is owned by EnginePlatformDispatcher. It not necessarily a bug, but could be confusing because it deviates from other similar classes. I think the initialization of HighContrastSupport may need to change for it to self-manage. Feel free to proceed how you see fit.
There was a problem hiding this comment.
Can you elaborate on "I think the initialization of HighContrastSupport may need to change for it to self-manage."?
There was a problem hiding this comment.
What I meant was that HighContrastSupport would need something like ensureInitialized method that creates the singleton and attaches a hot-restart listener that cleans up the singleton, similar to how KeyboardBinding does it.
3 participants
Add support for high contrast on Windows. Previously, the value of high contrast flag in AccessibilityFeatures was always false. This PR addresses the issue by reading the value from the browser using mediaQuery of
forced-colors: activeand updating AccessibilityFeatures accordingly.Fixes flutter/flutter#104272
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.