Add focusable map to preserve semantic elements#1458
Conversation
🦋 Changeset detectedLatest commit: 353fcb5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
2 issues found across 7 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/lib/v3/understudy/a11y/snapshot/domTree.ts">
<violation number="1" location="packages/core/lib/v3/understudy/a11y/snapshot/domTree.ts:24">
P2: Native form elements with `disabled` attribute are not focusable. The function should check for the `disabled` attribute before returning `true` for native focusable tags, otherwise `<button disabled>` and `<input disabled>` will incorrectly be marked as focusable.</violation>
<violation number="2" location="packages/core/lib/v3/understudy/a11y/snapshot/domTree.ts:24">
P2: Anchors are treated as focusable even when they lack an href, so `<a>` elements that are not actually focusable will be marked as focusable in focusableMap.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
Greptile SummaryAdds focusability tracking to preserve semantic interactive elements in accessibility tree snapshots. The implementation introduces a Key Changes:
Impact: Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Capture as capture.ts
participant DomTree as domTree.ts
participant A11yTree as a11yTree.ts
Note over Capture: tryScopedSnapshot() called
Capture->>DomTree: domMapsForSession()
DomTree->>DomTree: getDomTreeWithFallback()
DomTree->>DomTree: Traverse DOM nodes
loop For each DOM node
DomTree->>DomTree: isNodeFocusable(node)
Note over DomTree: Check tag in NATIVE_FOCUSABLE_TAGS<br/>or tabindex >= 0
DomTree->>DomTree: Store in focusableMap[encodedId]
end
DomTree-->>Capture: Return {tagNameMap, xpathMap, scrollableMap, focusableMap}
Capture->>A11yTree: a11yForFrame(session, frameId, opts)
Note over Capture: Pass focusableMap in opts
A11yTree->>A11yTree: Get accessibility tree
A11yTree->>A11yTree: buildHierarchicalTree(nodes, opts)
loop For each a11y node
A11yTree->>A11yTree: Check if isFocusable via opts.focusableMap
alt isFocusable == true
Note over A11yTree: Keep node in tree even if<br/>structural or has no children
A11yTree->>A11yTree: Preserve role or use tagName
else Not focusable
A11yTree->>A11yTree: Apply normal pruning rules
end
end
A11yTree-->>Capture: Return outline with preserved focusable elements
Capture-->>Capture: Merge frames into final snapshot
|
|
Closing until it makes sense to revisit this solution |
why
tabindex=0was not present in a11y treewhat changed
focusableMapto track focusable/interactive nodes, preserving them in a11y treetest plan
Summary by cubic
Add a focusable map to the DOM/a11y snapshot pipeline to mark focusable nodes and keep them in the accessibility tree. This preserves semantic elements (like links, buttons, and tabindex>=0) that were previously pruned.
Written for commit 353fcb5. Summary will update automatically on new commits.