TalkBack, 9 of 10: Improve main server list accessibility - #6167
eliotcougar wants to merge 16 commits into
Conversation
|
For the main server list, I think we should preserve the behavior from PR #6108 exactly, without introducing any further changes to the server-selection semantics. In particular: Since #6108 has already been merged into master, I would prefer #6167 to leave that part of the implementation exactly as it is in the current upstream behavior rather than replacing it with selectable/RadioButton semantics. The other accessibility improvements in #6167 can remain if they do not alter the server-selection semantics. I tested the current combined accessibility build with TalkBack. With #6167, every unselected server is announced as "Not selected ... Radio button", which is unnecessarily verbose and makes navigating the server list much more difficult. The behavior from #6108 did not have this problem. So for the main server list, please preserve #6108's behavior exactly rather than trying to make TalkBack's generated RadioButton announcement match the preferred wording. |
|
It may sound disappointing, but our previous implementation was full of unsupported hacks. That's why it was rejected. According to Android accessibility guidelines, the app should not suppress the control role (switch, checkbox, tab, button) only to prevent it from being spoken. Even more so, the app should not take over the TalkBack job by using ClearAndSetSemantics() and constructing the spoken sentence manually (that we did a lot in many places). That function should be used sparingly, and is strongly discouraged. According to guidelines, the objective of TalkBack is not to "sound good", it is to communicate semantic state of the app UI to accessibility devices. The pattern "state - content - role" is hardcoded and the app should not mess with it. Some of your concerns in adjacent PRs are valid and I'll try to address them. |
The thing is, this time I'm not replacing it with anything. If you think about it, the server row items are, semantically, radio buttons, despite looking different: it is a list of items where only one item can be selected at a time. |
|
I understand your point that the server rows represent a single-choice list semantically. However, I would like to request that the accessibility behavior from #6108 be restored as-is. In #6108, the server rows were not exposed as RadioButtons, and TalkBack did not announce "Not selected" and "Radio button" for every unselected server. This made navigating through a list of many servers significantly less verbose and much more comfortable in real-world TalkBack use. The current implementation introduces RadioButton semantics for every server row, which causes TalkBack to announce "Not selected" and "Radio button" for every unselected server. This is unnecessarily verbose and becomes quite annoying when navigating through a large server list. I am not asking for custom speech, ClearAndSetSemantics(), or any manual manipulation of TalkBack's output. I am specifically asking to restore the accessibility behavior that already existed in #6108, which worked well in practice. Please restore the accessibility behavior of #6108 for the server list without introducing the RadioButton role for these rows. |
|
@aliRahimi1997 What do you think about this implementation? I cannot test it on my device at the moment... I'm starting to think that this is the most correct approach to list items + attached buttons. Make buttons inert, and move their actions into custom actions (Swipe up/down)... According to the guidelines, this is preferred approach to navigating long lists. |
|
I think the new approach is cleaner for navigating a long server list, and I understand the rationale behind moving the actions to TalkBack's Actions menu. My only concern is that this changes an interaction pattern that blind users have been familiar with for a long time. Some users may initially find it less convenient to access Edit/Delete/Share through the Actions menu instead of having those buttons directly available next to each server. That said, I think the new approach is worth considering, especially if we can keep the selected-state announcement clear. The main issue I noticed in my testing is that "Selected" is currently no longer announced. |
|
Would it be possible to keep the new Actions-menu approach while also providing a long-press alternative for users who are accustomed to interacting with the Edit/Delete/Share controls directly on each server? I don't think this is necessarily required, but it might make the transition easier for existing blind users who are used to those actions being directly associated with each server. |
In order to enable per-row navigation, the individual action buttons have to be removed from the accessibility graph. Unfortunately, they also become inaccessible directly. It's difficult to find a good compromise. Navigation through the server list is much easier, but at the cost of unfamiliar UI. Also, custom actions menu can have additional "move to top/bottom" and "move up/down" actions for reordering servers in a more accessible way. I think it is more convenient for completely blind users, but those with some remaining vision would tap the button and it will not work as a button. |
|
Thanks for the explanation. I understand the trade-off much better now. I tested the current implementation with TalkBack on my device. Overall, I think the new approach works well for navigating a long server list. The individual Edit/Delete/Share buttons are no longer part of the TalkBack swipe navigation, so swiping moves directly from one server to the next. The actions are still available through TalkBack's Actions menu, which works correctly. I also think the additional custom actions you mentioned, such as Move up, Move down, Move to top, and Move to bottom, could be especially useful for completely blind users. I agree that this changes the interaction model for users with some remaining vision. They may still see the Edit/Delete/Share buttons next to each server and naturally expect them to behave as buttons. I think this is worth considering because existing users may need some time to get used to the new interaction. One possible compromise came to mind. Would it be possible to keep the Edit/Delete/Share buttons visually present for sighted and low-vision users, while keeping those individual buttons out of the TalkBack accessibility graph? TalkBack users could continue to access the same actions through the Actions menu. Additionally, the server row could support a long-press action that opens a menu with the same actions. This could provide a familiar direct interaction for users who prefer gestures, without adding the individual action buttons back into TalkBack's swipe navigation. One clarification about this idea: if we implement a long-press menu, would it make sense for it to expose the same full set of actions as the TalkBack Actions menu, including Move up/down and Move to top/bottom, or would those additional reordering actions be intended to remain TalkBack-only? That would give us something like:
I realize this may add some complexity, so I don't know whether it would be the best implementation. I'm mainly suggesting it as a possible compromise between the two interaction models. There is also one issue I noticed during my testing that I think is important to preserve: the selected-state announcement. The RadioButton announcement is now gone, which is good. However, the "Selected" announcement has disappeared as well. Previously, TalkBack announced the currently selected server as "Selected", but the current implementation no longer announces the selected state. Ideally, I think the behavior should be:
The "Not selected" announcement was particularly verbose and annoying when navigating through a long list, so I would strongly prefer not to reintroduce it. If we can preserve the selected-state indication while keeping the new per-row navigation and Actions-menu approach, I think this would provide a very good experience for completely blind users while also minimizing the impact on sighted and low-vision users. |
I don't know what's the "long-press menu"... The Actions menu that you can access with three-finger tap always contains the same items as the custom actions accessible with vertical swipe.
It was there, but only when VPN is connected. Now it's fixed. |
|
I tested the latest implementation extensively with TalkBack. Everything works correctly from an accessibility perspective:
One additional thought: many blind TalkBack users are accustomed to long-pressing an item to access its available actions. I understand that the current implementation intentionally uses TalkBack Custom Actions, but would it be possible to also support a long-press action/menu on the server row, without interfering with the current Actions menu and swipe navigation? In any case, based on my testing, the current Custom Actions implementation itself works very well for blind users. |
I don't see it anywhere in the TalkBack settings. Holding is announcing the item that is currently under the finger. It does not open any menu if you hold it... The Action menu is accessed by tapping with three fingers and selecting the first menu item - Actions. Maybe we're using different TalkBack versions? If you're suggesting to implement a separate accessibility menu for long-press, then it's not feasible as the long-press is already used to activate "drag reorder"… |
|
Thanks for checking this and for clarifying. What I meant by long-press was not to replace TalkBack's Actions menu, and I wasn't suggesting that the existing action buttons next to each server row should be removed either. I was wondering if it would be possible to support all of these interaction methods together:
For example, a long-press could open the actions for that server, such as Edit, Delete, QR Code, Copy Link, Copy Profile Link, and Copy Full Configuration. I think this could be useful for both sighted and TalkBack users. The visible buttons would remain available as they are now, while long-press could provide a convenient shortcut to the same actions. TalkBack's Actions menu would also remain available as the accessibility-specific way to access them. I also think this could be particularly convenient for blind users, because long-pressing directly on the server row can be easier and more straightforward than navigating through TalkBack's Actions menu to find the available actions. So I'm not suggesting replacing one interaction with another. The idea is simply to provide multiple ways to access the same server actions, if the existing UI architecture allows it. |
Reuse 2dust#6167's reviewed Selected translations in the shared locale catalogs. This removes the remaining English fallback and keeps focused accessibility branches conflict-free when merged in either order.
Reuse 2dust#6167's reviewed Selected translations in the shared locale catalogs. This removes the remaining English fallback and keeps focused accessibility branches conflict-free when merged in either order.
1951a54 to
56c9352
Compare
Use one dispatcher for the existing server menu actions and carry the display name alongside the stable GUID when requesting removal. The GUID remains the deletion identity; confirmation policy stays with MainScreen. This common prerequisite lets named confirmations and custom accessibility actions extend the same contract without conflicting or bypassing confirmation. Cover action routing, duplicate names and blank names with JVM regression tests.
Expose each server as one native selectable radio row while retaining the independent row actions. Name Share, Edit, and Delete with the target profile, use localized full delay units for speech, and replace the custom selected-state string with platform selection semantics.
Replace radio/selectable semantics with a native clickable row and one complete description. Move child-button accessibility entry points into ordered Edit, Delete and share actions, while retaining the visible controls for ordinary touch and keyboard input. Reuse existing menu filtering and dispatch so complex profiles and deletion confirmation keep their original behavior. Prefix the selected row with localized Active server only while the existing UI reports a running service. This intentionally follows UI selection and introduces no daemon identity tracking, IPC, custom announcements or focus manipulation. Add the prefix in all nine locale catalogs. Validation: 63 JVM tests and the Play Store debug app/instrumentation builds pass. Six native accessibility tests pass with TalkBack 17 on the Android 37.1 emulator, covering action ordering, complex types, stable focus/node identity, native activation and visible-button keyboard input. TalkBack speech logs confirm the active prefix without a selected-state phrase or radio role. Physical TalkBack gestures, all-locale audio, full rendering and VPN lifecycle matrices were not run.
Base the localized Selected prefix solely on the row GUID matching selectedGuid, rather than also requiring a running service. Remove the now-unused running-state parameter through the pager and row composables. Replace Active server with concise localized Selected wording in all nine catalogs. Update the JVM and native accessibility tests to require the prefix before connection while preserving row identity and accessibility focus. Validation: 63 JVM tests, Kotlin compilation, debug app assembly, and Android-test assembly passed. Six native tests passed with TalkBack enabled on the Android 37.1 emulator. The real main-screen tree showed “Selected. Flaunt g1...” next to a “Not connected” status, with other rows unprefixed.
Expose Move to top, Move up, Move down, and Move to bottom after the existing server row actions. Resolve moves by GUID at invocation time so a stale accessibility node cannot reorder the wrong server.
Move the server-delay plural and named server actions beside the existing server delay string. This leaves the shared accessibility block identical across focused PRs and avoids XML conflicts without changing any resource value.
Adopt the overlapping terminology corrections from PR 2dust#6169 and carry them into branch-only plural and accessibility resources. This keeps later merge resolution from restoring stale Bakhtiari wording.
Fold proxy-chain list accessibility into the existing server-row accessibility branch. Each member field now owns Remove and applicable reorder actions, while the visual delete button and ordinal stay out of TalkBack swipe navigation. Stable member keys also keep removal and reordering attached to the intended item.
Reposition only the feature-specific resource entries within the existing strings.xml catalog so adjacent translator corrections merge independently across the focused accessibility PRs. Keep every resource name, value, placeholder and plural unchanged; do not split the catalog.
Label the existing native server-row click action Select so TalkBack can describe double-tap selection instead of generic activation. Keep selection dispatch by GUID, the selected-server prefix, ordered secondary actions and direct keyboard operation unchanged. Acknowledge successful server-list and proxy-chain reorder actions through the shared polite live-region host with localized Moved feedback. Retain stable keys and do not request focus or announce rejected moves. Add the strings to all nine existing locale catalogs. Validation: 76 Play Store debug JVM tests, Kotlin compilation and app/test assembly. Focused emulator checks covered repeatable move feedback and proxy-chain actions; six server-row tests additionally verified click labels in both layouts, native/custom action dispatch, selected identity and touch/Enter/Space/D-pad activation. Combined regressions passed. Not run: listening verification in every locale.
Give the host a single explicit owner instead of combining a supplied SnackbarHostState with a composition-local controller. Remove the unused LocalAppSnackbar provider. Keep this behavior-preserving wiring change shared by the notification, subscription, routing and server-row PRs so they remain independently mergeable.
Keep the shared live-region renderer, action-feedback host, and JVM regression tests identical to Parts 2 and 4. Start message expiry after publication and clear pending feedback on pause without changing server-row actions or selection semantics.
Reveal partially clipped selected group tabs using viewport bounds while retaining keyed LazyRow semantics and independent TalkBack swipe traversal. Announce negative server-probe values as a localized connection-test failure in all nine catalogs, preserving compact visual delay text. Share stable-key proxy-chain movement between drag and custom actions. Remove redundant row forwarding and dispatcher coverage, restore private production declarations, and keep instrumentation fixtures as local QA material instead of PR scaffolding. Validation: 83 JVM tests, Play Store debug compilation and assembly, all-locale resource checks, and an external Android 13 / TalkBack 14.2 probe. Virtual-hardware swipes traversed nine tabs in both directions; clipped-tab reveal, list/grid result wording, stable member focus and saved chain order passed. Physical-device speech fluency and the complete theme/input matrix were not run.
Keep AppSnackbarBridge beside the explicit snackbar host and the accessibility action-feedback host after rebasing the shared theme cleanup. This preserves ordinary foreground messages without changing the row custom actions or upstream stable-key proxy-chain deletion. Validated Play Store debug compilation, manifest processing and JVM tests.
Reuse 2dust#6167's reviewed Selected translations in the shared locale catalogs. This removes the remaining English fallback and keeps focused accessibility branches conflict-free when merged in either order.
88e42c0 to
15283df
Compare
Summary
Make main-screen server rows, subscription-group tabs and proxy-chain member actions easier to navigate with TalkBack, while preserving the visible controls and existing profile operations. This is part 9 of the ten focused accessibility PRs.
Server rows
selectedGuid, including while disconnected. This represents the selected configuration, not a separate claim about which profile the daemon is currently running.Rows are actionable list items, not radio buttons or switches. The native activation/focus machinery remains intact; selection context is part of the localized description. The container action menu follows the documented Compose custom-action approach.
Subscription-group tabs
LazyRowand its native scrolling semantics so forward/backward TalkBack swipes can reach tabs outside either viewport edge.Proxy-chain members and move feedback
Localization and review boundaries
All nine declared catalogs include the new/accessibility labels and plural resources: English, Arabic, Bengali, Bakhtiari, Persian, Russian, Vietnamese, Simplified Chinese and Traditional Chinese. The existing XML catalogs remain intact.
No native dependency, profile schema or storage-transaction-policy changes. The storage atomicity/reconciliation work in #6135 is not duplicated here. The PR retains focused JVM regression coverage; temporary Android instrumentation fixtures are kept outside the contribution rather than exposing production declarations solely for tests.
Current validation —
88e42c063:app:testPlaystoreDebugUnitTest: 83 tests, no failures/errors. Includes clipped/oversized tab scroll distances, valid/invalid stable-key member moves, action ordering/filtering and existing dispatch.:app:compilePlaystoreDebugKotlinand:app:assemblePlaystoreDebug -PABI_FILTERS=x86_64: passed. Existing unrelated build warnings remain.-1andLong.MIN_VALUE), zero, and positive singular/plural results exposed the expected descriptions in both list and grid layouts, with no child accessibility targets on server rows.git diff --check: passed.The temporary fixtures were removed, and the emulator's original APK and accessibility settings were restored. Pre-existing subscription data was retained.
Not run
The gesture check used emulator-console virtual touchscreen events. Ordinary injected Android touch events bypass the accessibility input filter and were not counted as proof of TalkBack swipe traversal.