fix(a11y): expose unavailable relationship reason to keyboard users - #1500
fix(a11y): expose unavailable relationship reason to keyboard users#1500seonghobae wants to merge 22 commits into
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueNo actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe first-relationship control now remains keyboard-focusable when unavailable. It exposes explanatory tooltip text through ChangesAccessibility update
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change keeps the unavailable relationship button discoverable to keyboard and assistive-technology users while preventing activation. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
PR governance metadata gate is not ready for
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@coderabbitai review Please re-review exact current head |
|
Please review exact current head |
|
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@coderabbitai review Please re-review exact current head |
|
Please review exact current head |
|
|
|
@opencode-agent Please re-review exact current HEAD |
|
@coderabbitai review Please review exact current head |
|
|
Acknowledged. |
|
Supersession note: #1551 was closed after exact comparison showed its only potentially meaningful |
Acknowledged. |
seonghobae
changed the title
|
최신 head
현재 head는 focused regression이 GREEN이므로 Ready로 복원했습니다. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Acknowledged. Thanks for verifying the focused Playwright tests. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a3c70429c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <span | ||
| id={unavailableRelationshipDescriptionId} | ||
| role="tooltip" | ||
| className="pointer-events-none absolute bottom-full left-0 z-10 mb-2 w-max max-w-56 rounded-md bg-foreground px-2 py-1 text-xs font-medium text-background opacity-0 shadow-md transition-opacity group-hover:opacity-100 group-focus-within:opacity-100" |
There was a problem hiding this comment.
When a mouse or screen-magnifier user moves from the button toward the explanation, pointer-events-none removes the tooltip from hit testing and mb-2 leaves an additional gap, so the wrapper loses group-hover and the tooltip disappears before it can be inspected. Make the tooltip hit-testable with a continuous hover region, and extend the Playwright test to move the pointer from the button onto the tooltip.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Inherited in stable successor #1561 at exact head dea8edb64d4ae492aec8815798edad8b4200c1dd. The tooltip is now hit-testable, the margin gap is removed, and Playwright moves the pointer from the button onto the tooltip while asserting it remains visible. The predecessor stays open until successor merge and equivalence verification.
There was a problem hiding this comment.
Acknowledged.
| role="tooltip" | ||
| className="pointer-events-none absolute bottom-full left-0 z-10 mb-2 w-max max-w-56 rounded-md bg-foreground px-2 py-1 text-xs font-medium text-background opacity-0 shadow-md transition-opacity group-hover:opacity-100 group-focus-within:opacity-100" |
There was a problem hiding this comment.
When keyboard focus shows this absolutely positioned tooltip, it can overlap the preceding relationship summary—particularly with wrapped node labels—but its CSS-only group-focus-within state provides no way to dismiss it without moving focus. Add an Escape-key dismissal path so keyboard users can hide the newly introduced overlay while keeping focus on the unavailable action.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Inherited in stable successor #1561 at exact head dea8edb64d4ae492aec8815798edad8b4200c1dd. Escape now dismisses the focus-triggered tooltip without moving button focus; focus/mouse re-entry restores it. Unit and real-browser checks cover the behavior. The predecessor stays open until successor merge and equivalence verification.
There was a problem hiding this comment.
Acknowledged.
|
Concurrent head |
Acknowledged. I will not add any more no-op CI trigger commits as the tree SHA remains valid. |
|
This branch has received repeated empty |
Understood. Acknowledging that this work is now superseded by #1561 and the tree is already merged into |
seonghobae
removed
the
status: needs-review
seonghobae
added
the
status: draft
ChatGPT Codex Connector
1 participant
Current exact-head state
Head:
3a3c70429c0b8a0553211f75c3b3d6282b626960Base:
develop@042b0c70531b229af3acbd0421a2f23098d848b3Fresh compare:
behind_by=0; effective product delta is still the same four files (CHANGELOG.md,NetworkGraph.tsx,NetworkGraph.test.tsx,dashboard-branding.spec.ts).Lifecycle: Draft / source contract repaired / exact-head hosted evidence incomplete.
This PR changes the unavailable
첫 관계 보기control from nativedisabledbehavior to a focusablearia-disabledaction with anaria-describedbyrelationship to a visible tooltip shown on pointer hover or keyboard focus. Activation remains fail-closed when no relationship exists.Keyboard-navigation repair
The first browser regression called
button.focus()directly, which proved focus-state rendering but not sequential keyboard reachability. The stronger test instead focuses the adjacent real그래프 확대control, sends a browserShift+Tab, asserts that첫 관계 보기receives focus through the tab sequence, then verifies the explanatory tooltip becomes visible.Intervening commit
0602047fd53dc10f4b591c0e3e90d198758aa48dwas read rather than treated as a race. Despite itsTrigger CI: Retry LLM Review Timeout (Attempt 6)message, its material delta removed the realShift+Tabsequence and reverted the browser test to direct programmatic focus. That weakened the reviewed accessibility evidence and used a source mutation as a CI retry mechanism. Direct non-force childd0335859454cef6b0f8728f42045cfeefb321a95restored the exact stronger E2E blob while preserving the intervening commit in history.A later intervening commit advanced the branch from
d0335859…to current3a3c7042…. Exact commit comparison reports zero changed files for that commit. It is therefore a no-op history entry, not new product/test evidence, and no check/review result is inferred from it. It is retained because destructive history rewriting is prohibited; future CI recovery must use rerun/retry mechanisms rather than source-mutating or no-op commits.Supersession boundary
#1551 was closed after exact comparison showed no unique valid product/test/fixture/contract delta. Its
aria-label/nativetitleadditions on already text-labelled zoom/fit controls did not add a missing accessible-name contract, while its unavailable-action handling was weaker than this lane. No #1551 check or review evidence transfers.Delivery gate
aria-describedbyplus a visible tooltip.Lifecycle boundary
Keep Draft until the unchanged exact head has terminal-success for every then-live required hosted check, zero valid unresolved findings/threads, and the qualifying independent approval required by protected-branch governance.
No self-approval, protection bypass, force-push, destructive rebase, source-mutating/no-op CI retry commit, or gate weakening is authorized.