Read out text fields#2131
Conversation
| key: keys.issuerField, | ||
| controller: _issuerController, | ||
| autofocus: widget.credentialData == null, | ||
| //autofocus: widget.credentialData == null, |
There was a problem hiding this comment.
I’m curious about the autofocus behaviour you're seeing. I ran a quick test with NVDA on Windows, and it successfully announced the label text upon focus. What screen reader are you using? If it’s a specific bug there, it would be nice if we can find a workaround that lets us keep autofocus enabled, for UX consistency.
There was a problem hiding this comment.
Pull request overview
Improves VoiceOver/screen reader behavior by adding explicit semantics to dialog titles and text fields so their labels/titles are announced more reliably.
Changes:
- Wraps
ResponsiveDialogtitles in aSemanticswidget for both fullscreen and dialog layouts. - Wraps
AppTextFieldin aSemanticswidget, using the input decoration’slabelTextas the accessibility label.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lib/widgets/responsive_dialog.dart | Adds a helper to wrap dialog/app bar titles with Semantics to influence screen reader announcement. |
| lib/widgets/app_text_field.dart | Wraps text fields with Semantics(label: ...) to encourage VoiceOver to announce the field label. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Widget _wrapTitleWithSemantics(Widget? title) { | ||
| if (title == null) return const SizedBox.shrink(); |
There was a problem hiding this comment.
_wrapTitleWithSemantics() returning a non-null widget for a null title changes behavior: AppBar/AlertDialog will now treat the title as present and can apply title padding/spacing even when the caller intended title: null (e.g., empty top padding in AlertDialog). Consider changing the helper to return Widget? and return null when title is null, and only wrap when non-null.
| Widget _wrapTitleWithSemantics(Widget? title) { | |
| if (title == null) return const SizedBox.shrink(); | |
| Widget? _wrapTitleWithSemantics(Widget? title) { | |
| if (title == null) return null; |
|
|
||
| Widget _wrapTitleWithSemantics(Widget? title) { | ||
| if (title == null) return const SizedBox.shrink(); | ||
| return Semantics(enabled: true, onTap: () {}, child: title); |
There was a problem hiding this comment.
The Semantics wrapper adds an onTap action with an empty callback. This will expose the title as an actionable element to assistive tech (often announced as a button) but tapping it does nothing, which is confusing and can fail accessibility expectations. Prefer making the title readable via semantics properties that don’t imply an action (e.g., header/focusable/label), or wire onTap to a real interaction if one exists.
| return Semantics(enabled: true, onTap: () {}, child: title); | |
| return Semantics(header: true, child: title); |
| child: Semantics( | ||
| label: labelText, | ||
| enabled: true, | ||
| child: DefaultSelectionStyle( | ||
| selectionColor: decoration?.errorText != null | ||
| ? Theme.of(context).colorScheme.error | ||
| : null, | ||
| child: this, |
There was a problem hiding this comment.
The Semantics wrapper forces enabled: true unconditionally. If the underlying TextField is disabled (enabled: false), screen readers will still treat it as enabled, which is incorrect. Remove enabled here or bind it to the TextField’s enabled state (and/or let TextField provide its own enabled semantics).
|
This is a bug in Flutter. See flutter/flutter#120779 |
Makes VoiceOver read out the text field. However this fix may not be optimal as it's not the most intuitive.
Issues: