Skip to content

fix: set aria-describedby when overlay is a false but valid node - #544

Merged
yoyo837 merged 2 commits into
react-component:masterfrom
zigzagdev:fix/tooltip-overlay-falsy-aria-describedby
Sep 3, 2026
Merged

fix: set aria-describedby when overlay is a false but valid node#544
yoyo837 merged 2 commits into
react-component:masterfrom
zigzagdev:fix/tooltip-overlay-falsy-aria-describedby

Conversation

@zigzagdev

@zigzagdev zigzagdev commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Motivation

  • getChildren decided whether to attach aria-describedby using overlay && open, so a falsy-but-valid overlay (e.g. overlay={0}) was rendered visibly by Popup but never got an aria-describedby reference, leaving the accessible relationship out of sync with what's on screen.

What I have done

  • Changed the check to overlay != null && open so only "no overlay provided" (null/undefined) skips the attribute, while any other renderable value (including 0 or '') still gets described correctly when it open.

Test plan

  • rc-test tests/index.test.tsx — all 29 existing tests pass (run in a Docker node:20 container since local node_modules wasn't installed)

Summary by CodeRabbit

  • Bug Fixes
    • 修复提示浮层打开时 aria-describedby 标识未能正确添加的问题。
    • 现在只要浮层内容不是 nullundefined,即使为空字符串或其他假值,也能正确关联辅助功能描述。

overlay values like 0 are valid renderable content, but the previous
`overlay && open` check treated them as absent and skipped
aria-describedby even while the tooltip was visible.
@vercel

vercel Bot commented Aug 31, 2026

Copy link
Copy Markdown

@zigzagdev is attempting to deploy a commit to the afc163's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: d254425d-fe65-4867-a900-e6f92baff0c2

📥 Commits

Reviewing files that changed from the base of the PR and between a2629bf and 90a6902.

📒 Files selected for processing (1)
  • tests/index.test.tsx

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


Walkthrough

Tooltip 现在在 overlaynull/undefined 且处于打开状态时设置 aria-describedby。新增测试覆盖 overlay={0}nullundefined

Changes

Tooltip 可访问性属性

Layer / File(s) Summary
调整 aria-describedby 条件
src/Tooltip.tsx, tests/index.test.tsx
overlay != null && open 时,aria-describedby 数组包含 mergedId。测试验证 overlay={0} 会设置该属性,而 nullundefined 不会设置该属性。

Estimated code review effort: 1 (简单) | ~5 分钟

Merge Risk: ⚪ Minimal · up to 90a69

Tooltips now provide an accessibility description for visible falsy values such as 0 while continuing to omit it for null and undefined overlays. The expected behavior is covered by tests, with no current merge-readiness risk identified.

Suggested reviewers: zombiej, nrps9909

Poem

小兔检查提示层,
0 也能留下描述名。
nullundefined 不触发,
打开条件保持清晰,
测试为属性守门。

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题准确概括了主要变更:当 overlay 是有效但为假值的节点时,仍设置 aria-describedby。标题简洁且与变更内容一致。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@yoyo837

yoyo837 commented Sep 1, 2026

Copy link
Copy Markdown
Member

Need some test case to cover this.

@zigzagdev

Copy link
Copy Markdown
Contributor Author

@yoyo837

I add new tests.

@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (a43ecc7) to head (90a6902).

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #544   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines           38        38           
  Branches        15        15           
=========================================
  Hits            38        38           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yoyo837
yoyo837 merged commit 2d2edbb into react-component:master Sep 3, 2026
11 of 12 checks passed
@zigzagdev
zigzagdev deleted the fix/tooltip-overlay-falsy-aria-describedby branch September 3, 2026 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants