-
-
Notifications
You must be signed in to change notification settings - Fork 318
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
feat: support prefix prop #884
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改在多个组件和接口中引入了一个新的可选属性 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Picker
participant RangeSelector
participant SingleSelector
participant SharedPickerProps
User->>Picker: 设置prefix属性
Picker->>SharedPickerProps: 传递prefix
Picker->>RangeSelector: 渲染RangeSelector
RangeSelector->>SharedPickerProps: 使用prefix
Picker->>SingleSelector: 渲染SingleSelector
SingleSelector->>SharedPickerProps: 使用prefix
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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
🧹 Outside diff range and nitpick comments (5)
docs/examples/customize.tsx (1)
124-124
: 新增的prefix属性增强了Picker组件的自定义性这个更改很好地实现了支持prefix属性的目标。它为Picker组件增加了更多的定制选项,使界面更加灵活。
考虑使用更有意义的示例值来代替"Foobar",以便更好地展示prefix属性的实际用途。例如:
-prefix="Foobar" +prefix="日期:"这样可以更直观地展示prefix属性的效果,有助于其他开发者理解其用途。
src/PickerInput/Selector/RangeSelector.tsx (3)
55-55
: 新属性prefix
添加正确
prefix
属性的添加增强了组件的灵活性,允许渲染前缀元素。这是一个很好的功能增强。建议更新组件的文档,以反映这个新的
prefix
属性及其用法。
242-242
: 前缀渲染逻辑实现正确前缀元素的条件渲染逻辑实现正确,使用了适当的类名。这与新添加的
prefix
属性保持一致。为了提高代码的可读性,可以考虑将前缀渲染逻辑提取为一个单独的函数或组件。例如:
const renderPrefix = () => prefix && <div className={`${prefixCls}-prefix`}>{prefix}</div>; // 然后在 return 语句中使用 {renderPrefix()}这样可以使主要的渲染逻辑更加清晰。
Line range hint
1-270
: 总体评审:更改符合预期且实现正确
- 新增的
prefix
属性及其实现与 PR 的目标一致。- 更改没有引入明显的 bug 或性能问题。
- 组件的整体结构和功能保持不变,仅增加了前缀渲染的灵活性。
考虑在未来的迭代中:
- 为新添加的
prefix
属性添加类型检查,例如PropTypes
或 TypeScript 类型定义。- 如果
prefix
可能是复杂的 React 元素,考虑将其类型从string
扩展为React.ReactNode
。- 为了保持一致性,可能需要在相关的组件(如
SingleSelector
)中也添加类似的prefix
支持。src/interface.tsx (1)
357-357
: 考虑属性命名的一致性建议将属性
prefix
命名为prefixIcon
,以与现有的suffixIcon
属性保持一致,增强代码的可读性和一致性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- docs/examples/customize.tsx (1 hunks)
- src/PickerInput/Selector/RangeSelector.tsx (2 hunks)
- src/PickerInput/Selector/SingleSelector/index.tsx (2 hunks)
- src/interface.tsx (1 hunks)
🧰 Additional context used
🔇 Additional comments (4)
src/PickerInput/Selector/SingleSelector/index.tsx (3)
46-46
: 新增的prefix
属性符合预期。
prefix
属性的添加与接口更改一致,并且在解构列表中的位置适当。这个改动增强了组件的灵活性,允许渲染前缀元素。
228-228
:prefix
渲染实现正确,建议添加测试。
prefix
的条件渲染实现正确,使用单独的 div 和特定的类名有利于样式和布局控制。前缀元素放置在selectorNode
之前的位置也符合逻辑。然而,为确保这个新功能的稳定性和正确性,建议为
prefix
属性添加相应的单元测试。您是否需要我协助生成用于测试
prefix
功能的单元测试代码?
Line range hint
1-243
: 总体评价:prefix
属性的添加增强了组件的灵活性。此次更改成功地在
SingleSelector
组件中引入了prefix
属性,实现方式简洁有效。这个新特性增强了组件的可定制性,同时保持了与现有代码的一致性和兼容性。建议:
- 确保更新相关文档,说明新增的
prefix
属性及其用法。- 考虑添加示例代码,展示如何使用
prefix
属性。- 如前所述,为新增的
prefix
功能添加单元测试。总的来说,这是一个有价值的改进,能够为用户提供更大的灵活性。
src/interface.tsx (1)
Line range hint
359-361
: 属性定义正确,符合预期新增的属性
allowClear
和弃用的属性clearIcon
处理得当,接口设计合理。
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
🧹 Outside diff range and nitpick comments (2)
tests/picker.spec.tsx (1)
585-593
: 测试用例结构良好,但可以进一步改进这个新增的测试用例很好地验证了
prefix
属性的功能。测试结构清晰,使用了适当的断言方法。为了使测试更加健壮,建议考虑以下改进:
- 检查
prefix
元素的具体位置,确保它正确地渲染在输入框之前。- 验证
prefix
的内容是否正确渲染。您可以考虑使用以下代码来增强测试:
it('prefix', () => { render( <DayPicker prefix={<span className="prefix">Test Prefix</span>} allowClear />, ); - expect(document.querySelector('.prefix')).toBeInTheDocument(); + const prefixElement = document.querySelector('.prefix'); + expect(prefixElement).toBeInTheDocument(); + expect(prefixElement.textContent).toBe('Test Prefix'); + expect(prefixElement.nextElementSibling).toHaveClass('rc-picker-input'); });这样可以更全面地测试
prefix
属性的功能。tests/range.spec.tsx (1)
707-715
: 新增的prefix属性测试看起来不错!这个测试用例很好地验证了prefix属性的功能。不过,我有一个小建议可以使测试更加健壮:
考虑添加一个额外的断言,以确保prefix元素被正确地放置在输入框之前。例如:
expect(document.querySelector('.prefix').nextElementSibling).toBe(document.querySelector('input'));这样可以进一步确保prefix元素的位置正确。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- docs/examples/customize.tsx (1 hunks)
- src/PickerInput/Selector/RangeSelector.tsx (2 hunks)
- src/PickerInput/Selector/SingleSelector/index.tsx (2 hunks)
- src/interface.tsx (1 hunks)
- tests/picker.spec.tsx (1 hunks)
- tests/range.spec.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/examples/customize.tsx
- src/PickerInput/Selector/RangeSelector.tsx
- src/PickerInput/Selector/SingleSelector/index.tsx
- src/interface.tsx
🧰 Additional context used
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: 2
🧹 Outside diff range and nitpick comments (4)
src/PickerInput/Selector/SingleSelector/index.tsx (1)
228-228
: LGTM:prefix 渲染实现
prefix
的条件渲染实现正确,类名命名符合组件的命名约定,并且前缀元素的位置恰当。建议:考虑为前缀容器添加
aria-hidden="true"
属性,以提高屏幕阅读器的可访问性,除非prefix
包含重要的语义信息。如果您同意,可以应用以下更改:
- {prefix && <div className={`${prefixCls}-prefix`}>{prefix}</div>} + {prefix && <div className={`${prefixCls}-prefix`} aria-hidden="true">{prefix}</div>}src/PickerInput/Selector/RangeSelector.tsx (1)
55-55
: 新增的prefix
属性增强了组件的灵活性
prefix
属性的添加使得组件能够支持前缀元素的渲染,这是一个很好的功能增强。建议更新组件的文档,以反映这个新增的属性及其用法。
src/interface.tsx (1)
Line range hint
89-98
: 注意:已弃用的属性和建议的替代方案以下属性已被标记为弃用:
defaultValue
disabledHours
disabledMinutes
disabledSeconds
建议使用以下替代方案:
- 使用
defaultOpenValue
替代defaultValue
- 使用
disabledTime
替代disabledHours
、disabledMinutes
和disabledSeconds
请更新您的代码以使用新的推荐属性,以确保与未来版本的兼容性。
tests/picker.spec.tsx (1)
585-593
: 新增的 prefix 测试用例看起来不错这个测试用例很好地验证了
prefix
属性的功能。不过,我们可以稍微改进一下:
- 考虑添加一个更具体的类名或数据属性来选择 prefix 元素,这样可以使测试更加健壮。
- 可以进一步测试 prefix 内容是否正确渲染。
建议修改如下:
it('prefix', () => { render( <DayPicker - prefix={<span className="prefix" />} + prefix={<span className="custom-prefix" data-testid="picker-prefix">测试前缀</span>} allowClear />, ); - expect(document.querySelector('.prefix')).toBeInTheDocument(); + const prefixElement = document.querySelector('[data-testid="picker-prefix"]'); + expect(prefixElement).toBeInTheDocument(); + expect(prefixElement).toHaveTextContent('测试前缀'); });这样的修改可以使测试更加详细和可靠。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- docs/examples/customize.tsx (1 hunks)
- src/PickerInput/Selector/RangeSelector.tsx (2 hunks)
- src/PickerInput/Selector/SingleSelector/index.tsx (2 hunks)
- src/interface.tsx (3 hunks)
- tests/picker.spec.tsx (1 hunks)
- tests/range.spec.tsx (1 hunks)
🧰 Additional context used
🔇 Additional comments (8)
src/PickerInput/Selector/SingleSelector/index.tsx (1)
46-46
: LGTM:新增 prefix 属性
prefix
属性的添加符合接口的变更,并且在解构列表中的位置恰当。src/PickerInput/Selector/RangeSelector.tsx (1)
55-57
: 新增的属性增强了组件的可定制性
prefix
、clearIcon
和suffixIcon
属性的添加增强了组件的可定制性,这是一个很好的改进。请确保这些新增的属性在组件的 prop 类型定义中有正确的类型声明。可以运行以下脚本来验证:
#!/bin/bash # 描述:验证新增属性的类型声明 # 测试:搜索 RangeSelectorProps 接口中的新属性定义 rg -A 5 'interface RangeSelectorProps' src/PickerInput/Selector/RangeSelector.tsxsrc/interface.tsx (3)
358-358
: 新增prefix
属性
SharedPickerProps
接口中新增了prefix
属性,类型为React.ReactNode
。这个新属性允许在选择器组件中添加自定义的前缀元素,增强了组件的灵活性和可定制性。使用示例:
<Picker prefix={<Icon type="calendar" />} />这个改动与PR中其他文件的变更保持一致,增强了组件的功能。
305-305
: 更新SharedHTMLAttrs
类型在
SharedHTMLAttrs
类型中,prefix
属性已被添加到省略列表中。这个更改是为了避免与新添加的prefix
属性发生冲突,确保了类型定义的一致性和准确性。这个改动是必要的,因为它防止了 HTML 原生的
prefix
属性与我们自定义的prefix
属性之间的潜在冲突。
473-473
: 新增prefix
属性到SelectorProps
SelectorProps
接口中新增了prefix
属性,类型为React.ReactNode
。这个改动与SharedPickerProps
的变更保持一致,允许在选择器组件中添加自定义的前缀元素。使用示例:
<Selector prefix={<Icon type="search" />} />这个新属性增强了选择器组件的可定制性,使其与整体设计保持一致。
tests/picker.spec.tsx (1)
Line range hint
1-1037
: 整体文件评审总结这个测试文件整体上看起来很全面,涵盖了 DayPicker 组件的多个方面。新增的
prefix
测试用例是一个很好的补充,进一步增强了组件的测试覆盖率。建议:
- 考虑为其他新功能或属性添加类似的测试用例,以保持测试套件的全面性。
- 可以考虑添加一些边缘情况的测试,例如
prefix
为 null 或 undefined 时的行为。总的来说,这个文件的变更是积极的,提高了组件的质量和可靠性。
tests/range.spec.tsx (2)
707-716
: 新增的测试用例很好地覆盖了 prefix 属性的功能。这个测试用例验证了 DayRangePicker 组件的 prefix 属性是否正确渲染。这是一个很好的补充,可以确保该功能按预期工作。
建议:考虑添加一个额外的断言,以验证 prefix 元素的具体位置或结构,以确保它不仅存在于文档中,而且在正确的位置渲染。例如:
expect(document.querySelector('.rc-picker .prefix')).toBeInTheDocument();
Line range hint
1-1157
: 测试文件整体覆盖面广泛,新增测试用例进一步增强了组件功能的验证。这个测试文件包含了大量针对 DayRangePicker 组件的测试用例,涵盖了各种功能和边界情况。新增的 prefix 属性测试进一步完善了测试套件。整体而言,这个文件提供了全面的测试覆盖,有助于确保组件的稳定性和可靠性。
建议:
- 考虑添加一些针对边缘情况的测试,例如不同时区、特殊日期格式等。
- 可以考虑使用测试数据生成器来创建更多的测试场景,以提高测试的多样性。
@@ -121,6 +121,7 @@ class Customize extends React.Component<{}, DateRangeState> { | |||
// format="YYYY/MM/DD" | |||
format={['YYYY-MM-DD', 'YYYY/MM/DD']} | |||
allowClear | |||
prefix="Foobar" |
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.
💡 Codebase verification
Picker 组件未正确实现 prefix
属性
在执行验证脚本后,未找到 prefix
属性在 Picker 组件中的实现。这意味着 prefix
属性可能尚未被正确添加或处理。
建议:
- 检查
Picker
组件的实现文件,确保prefix
属性已正确添加并处理。 - 确认
PickerProps
接口中已包含prefix
属性的声明。
🔗 Analysis chain
新增 prefix
属性的实现
新增的 prefix
属性为 Picker 组件增加了前缀功能,这可以提高组件的灵活性和可定制性。
建议:
- 确保
prefix
属性在 Picker 组件的类型定义中已正确声明。 - 在组件文档中添加关于
prefix
属性的使用说明和示例。 - 考虑添加一个测试用例,以验证
prefix
属性的正确渲染和功能。
请运行以下脚本来验证 prefix
属性的实现:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:验证 Picker 组件是否正确实现了 prefix 属性
# 测试:搜索 Picker 组件的实现,预期:找到 prefix 属性的处理逻辑
ast-grep --pattern $'const Picker = ($_) => {
$$$
prefix={$_}
$$$
}'
# 测试:搜索 Picker 组件的类型定义,预期:找到 prefix 属性的类型声明
rg -A 5 'interface PickerProps'
Length of output: 603
@@ -238,6 +239,7 @@ function RangeSelector<DateType extends object = any>( | |||
onMouseDown?.(e); | |||
}} | |||
> | |||
{prefix && <div className={`${prefixCls}-prefix`}>{prefix}</div>} |
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.
💡 Codebase verification
**缺少 `prefix` 属性的测试用例**
未发现针对 `prefix` 属性的测试用例。建议为此功能添加相应的测试,以确保其正确性并防止未来的回归。
🔗 Analysis chain
prefix
元素的渲染实现正确
prefix
元素的条件渲染实现正确,并且使用了 prefixCls
来保持类名的一致性,这很好。
建议为这个新功能添加测试用例,以确保其正确性和防止未来的回归。可以运行以下脚本来检查是否已经添加了相关测试:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查是否已添加 prefix 属性的测试用例
# 测试:在测试文件中搜索 prefix 相关的测试
rg -A 5 'test.*prefix' tests
Length of output: 255
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #884 +/- ##
=======================================
Coverage 95.43% 95.43%
=======================================
Files 64 64
Lines 2717 2719 +2
Branches 732 734 +2
=======================================
+ Hits 2593 2595 +2
Misses 121 121
Partials 3 3 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
新功能
prefix
,增强了灵活性。SharedPickerProps
接口中新增了suffixIcon
、allowClear
和needConfirm
属性。弃用