fix(ui): make shared description tooltip triggers keyboard accessible. Fixes #16574 - #16609
fix(ui): make shared description tooltip triggers keyboard accessible. Fixes #16574#16609Abmun wants to merge 5 commits into
Conversation
Icon-only description tooltips (a bare `fa fa-question-circle`/`fa-info-circle` icon wrapped in the shared `Tooltip`) were not keyboard-focusable and had no accessible name, so keyboard and screen-reader users could not discover the explanatory content. Add `TooltipIcon`, a shared trigger that renders a native <button> with an aria-label derived from the tooltip content (or an explicit `label`/`icon` override), and switch every icon-only `Tooltip` usage over to it: - shared/components/parameters-input.tsx - workflows/components/retry-workflow-node-panel.tsx - workflows/components/workflow-details/suspend-inputs.tsx - workflows/components/workflows-summary-container/workflows-summary-container.tsx - workflows/components/workflow-node-info/workflow-node-info.tsx submit-workflow-panel.tsx is fixed transitively via ParametersInput. Tippy (the underlying Tooltip implementation) already opens on focus as well as hover by default, so no changes were needed there once the trigger itself became focusable. Adds tests in tooltip.test.tsx covering focusability, default/overridden accessible names, icon selection, and forwarded tooltip props. Fixes argoproj#16574 Signed-off-by: Abmun <abhimanyu12345@gmail.com>
1ec1575 to
9960442
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughAdds the shared ChangesAccessible tooltip triggers
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/src/shared/components/parameters-input.tsx`:
- Line 52: Move each TooltipIcon outside its surrounding label so the native
tooltip button is a sibling of the label text/control rather than a nested
interactive descendant. Apply this in
ui/src/shared/components/parameters-input.tsx lines 52-52,
ui/src/workflows/components/retry-workflow-node-panel.tsx lines 80-80, and
ui/src/workflows/components/workflow-details/suspend-inputs.tsx lines 34-34,
preserving the existing tooltip content and parameter-label behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b40d01ae-d760-46ad-a770-a530c32a2491
📒 Files selected for processing (7)
ui/src/shared/components/parameters-input.tsxui/src/shared/components/tooltip.test.tsxui/src/shared/components/tooltip.tsxui/src/workflows/components/retry-workflow-node-panel.tsxui/src/workflows/components/workflow-details/suspend-inputs.tsxui/src/workflows/components/workflow-node-info/workflow-node-info.tsxui/src/workflows/components/workflows-summary-container/workflows-summary-container.tsx
CodeRabbit flagged that the Restart Successful label wrapped the new TooltipIcon <button>, making an interactive element an unrelated descendant of the <label>. Pull the tooltip trigger out so it's a sibling of the label, matching the pattern already used in parameters-input.tsx and suspend-inputs.tsx (which were already structured this way and needed no change). Signed-off-by: Abmun <abhimanyu12345@gmail.com>
1e6628b to
14b0fe1
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
@jonas please review |
Fixes #16574
Motivation
Icon-only description tooltips across the UI (a bare
fa fa-question-circle/fa-info-circleicon wrapped in the sharedTooltip) were neither keyboard-focusable nor given an accessible name, so keyboard and assistive-technology users couldn't discover the explanatory content.Modifications
TooltipIconinui/src/shared/components/tooltip.tsx: a shared, accessible trigger that renders a native<button type="button">instead of a bare<i>.tabIndexhacks needed.aria-labeldefaults to the tooltipcontentwhen it's a plain string, falls back to"More information"for rich content, and can be overridden vialabel.fa-question-circle, overridable viaicon; button chrome is reset inline so the visual presentation is unchanged.tooltipPropsforwards extra props (e.g.arrow) to the underlyingTooltip.Tooltip(Tippy-based) already opens onfocusas well asmouseenterby default, so no changes were needed there once the trigger became focusable.Tooltipusages over toTooltipIconin:ui/src/shared/components/parameters-input.tsxui/src/workflows/components/retry-workflow-node-panel.tsxui/src/workflows/components/workflow-details/suspend-inputs.tsxui/src/workflows/components/workflows-summary-container/workflows-summary-container.tsxui/src/workflows/components/workflow-node-info/workflow-node-info.tsx(hidden-secret-value key icon)ui/src/workflows/components/submit-workflow-panel.tsxneeded no direct change - fixed transitively viaParametersInput.Out of scope:
clipboard-text.tsxandtimestamp.tsx'sTimestampSwitchwrap icon-only triggers in an<a>withouthref(same underlying bug), but those carry realonClickbehavior, so fixing them needs keyboard-activation semantics too, not just focusability. Left out to keep this PR scoped to the description-tooltip pattern the issue calls out; happy to follow up separately.Verification
yarn test- all suites pass, including 6 new tests forTooltipIcon(focusability, default/overridden accessible name, icon selection, forwarded tooltip props).yarn lint(eslint +tsc --noEmit) - clean.Documentation
Not required - internal UI accessibility fix, no user-facing API or docs change.
AI
Claude (Anthropic) was used to draft the implementation, tests, and this PR description based on the issue's stated requirements and acceptance criteria. I reviewed the diff, ran the test/lint/typecheck suite locally, and verified the behavior before opening this PR.
Summary by CodeRabbit
Accessibility
Consistency
Tests