fix(button): announce loading once and fix icon-only name duplication #70 - #763
Conversation
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughSpinner and overlay accessibility semantics now support decorative loading indicators and visual-only tooltips. Button loading and icon-only tooltip tests verify the resulting ARIA attributes, roles, hidden content, and accessible names. ChangesAccessibility semantics
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ButtonContent
participant Tooltip
participant OverlayContent
ButtonContent->>Tooltip: Pass ariaHidden for icon-only tooltip
Tooltip->>OverlayContent: Render tooltip content as aria-hidden
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/tedi/components/loaders/spinner/spinner.spec.tsx (1)
64-71: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert that decorative mode removes
aria-livetoo.The implementation changes both
roleandaria-live, but this test only guards the role and label. Add a negativearia-liveassertion so a future live-region regression cannot reintroduce announcements.Proposed assertion
expect(spinner).toHaveAttribute('aria-hidden', 'true'); + expect(spinner).not.toHaveAttribute('aria-live'); expect(queryByRole('status')).not.toBeInTheDocument();🤖 Prompt for 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. In `@src/tedi/components/loaders/spinner/spinner.spec.tsx` around lines 64 - 71, Update the decorative Spinner test to also assert that the rendered .tedi-spinner does not have an aria-live attribute. Keep the existing aria-hidden, status-role, and label assertions unchanged.
🤖 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 `@src/tedi/components/buttons/button-content/button-content.spec.tsx`:
- Around line 122-127: Update the regression test around the ButtonContent
render to set showTooltip to true, trigger the tooltip’s open behavior, and
assert the resulting tooltip overlay is aria-hidden. Preserve the accessible
name assertion and ensure the test specifically exercises the Tooltip ariaHidden
branch rather than passing with the default tooltip-disabled state.
---
Nitpick comments:
In `@src/tedi/components/loaders/spinner/spinner.spec.tsx`:
- Around line 64-71: Update the decorative Spinner test to also assert that the
rendered .tedi-spinner does not have an aria-live attribute. Keep the existing
aria-hidden, status-role, and label assertions unchanged.
🪄 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: b0151bcf-7ea9-4e30-808a-bdad259694d2
📒 Files selected for processing (8)
src/tedi/components/buttons/button-content/button-content.spec.tsxsrc/tedi/components/buttons/button-content/button-content.tsxsrc/tedi/components/loaders/spinner/spinner.spec.tsxsrc/tedi/components/loaders/spinner/spinner.tsxsrc/tedi/components/overlays/overlay/overlay-content.tsxsrc/tedi/components/overlays/overlay/overlay-trigger.tsxsrc/tedi/components/overlays/overlay/overlay.tsxsrc/tedi/components/overlays/tooltip/tooltip.spec.tsx
Summary by CodeRabbit