[A11y] Add UI for AccessibilityOverridesPane#9892
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the UI and corresponding tests for the AccessibilityOverridesPane in Flutter DevTools, replacing a placeholder with interactive controls for brightness, text scale, bold text, screen reader debugger, and high contrast. The reviewer suggests two key improvements: simplifying the widget tree in _TextScaleOverride by consolidating two separate ValueListenableBuilder widgets into a single builder, and making the widget tests more robust by finding Switch widgets using their associated labels rather than fragile index-based lookups.
srawlins
left a comment
srawlins
left a comment
There was a problem hiding this comment.
Love it. @kenzieschmoll should also review.
| final theme = Theme.of(context); | ||
| final controller = screenControllers.lookup<AccessibilityController>(); | ||
| return DevToolsAreaPane( | ||
| header: const AreaPaneHeader(title: Text('Accessibility Overrides')), |
There was a problem hiding this comment.
Based on screenshot, it looks like this is causing the border to be rendered twice. You likely need to pass includeTopBorder: false (ditto for the semantics tree pane).
There was a problem hiding this comment.
Consider splitting AccessibilityOverridesPane and AccessibilitySemanticsTreePane into separate files (e.g. accessibility/accessibility_screen.dart, accessibility/overrides_pane.dart, and accessibility/semantics_tree_pane.dart) before this file gets too big :)
| style: theme.subtleTextStyle, | ||
| ), | ||
| const SizedBox(height: denseSpacing), | ||
| ValueListenableBuilder<String>( |
There was a problem hiding this comment.
Since there are set number of options here, I think it might be better for controller.brightness to be an enum instead of a string to allow for exhaustive checks and prevent any typo bugs.
| Text( | ||
| 'Brightness', | ||
| style: theme.boldTextStyle, | ||
| ), | ||
| const SizedBox(height: densePadding), | ||
| Text( | ||
| 'Override the color scheme mode of the app.', | ||
| style: theme.subtleTextStyle, | ||
| ), |
There was a problem hiding this comment.
Consider defining a widget (e.g. _AccessibilityPanelLabel) for easier reuse:
_AccessibilityPanelLabel(
label: 'Brightness',
description: 'Override the color scheme mode of the app.',
)
| ValueListenableBuilder<bool>( | ||
| valueListenable: notifier, | ||
| builder: (context, enabled, _) { | ||
| return Switch( |
There was a problem hiding this comment.
nit: use NotifierSwitch common widget. This enforces DevTools styling and size defaults as part of the implementation
hannah-hyj
added
the
autosubmit
4 participants
This PR is only UI for buttons, no functions yet.
The screen is gated by the FeatureFlags.accessibility flag, internal link: go/devtool-a11y-userjourney
issue: #9893
Pre-launch Checklist
General checklist
///).Issues checklist
contributions-welcomeorgood-first-issuelabel.contributions-welcomeorgood-first-issuelabel. I understand this means my PR might take longer to be reviewed.Tests checklist
AI-tooling checklist
Feature-change checklist
release-notes-not-requiredlabel or left a comment requesting the label be added.packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md.If you need help, consider asking for help on Discord.