Skip to content

refactor: use renderable guards for panel content - #455

Merged
yoyo837 merged 2 commits into
react-component:masterfrom
QDyanbing:refactor-renderable-guards
Sep 3, 2026
Merged

refactor: use renderable guards for panel content#455
yoyo837 merged 2 commits into
react-component:masterfrom
QDyanbing:refactor-renderable-guards

Conversation

@QDyanbing

@QDyanbing QDyanbing commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

说明

  • 使用 isReactRenderable 统一判断面板 extra 和展开图标是否需要渲染
  • 支持数字 0 等有效 React 内容,并统一空内容边界
  • @rc-component/util 的最低版本提升到首次提供该 helper 的 ^1.13.0

验证

  • npm run tsc
  • npm run lint
  • npm test -- tests/index.spec.tsx --runInBand

Summary by CodeRabbit

  • Bug 修复

    • 优化折叠面板额外内容的渲染判断,确保 0、空字符串和 false 不会生成多余节点,同时正确显示可渲染内容。
    • 改进图标渲染判断,提升不同内容类型下的展示一致性。
  • 维护

    • 更新基础组件依赖版本,增强渲染兼容性与稳定性。

@vercel

vercel Bot commented Sep 2, 2026

Copy link
Copy Markdown

@QDyanbing is attempting to deploy a commit to the React Component Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

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: dc78e694-91c9-4f12-951b-9b5751668f9a

📥 Commits

Reviewing files that changed from the base of the PR and between 0f3d661 and eb2ba73.

📒 Files selected for processing (2)
  • src/Panel.tsx
  • tests/index.spec.tsx

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


Walkthrough

本次变更升级 @rc-component/util,并让 CollapsePanel 使用共享工具判断 extra 与图标节点的渲染条件。测试新增 0、空字符串、truefalse 场景。

Changes

可渲染内容判断

Layer / File(s) Summary
共享可渲染性判断
package.json, src/Panel.tsx, tests/index.spec.tsx
@rc-component/util^1.11.1 升级到 ^1.13.0CollapsePanel 使用 isNonNullable 和布尔值排除条件判断 extra,并使用 isReactRenderable 判断图标节点。参数化测试验证不同 extra 值的渲染结果。

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to eb2ba

Panel extras retain their existing empty and boolean handling, while renderable numeric content such as 0 is supported for expand icons. The covered behavior is ready to merge with no concrete current risk identified.

Poem

小兔检查依赖栏,
工具版本向前攀。
extra 值按规则显,
图标节点稳稳看。
测试跑过心不慌。

🚥 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 标题“refactor: use renderable guards for panel content”准确概括了主要变更,包括使用可渲染内容判断逻辑处理面板内容。标题简洁且与代码和测试变更相关。
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.

@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.19%. Comparing base (dcd0f78) to head (eb2ba73).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #455   +/-   ##
=======================================
  Coverage   99.19%   99.19%           
=======================================
  Files           5        5           
  Lines         124      124           
  Branches       45       45           
=======================================
  Hits          123      123           
  Misses          1        1           

☔ 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 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra no longer preserves the existing value semantics.

The previous guard excluded all booleans but treated an empty string as present. isReactRenderable(extra) instead treats true as renderable and an empty string as absent. As a result, extra={true} now creates an empty .rc-collapse-extra wrapper, while extra="" removes a wrapper that previously existed. That can affect header layout, spacing, and the clickable area.

Please preserve the existing contract, for example with isNonNullable(extra) && typeof extra !== 'boolean', and add coverage for true, false, '', and 0. The iconNodeInner migration to isReactRenderable looks correct.

@QDyanbing

Copy link
Copy Markdown
Contributor Author

Addressed in eb2ba73. The extra guard now preserves the previous contract with isNonNullable(extra) && typeof extra !== 'boolean', and the tests cover true, false, '', and 0. The full test suite (98 tests), TypeScript, and diff checks pass locally.

@QDyanbing
QDyanbing requested a review from yoyo837 September 3, 2026 03:44

@yoyo837 yoyo837 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The follow-up preserves the original extra semantics and adds the requested boundary coverage for 0, '', true, and false. LGTM.

@yoyo837
yoyo837 merged commit 305b30a into react-component:master Sep 3, 2026
9 of 10 checks passed
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