-
Notifications
You must be signed in to change notification settings - Fork 336
fix(select): add max-height:50% for select-dropdown #3978
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughAdded a CSS utility limiting select popper height to 50% in two theme packages, and applied that class via Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (1)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/theme-saas/src/select-dropdown/index.less:
- Around line 147-149: The LESS file defines a rule using the variable
@select-prefix-cls (see .@{select-prefix-cls}__popper-maxh-50) but never
declares it; add the missing variable declaration @select-prefix-cls:
~'@{css-prefix}select'; alongside the other prefix variables near the top of
packages/theme-saas/src/select-dropdown/index.less so the class compiles to the
correct selector.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/renderless/src/select/vue.tspackages/theme-saas/src/select-dropdown/index.lesspackages/theme/src/select-dropdown/index.less
⏰ 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). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (3)
packages/renderless/src/select/vue.ts (1)
295-300: LGTM - Default max-height class applied to tooltip.The implementation correctly adds a default
popperClassfor the max-height constraint. The spread ofprops.tooltipConfigafterpopperClassallows users to override this default when needed.Note: If users want to provide a custom
popperClasswhile retaining the max-height behavior, they'll need to include'tiny-select__popper-maxh-50'in their custom class string.packages/theme-saas/src/select-dropdown/index.less (1)
44-44: LGTM - Formatting improvements.The spacing changes in selectors (
> .tiny-svgvs>.tiny-svg) and the semicolon addition improve consistency and readability.Also applies to: 92-93, 102-102
packages/theme/src/select-dropdown/index.less (1)
199-201: No action needed—overflow is handled by the tooltip component wrapper.The class applies
max-height: 50%to the popper element. The tooltip component's content-wrapper (.@{tooltip-prefix-cls}__content-wrapper) already includesoverflow: auto;at the theme level, which handles content overflow properly. The class design is intentional: it constrains the popper's maximum height while delegating overflow handling to the parent tooltip component.The percentage-based
max-heightis appropriate for popper elements, which are typically viewport or ancestor-positioned.
PR
select 的多选提示框,增加 max-height:50% 的限制。 用户可以通过属性传入,覆盖掉这个默认值。
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.