Skip to content

Conversation

@Pareder
Copy link
Contributor

@Pareder Pareder commented Dec 8, 2025

Added support for forceRender prop that is already supported by @rc-component/trigger to pre-render the listbox on mount.

Closes #1181
Closes #804
Closes #853

Not sure about adding this prop to documentation, let me know if it is needed.

Summary by CodeRabbit

  • 新增功能
    • BaseSelect 暴露了 forceRender 属性,可控制下拉触发器是否强制渲染,提供更灵活的渲染行为配置。
  • 测试
    • 增加了 forceRender 相关测试,验证在不同取值下下拉列表的渲染与不渲染行为。

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link

vercel bot commented Dec 8, 2025

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

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 8, 2025

Walkthrough

本次变更在类型层面新增并向下传递 forceRender 属性:将其从 @rc-component/triggerTriggerProps 引入到 SelectTriggerProps,并公开到 BaseSelectProps,同时增加测试以验证预渲染行为。

Changes

Cohort / File(s) Summary
Trigger 类型扩展
src/SelectTrigger.tsx
@rc-component/trigger 导入 TriggerProps 类型;更新 SelectTriggerProps 接口,使其继承 Pick<TriggerProps, 'forceRender'>,将 forceRender 纳入 SelectTrigger 的公开类型。
BaseSelect API 暴露与传递
src/BaseSelect/index.tsx
导入 SelectTriggerProps 类型;更新 BaseSelectProps 继承 Pick<SelectTriggerProps, 'forceRender'>;在 props 解构中加入 forceRender 并将其透传给内部 SelectTrigger 使用位置。
测试
tests/Select.test.tsx
新增 “forceRender prop” 测试用例:验证默认不预渲染 listbox 以及传入 forceRender(truthy)时会预渲染 listbox。

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • 需重点核对:
    • SelectTriggerPropsTriggerProps 的类型导入/导出是否正确无歧义;
    • BaseSelectPropsforceRender 的公开声明与实际透传路径是否一致;
    • 新增测试在不同渲染策略下的可靠性与断言正确性。

Possibly related PRs

Suggested reviewers

  • zombieJ

Poem

🐰 我是小兔子,跳进代码堆,
forceRender 带来,列表先到位,
无障碍更安稳,测试也安心,
跳跃一圈笑嘻嘻,胡萝卜请记得备 🎋✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题清晰准确地描述了主要变更:添加对forceRender属性的支持以实现listbox的预渲染。
Linked Issues check ✅ Passed 代码变更完全满足所有关联问题的要求:在BaseSelect和SelectTrigger中暴露forceRender属性,并包含了适当的测试用例。
Out of Scope Changes check ✅ Passed 所有代码变更都与关联问题的目标相关联,包括类型定义更新、属性转发和测试。没有发现超出范围的更改。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 00b8e44 and 6b16fe0.

📒 Files selected for processing (1)
  • tests/Select.test.tsx (1 hunks)
🔇 Additional comments (1)
tests/Select.test.tsx (1)

2847-2859: forceRender 行为用例覆盖合理,命名与实现一致

这两个用例分别验证了「默认不传 forceRender 时不预渲染下拉」以及「显式传入 forceRender 时会预渲染 .rc-select-dropdown」,同时修正了之前“by default”但手动传 forceRender={false} 的语义不一致问题,和本次 PR 的可访问性目标对齐,没有发现需要调整的地方。


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 and usage tips.

@gemini-code-assist
Copy link

Summary of Changes

Hello @Pareder, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request implements a new forceRender prop for the BaseSelect component. This prop enables the pre-rendering of the listbox content immediately upon component mount, rather than waiting for it to be opened. This enhancement provides more control over the rendering behavior of the select component, particularly for scenarios where initial content visibility or SEO might be a concern, by directly utilizing the forceRender capability of the @rc-component/trigger library.

Highlights

  • New forceRender Prop: Introduced a new forceRender prop to the BaseSelect component, allowing the listbox to be pre-rendered on mount.
  • Integration with @rc-component/trigger: The forceRender prop is seamlessly passed down through SelectTrigger to the underlying @rc-component/trigger library, leveraging its existing functionality.
  • Issue Resolution: This change addresses and closes issue Add support for forceRender prop #1181, which likely requested this pre-rendering capability.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds support for the forceRender prop to pre-render the listbox, which is a useful feature for certain use cases like SEO or accessing popup content programmatically before it's visible. The implementation is clean and correctly passes the prop down through BaseSelect to SelectTrigger. The changes look good.

