list: support accessible names on row containers - #2977
Conversation
huacnlee
left a comment
There was a problem hiding this comment.
Thanks for addressing accessible names on composite list rows. Keeping the name on the same accessibility node as selection and activation is the right goal. However, please revise the public API before merging.
I would prefer the item to own its accessible name, rather than adding ListDelegate::item_accessibility_label. The proposed hook splits the definition of one row between render_item and a second index-based lookup. Applications must retrieve the same data twice and keep the accessible description consistent with the rendered item. It also makes this metadata specific to the containing List rather than reusable with the item itself.
ListItem already implements InteractiveElement, so callers can express the intended API using the existing .aria_label(...) builder alongside the visible content. The important implementation detail is that List currently puts the role, selected state, and activation handling on an outer wrapper. Simply moving the label to the child would not fully solve the problem: the name and row actions still need to belong to the same semantic node.
Please consider these alternatives:
- Prefer making the item the row's semantic node. List supplies the row state and activation behavior, while the item carries its accessible name, role, and actions. Keep any additional wrapper for layout. Check compatibility with custom delegate item types and preserve nested interactive controls.
- If the outer semantic wrapper must remain, let the item supply its semantics through an explicit item interface or wrapper type. List can then apply those semantics to its outer node. This keeps content and naming together without introducing another index-based delegate callback. The current
Selectable + IntoElementbound does not expose that information, so this needs an explicit compatibility design rather than assuming arbitrary rendered children can be inspected.
Please avoid adding label accessors to the generic Selectable trait solely for this purpose; selection state and accessible naming are separate responsibilities.
For the revised design, keep the Story example, define the behavior when no explicit name is provided, and test the resulting accessibility tree. In particular, verify that the row's name, role, selected state, and activation action are on the intended node, that custom items work, and that nested controls remain accessible. The current regression test passes locally, but it checks the wrapper's label and role in isolation; it does not establish those broader ownership and composition guarantees.
This request is about API ownership and composition, not a claim that the current hook fails to set the label. Please resolve that boundary before adding it to the public delegate API.
Allow composite rows to supply a concise accessible name without changing their visual children. Use the reviewed item_accessibility_label API rather than the earlier fork-only spelling. Includes the regression, Story example and docs from upstream PR longbridge#2977, which is still open.
List owns the container that represents each selectable row. A delegate can render text, icons, and other content inside it, but cannot give that outer container an explicit accessible name.
This adds the optional
ListDelegate::item_accessibility_labelhook and applies its result to the existing row container. Applications can describe a composite row as one item, rather than relying on the names of its individual children. The List Story example combines the company name, price, and price change.The default returns
None, so existing delegates need no changes. The patch does not hide children, change the visible layout, or replace selection and activation handling.Accessibility check
Captured from a company row in the macOS Story example:
{ "role": "AXGroup", "title": "Grimes and Pouros LLC, 171.29, -19.90%", "actions": [ "AXPick", "AXPress" ] }The name is on the same node as the row's selection and activation actions. The agent verified that
AXPressselects the named row and that names remain available after changing the query.The AX capture verifies the metadata. I also manually checked row navigation and announcements with VoiceOver on macOS.
How to Test
On macOS:
AXGrouptitle should contain the company, price, and price change.AXPress. The row should become selected.The Story search and row-measurement bugs are separate fixes in #2976 and #2975. This PR does not fix stale search results or overlapping rows.
The regression checks the row container without opening a native window:
cargo test -p gpui-component --lib accessible_names_belong_to_the_row_container --lockedIt checks both an explicit name and
None, while preserving theListItemrole.Automated checks
cargo test -p gpui-base -p gpui-component --lib --locked cargo clippy -p gpui-base -p gpui-component -p gpui-component-story --all-targets --locked -- -D warnings771 base and 419 component tests passed, along with strict Clippy, formatting, and the diff whitespace check.
Windows and Linux runtime were not tested.
Checklist
AI assistance
I used this change in my application, then asked OpenAI's
gpt-6-astrato extract it into a standalone PR. The agent prepared the patch, regression test, and description, and ran the automated checks. I reviewed the extracted patch and manually tested it again on macOS, including VoiceOver.