-
Notifications
You must be signed in to change notification settings - Fork 38
feat: Support prefix #54
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
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough为 Overflow 组件添加对前缀(prefix)与后缀(suffix)节点的支持,更新组件类型以避开原生 HTML 属性冲突;新增演示示例与文档,并扩展测试与测试工具以覆盖前后缀行为。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Overflow as Overflow Component
participant Renderer as Render Logic
User->>Overflow: 提供 items、maxCount、prefix、suffix
Overflow->>Renderer: 计算可见容量 (考虑 prefix/suffix 占用)
alt 有 prefix
Renderer->>Renderer: 渲染 prefix 节点
else 无 prefix
Renderer-->>Renderer: 跳过 prefix
end
Renderer->>Renderer: 渲染 items(遵循 maxCount / responsive)
alt 有溢出
Renderer->>Renderer: 渲染 rest 指示器
end
alt 有 suffix
Renderer->>Renderer: 渲染 suffix 节点
else 无 suffix
Renderer-->>Renderer: 跳过 suffix
end
Renderer-->>User: 返回包含 prefix + items (+ rest) + suffix 的输出
代码审查工作量评估🎯 4 (复杂) | ⏱️ ~45 分钟 理由:包含对公共 API 的变更(新增 prefix/suffix 并使用 Omit 以规避 HTML 属性名冲突)、示例/文档/测试/测试工具多处改动,需审查类型兼容性、渲染次序(prefix/suffix 对溢出计算的影响)及测试覆盖正确性。 诗
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (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 |
Summary of ChangesHello @zombieJ, 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! 此拉取请求的核心目标是增强 Highlights
Using Gemini Code AssistThe 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
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 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
|
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.
Code Review
本次 PR 主要是为 Overflow
组件添加了 prefix
属性,实现方式与现有的 suffix
类似,整体实现清晰。代码包含了对应的示例、文档和单元测试,覆盖比较完整。我发现了一些可以改进的地方,主要是在示例代码中可以增强类型安全,以及在核心逻辑中修复一个可能导致运行时错误的非空断言。具体的修改建议请见代码评论。
} | ||
|
||
function registerPrefixSize(_: React.Key, width: number | null) { | ||
setPrefixWidth(width!); |
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.
<label style={{ marginRight: 8 }}>Prefix:</label> | ||
<select | ||
value={prefixType} | ||
onChange={(e) => setPrefixType(e.target.value as any)} |
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.
<label style={{ marginRight: 8 }}>Suffix:</label> | ||
<select | ||
value={suffixType} | ||
onChange={(e) => setSuffixType(e.target.value as any)} |
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.
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Overflow.tsx (1)
251-296
: 修复:suffix 固定定位判定未计入 prefix 宽度,导致窄容器下定位错误当前判断仅使用 getItemWidth(0) + suffixWidth,与 prefixWidth 无关;当 prefix 存在且占宽时,suffix 可能被错误固定定位。
请按下述方式修复(并避免 undefined 宽度导致 NaN):
- if (suffix && getItemWidth(0) + suffixWidth > mergedContainerWidth) { + const firstItemWidth = getItemWidth(0) || 0; + if (suffix && (prefixWidth + firstItemWidth + suffixWidth) > mergedContainerWidth) { setSuffixFixedStart(null); }这也与上方总宽计算(totalWidth 以 prefixWidth + suffixWidth 起始)保持一致。修复后建议补充对应单测,见 tests/responsive.spec.tsx 的建议用例。
🧹 Nitpick comments (7)
docs/demo/prefix-suffix.md (1)
1-12
: 文档结构 OK,但可做轻量一致性优化
- 标题建议与页面主标题一致为 “Prefix & Suffix”,便于检索与统一命名。
- 请确认示例路径 ../../examples/prefix-demo.tsx 在构建环境下可被 dumi 正确解析(本地/CI 都跑一遍确保无 404)。
tests/setup.js (1)
35-41
: 提升过滤鲁棒性,避免严格等于 className 导致误判当 Item 附带额外类名时,严格等于可能失效。建议改为正则/包含判断,或基于类名边界匹配:
findItems() { - return this.find('Item').filterWhere( - (item) => - item.props().className !== 'rc-overflow-item-rest' && - item.props().className !== 'rc-overflow-item-prefix' && - item.props().className !== 'rc-overflow-item-suffix', - ); + return this.find('Item').filterWhere((item) => { + const cls = item.props().className || ''; + return !/\brc-overflow-item-(rest|prefix|suffix)\b/.test(cls); + }); },tests/responsive.spec.tsx (1)
229-287
: 用例整体合理;建议强化断言与补充一条覆盖 suffix 定位的用例
- 在“should show overflow with prefix taking space”中,建议补充对 rest 的显示态断言,避免仅以节点存在判定溢出:
- // Should show rest due to limited space - expect(wrapper.findRest()).toHaveLength(1); + // Should show rest due to limited space + expect(wrapper.findRest().props().display).toBeTruthy();
- 建议新增一条覆盖“存在 prefix 时 suffix 固定定位判定”的用例,防止回归(见 Overflow.tsx 相关评论):
it('prefix 存在时,若 prefix + 第一个 item + suffix 宽度超过容器,应取消 suffix 固定定位', () => { const wrapper = mount( <Overflow<ItemType> data={getData(1)} itemKey="key" renderItem={renderItem} maxCount="responsive" prefix="P" suffix="S" />, ); // 设定每个 Item 约 20px 宽,容器较小,触发溢出 wrapper.initSize(40, 20); wrapper.triggerItemResize(0, 20); expect(wrapper.findSuffix().props().style.position).toBeFalsy(); });src/Overflow.tsx (3)
101-105
: 记录 prefix 宽度方案合理;可选进一步防抖以减少闪动当前仅保存 prefixWidth 实时值。可选:参考 restWidth 的“最大值合并”策略,增加 prevPrefixWidth 与 mergedPrefixWidth,降低首次测量导致的闪动。
- const [prefixWidth, setPrefixWidth] = useEffectState<number>( - notifyEffectUpdate, - 0, - ); + const [prefixWidth, setPrefixWidth] = useEffectState<number>(notifyEffectUpdate, 0); + const [prevPrefixWidth, setPrevPrefixWidth] = useEffectState<number>(notifyEffectUpdate, 0); + const mergedPrefixWidth = Math.max(prevPrefixWidth, prefixWidth);并在 registerPrefixSize 中维护 prev:
function registerPrefixSize(_: React.Key, width: number | null) { - setPrefixWidth(width!); + setPrefixWidth(width!); + setPrevPrefixWidth(prefixWidth); }同时在 useLayoutEffect 使用 mergedPrefixWidth 参与 totalWidth。
232-235
: registerPrefixSize 的非空断言可保留;注意 null 情况实现与现有 rest/suffix 的写法一致。若后续支持卸载置空,考虑将 null 归一到 0 以避免 NaN:
- setPrefixWidth(width!); + setPrefixWidth(width == null ? 0 : width);
139-156
: 响应式初始截断可考虑预留 prefix/suffix 宽度以减少首屏过渲染目前 items 初始 slice 仅以 mergedContainerWidth/itemWidth 粗估,未扣除 prefix/suffix 宽度,可能在窄容器 + 大 prefix/suffix 时多渲染若干 Item 后再隐藏。可在已测量后参与估算:
- items = data.slice( - 0, - Math.min(data.length, mergedContainerWidth / itemWidth), - ); + const reserved = (prefixWidth || 0) + (suffixWidth || 0); + const avail = Math.max(0, mergedContainerWidth - reserved); + items = data.slice(0, Math.min(data.length, avail / itemWidth));并将
prefixWidth, suffixWidth
加入该 useMemo 的依赖。这样能减少不必要的首次渲染与测量。examples/prefix-demo.tsx (1)
11-51
: 可考虑提取共享辅助函数。
createData
、renderItem
和renderRest
函数与examples/basic.tsx
中的实现存在相似性。虽然示例文件中保持独立实现有助于清晰展示,但如果未来有更多示例文件,可以考虑将这些通用逻辑提取到共享工具模块中,以减少维护成本。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/demo/prefix-suffix.md
(1 hunks)examples/basic.tsx
(3 hunks)examples/prefix-demo.tsx
(1 hunks)src/Overflow.tsx
(8 hunks)tests/prefix.spec.tsx
(1 hunks)tests/responsive.spec.tsx
(1 hunks)tests/setup.js
(1 hunks)tests/wrapper.ts
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/prefix.spec.tsx (1)
tests/wrapper.ts (1)
mount
(16-18)
src/Overflow.tsx (1)
src/hooks/useEffectState.tsx (1)
useEffectState
(43-58)
tests/responsive.spec.tsx (1)
tests/wrapper.ts (1)
mount
(16-18)
🔇 Additional comments (17)
tests/wrapper.ts (1)
12-13
: 类型扩展与原型实现一致,LGTM新增的 findPrefix 与 tests/setup.js 的原型扩展匹配,可在测试中链式使用。
tests/setup.js (1)
46-48
: findPrefix 实现与命名一致,LGTM选择器 Item.rc-overflow-item-prefix 与源码类名保持一致,测试可读性好。
src/Overflow.tsx (2)
19-19
: 类型设计正确避免 DOM 属性冲突,LGTM通过 Omit<..., 'prefix'> 并以 ReactNode 重新定义,避免与原生 HTML 全局属性冲突,类型更安全。
408-421
: Prefix 节点插入与样式类命名一致,LGTM
- order=-1 保证前置,类名 rc-overflow-item-prefix 与测试用选择器一致。
- 与 suffix 同步了 responsive 与 responsiveDisabled,行为一致。
examples/basic.tsx (3)
51-52
: 状态变量添加正确。新增的
showPrefix
和showSuffix
状态变量命名清晰,初始化合理。
64-83
: 切换按钮实现良好。前缀和后缀的切换按钮与现有的响应式按钮保持一致的结构和交互模式,用户体验清晰。
123-146
: 前缀和后缀的条件渲染实现正确。使用三元运算符进行条件渲染,逻辑清晰。样式通过颜色主题(前缀绿色,后缀蓝色)有效区分了两个元素。
tests/prefix.spec.tsx (6)
1-29
: 测试设置规范。测试文件的结构和设置遵循项目标准,包括正确的 fake timers 初始化和清理。
31-55
: 基础前缀测试覆盖全面。测试涵盖了提供前缀和不提供前缀两种场景,正反向用例完整,断言清晰准确。
57-81
: 响应式模式测试设计合理。通过
initSize
模拟容器尺寸变化触发溢出,使用灵活的断言(toBeGreaterThan
/toBeLessThan
)来验证动态行为,测试策略恰当。
83-101
: 固定 maxCount 测试准确。针对固定
maxCount
场景使用精确断言验证项目数量,测试逻辑正确。
103-124
: 前缀和后缀组合测试完善。测试验证了前缀和后缀同时使用的场景,断言覆盖了两个元素的存在性、内容以及项目数量,集成测试完整。
126-146
: 复杂前缀内容测试周到。测试验证了 React 节点作为前缀的高级用法,通过查找 className 和验证文本内容确保复杂结构正确渲染。
examples/prefix-demo.tsx (4)
53-58
: 状态管理和性能优化良好。使用
useMemo
缓存数据生成结果,避免不必要的重新计算。类型定义清晰,状态管理设计合理。
60-111
: 前缀渲染逻辑清晰且富有展示性。使用
switch
语句处理不同前缀类型,每种类型都有独特的视觉设计。从简单文本到复杂布局的多样化展示有效演示了组件的灵活性。
113-149
: 后缀渲染实现完整。后缀类型涵盖文本和交互式按钮,展示了不同的使用场景。按钮的
onClick
事件使用alert
演示交互功能,对于示例代码来说是合适的。
151-251
: 演示界面设计完善。控制面板提供了交互式配置选项,两个演示区域分别展示响应式和固定
maxCount
模式,清晰地演示了组件在不同配置下的行为。布局和样式设计直观易用。
Semantic structure 上看,Select 组件的 placeholder 必须要在 Overflow 中,否则会和 Select 的 prefix(非该 feature 新增的 prefix) 冲突。因而将 placeholder 挪至 Overflow 中实现语义化结构。
Summary by CodeRabbit
新功能
文档
测试