Regarding your question about documentation: since forceRender is now exposed as part of the component's public API, it would be beneficial to add it to the documentation. This will help users understand what the prop does and when to use it, especially considering potential performance implications with large option lists.

@codecov
Copy link

codecov bot commented Dec 9, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.42%. Comparing base (a1e4a12) to head (6b16fe0).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1185   +/-   ##
=======================================
  Coverage   99.42%   99.42%           
=======================================
  Files          31       31           
  Lines        1213     1213           
  Branches      430      430           
=======================================
  Hits         1206     1206           
  Misses          7        7           

☔ View full report in Codecov by Sentry.
📢 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
Copy link
Member

yoyo837 commented Dec 9, 2025

Could you add some test case for it?

@Pareder
Copy link
Contributor Author

Pareder commented Dec 9, 2025

@yoyo837 Done.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
tests/Select.test.tsx (1)

2847-2859: 可以考虑增强测试覆盖率

当前测试套件涵盖了基本的正面和负面场景,符合评审要求。以下是一些可选的增强建议:

  1. 添加一个测试用例来验证真正的默认行为(不传递 forceRender 属性)
  2. 明确测试 forceRender={true} 的情况
  3. 测试 forceRenderopen 属性的组合场景,以确保预渲染的下拉框在打开时仍然正常工作

示例增强:

it('should not render listbox when forceRender prop is not provided', () => {
  const { container } = render(
    <Select options={[{ value: 'a', label: '1' }]} />,
  );
  expect(container.querySelector('.rc-select-dropdown')).not.toBeInTheDocument();
});

it('should keep listbox visible when both forceRender and open are true', () => {
  const { container } = render(
    <Select options={[{ value: 'a', label: '1' }]} forceRender open />,
  );
  const dropdown = container.querySelector('.rc-select-dropdown');
  expect(dropdown).toBeInTheDocument();
  expect(dropdown).toBeVisible();
});
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fced601 and 00b8e44.

📒 Files selected for processing (1)
  • tests/Select.test.tsx (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test / react component workflow
  • GitHub Check: Analyze (javascript)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@zombieJ
Copy link
Member

zombieJ commented Dec 9, 2025

Sorry for delay. Select will create popup element when open, which is work for the real world usage. Is this just for a11y tools checker or a real usage case?

@Pareder
Copy link
Contributor Author

Pareder commented Dec 9, 2025

@zombieJ Mainly for accessibility but also might be used in some scenarios. The thing is Select's input component has aria-owns and aria-controls attributes which values are equal to the listbox id that is not rendered on mount, so they reference unexisting element.
image

@zombieJ
Copy link
Member

zombieJ commented Dec 9, 2025

@zombieJ Mainly for accessibility but also might be used in some scenarios. The thing is Select's input component has aria-owns and aria-controls attributes which values are equal to the listbox id that is not rendered on mount, so they reference unexisting element.

If concern about this, I guess we can do like rc-tooltips logic. When not open, not add the aria-xxx props instead?

@Pareder
Copy link
Contributor Author

Pareder commented Dec 9, 2025

@zombieJ Yes, it will work even better. By the way Tooltip accepts forceRender prop (https://github.com/react-component/tooltip/blob/master/src/Tooltip.tsx#L22).

@Pareder
Copy link
Contributor Author

Pareder commented Dec 9, 2025

@zombieJ Should I close this PR and create a new one?

@zombieJ
Copy link
Member

zombieJ commented Dec 9, 2025

forceRender in Tooltip is not aim to using for a11y. It's used for the pre-render popurse such as async data loading for tip content. It's OK to add for Select but should be more reasonable instead of tools checking.

Should I close this PR and create a new one?

You are welcomed : )

@Pareder
Copy link
Contributor Author

Pareder commented Dec 9, 2025

Closed in favor of #1186.

@Pareder Pareder closed this Dec 9, 2025
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.

Render the listbox whenever the Select component is rendered Add support for forceRender prop

3 participants