feat: support prefix #1060
feat: support prefix #1060
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
README.md
Outdated
| @@ -121,6 +121,7 @@ export default () => ( | |||
| | showAction | actions trigger the dropdown to show | String[]? | - | | |||
| | autoFocus | focus select after mount | boolean | - | | |||
| | autoClearSearchValue | auto clear search input value when multiple select is selected/deselected | boolean | true | | |||
| | prefix | specify the select prefix icon or text | ReactNode | - | | | |||
There was a problem hiding this comment.
我在实践中发现 prefix 和 suffixIcon 并不是对应的，prefix 是 relative 布局，不会和搜索框重叠，而 suffixIcon 是 absolute 布局，可能会和搜索框重叠（虽然可以用 CSS 定宽 padding/margin 解决，但是不能动态计算，导致 suffixIcon 不能是任意宽度的内容）。
There was a problem hiding this comment.
当年没有 flex 布局用的 absolute。前置后和后置是等价的。从 API 看一个叫 prefix 却没有对应的 suffix，一个叫 suffixIcon 却没有对应的 prefixIcon，相当诡异。
There was a problem hiding this comment.
prefix 和 suffix 对应，可以把 suffixIcon 废弃掉，重命名为 suffix，并重构为 flex 布局。
src/Selector/index.tsx
Outdated
| @@ -72,6 +72,7 @@ export interface SelectorProps { | |||
| disabled?: boolean; | |||
| placeholder?: React.ReactNode; | |||
| removeIcon?: RenderNode; | |||
| prefix?: RenderNode; | |||
There was a problem hiding this comment.
为什么不和 suffixIcon 一样，放 BaseSelect 里渲染？
There was a problem hiding this comment.
尝试了一下，如果放在 BaseSelect 里 flex 布局，preffix 会跑到 border 外面，视觉效果是不符合预期的
Walkthrough此次更改引入了一个新的可选属性 Changes
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
docs/examples/custom-icon.tsx (2)
106-106: 考虑让前缀值可配置。将前缀值硬编码为 "Foobar" 可能不适合所有用例。建议通过 props 让前缀值可配置，以提高组件的灵活性和可重用性。
例如，可以应用以下更改：
- prefix="Foobar" + prefix={this.props.prefix}然后在使用
CustomIconComponent时传递prefix属性：<CustomIconComponent prefix="Foobar" />
197-197: 考虑让前缀值可配置。将前缀值硬编码为 "Foobar" 可能不适合所有用例。建议通过 props 让前缀值可配置，以提高组件的灵活性和可重用性。
例如，可以应用以下更改：
- prefix="Foobar" + prefix={this.props.prefix}然后在使用
Test组件时传递prefix属性：<Test prefix="Foobar" />
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
tests/__snapshots__/Combobox.test.tsx.snapis excluded by!**/*.snaptests/__snapshots__/ssr.test.tsx.snapis excluded by!**/*.snap
Files selected for processing (6)
- README.md (1 hunks)
- assets/index.less (3 hunks)
- docs/examples/custom-icon.tsx (2 hunks)
- src/BaseSelect/index.tsx (3 hunks)
- src/Selector/SingleSelector.tsx (2 hunks)
- src/Selector/index.tsx (3 hunks)
Additional context used
Markdownlint
README.md
124-124: Expected: 4; Actual: 5; Too many cells, extra data will be missing
Table column count(MD056, table-column-count)
Additional comments not posted (8)
src/Selector/SingleSelector.tsx (1)
107-108: 代码更改没有问题！将占位符节点移动到值显示部分之前的新位置可以改善用户界面，尤其是在输入为空时，为用户输入提供即时上下文。这种调整确保占位符在组件的输出中始终如一地呈现。
assets/index.less (2)
75-75: 代码更改没有问题！添加
position: relative;可以更好地控制子元素的布局。
Line range hint
118-118: 请验证此更改对组件的影响。移除
flex-wrap: wrap;可能会导致以下影响：
- 多选项或溢出项的显示方式发生变化，可能变为线性排列。
- 组件在处理多个选择或溢出项时的响应性可能受到影响。
请运行以下脚本来验证更改的影响：
src/Selector/index.tsx (2)
75-75: LGTM!添加
prefix属性到SelectorProps接口是一个有效的改动，它与支持 Select 组件中前缀的 PR 目标一致。使用RenderNode类型为用户提供了传递自定义渲染节点作为前缀的灵活性。
274-274: 之前的评论仍然有效，可以跳过。zombieJ 之前建议将前缀渲染在
BaseSelect中，类似于suffixIcon，但是 guoyunhe 的回应表明这会导致意外的视觉效果。因此，在Selector组件的 JSX 结构中有条件地渲染prefix内容是一个有效的实现，它与 PR 的目标一致。该代码处理了prefix属性的函数和直接节点类型。README.md (1)
124-124: 之前的评论仍然有效，讨论了属性的命名和行为。我同意prefix属性的命名和类型是合适的，不需要生成类似的评论。Tools
Markdownlint
124-124: Expected: 4; Actual: 5; Too many cells, extra data will be missing
Table column count(MD056, table-column-count)
src/BaseSelect/index.tsx (2)
176-176: 代码更改已批准。新增的
prefix属性已正确地从props中解构出来。
800-800: 代码更改已批准。
prefix属性已正确地传递给了Selector子组件。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1060 +/- ##
=======================================
Coverage 98.23% 98.23%
=======================================
Files 39 39
Lines 1473 1474 +1
Branches 438 416 -22
=======================================
+ Hits 1447 1448 +1
Misses 26 26 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
tests/__snapshots__/Combobox.test.tsx.snapis excluded by!**/*.snaptests/__snapshots__/ssr.test.tsx.snapis excluded by!**/*.snap
Files selected for processing (7)
- README.md (1 hunks)
- assets/index.less (3 hunks)
- docs/examples/custom-icon.tsx (2 hunks)
- src/BaseSelect/index.tsx (3 hunks)
- src/Selector/SingleSelector.tsx (2 hunks)
- src/Selector/index.tsx (3 hunks)
- tests/Multiple.test.tsx (2 hunks)
Files skipped from review due to trivial changes (1)
- src/Selector/SingleSelector.tsx
Files skipped from review as they are similar to previous changes (4)
- assets/index.less
- docs/examples/custom-icon.tsx
- src/BaseSelect/index.tsx
- src/Selector/index.tsx
Additional context used
Markdownlint
README.md
124-124: Expected: 4; Actual: 5; Too many cells, extra data will be missing
Table column count(MD056, table-column-count)
Additional comments not posted (4)
README.md (1)
124-124: 新增prefix属性此更改允许用户在选择组件中指定图标或文本作为前缀，增强了组件的功能性和可定制性。
Tools
Markdownlint
124-124: Expected: 4; Actual: 5; Too many cells, extra data will be missing
Table column count(MD056, table-column-count)
tests/Multiple.test.tsx (3)
22-22: 导入语句更新审查更新了导入语句以包括
screen，这对于新的测试用例是必要的。
377-384: 静态前缀显示测试测试用例检查静态字符串前缀是否正确渲染。使用
screen.findByText来断言前缀的存在，结构清晰，逻辑正确。
386-393: 动态前缀显示测试测试用例评估当作为
prefix属性提供返回字符串的函数时的行为。使用screen.findByText来断言前缀的存在，正确处理动态前缀，使用适当的断言来验证功能。
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
tests/__snapshots__/Combobox.test.tsx.snapis excluded by!**/*.snaptests/__snapshots__/ssr.test.tsx.snapis excluded by!**/*.snap
Files selected for processing (7)
- README.md (1 hunks)
- assets/index.less (3 hunks)
- docs/examples/custom-icon.tsx (2 hunks)
- src/BaseSelect/index.tsx (3 hunks)
- src/Selector/SingleSelector.tsx (2 hunks)
- src/Selector/index.tsx (3 hunks)
- tests/Multiple.test.tsx (2 hunks)
Files skipped from review due to trivial changes (2)
- docs/examples/custom-icon.tsx
- src/Selector/SingleSelector.tsx
Files skipped from review as they are similar to previous changes (5)
- README.md
- assets/index.less
- src/BaseSelect/index.tsx
- src/Selector/index.tsx
- tests/Multiple.test.tsx
src/BaseSelect/index.tsx
Outdated
| @@ -173,6 +173,7 @@ export interface BaseSelectProps extends BaseSelectPrivateProps, React.AriaAttri | |||
|
|
|||
| // >>> Icons | |||
| allowClear?: boolean | { clearIcon?: RenderNode }; | |||
| prefix?: RenderNode; | |||
There was a problem hiding this comment.
不要 renderProps，用户 HOC 自行转成 func 就行了。rc-select 不用支持。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
src/Selector/SingleSelector.tsx (1)
107-108: 代码更改看起来不错，建议小幅改进以提高可读性。这个更改正确地将
placeholderNode的渲染移到了更合适的位置，符合支持 Select 组件前缀的 PR 目标。放置在 selection-search span 内是恰当的，因为这可能是前缀可见的地方。为了提高代码的可读性和可维护性，我建议添加一个简短的注释来解释为什么
placeholderNode被放在这个位置。这将有助于其他开发者理解这个更改的意图。建议在第 107 行之前添加以下注释：
+ {/* 渲染占位符或前缀（如果有的话） */} {/* Display placeholder */} {placeholderNode}docs/examples/custom-icon.tsx (3)
Line range hint
106-112: 新增的prefix属性符合PR目标，图标渲染得到改进这些更改很好地实现了PR的目标，同时改进了图标的渲染方式。
建议：
- 考虑使用更有意义的prefix值，而不是"Foobar"，以便更好地展示此功能的实际用途。
- 可以考虑将prefix也设置为一个动态值或组件，以增加灵活性。
Line range hint
197-201: Test组件的更改与CustomIconComponent保持一致这些更改与CustomIconComponent中的修改保持一致，很好地实现了PR的目标。
建议：
- 考虑将prefix值设置为可配置的prop，而不是硬编码的"Foobar"。这样可以增加组件的灵活性，允许用户自定义prefix。
- 可以添加一个示例，展示如何使用不同类型的prefix（如图标或文本）。
Line range hint
1-224: 总体评价：更改符合PR目标，实现了Select组件的prefix支持这些更改很好地实现了PR的目标，为Select组件添加了prefix支持。主要亮点包括：
- 在CustomIconComponent和Test组件中一致地实现了prefix属性。
- 改进了图标渲染，使用getSvg函数确保了一致性。
建议进一步改进：
- 将prefix值设置为可配置的prop，增加灵活性。
- 添加更多示例，展示不同类型的prefix（如图标、文本）的使用。
- 考虑添加对prefix样式的自定义支持。
这些改进将使新功能更加强大和灵活，为用户提供更多定制选项。
tests/Multiple.test.tsx (1)
377-384: 新的测试用例结构良好，但需要小改动这个测试用例很好地验证了静态前缀的功能。然而，
screen.findByText()是一个异步方法，建议将测试改为异步并使用await。建议如下修改：
- it('show static prefix', () => { + it('show static prefix', async () => { render(<Select mode="multiple" value={['']} prefix="Foobar"> <Option value={1}>1</Option> <Option value={2}>2</Option> </Select>); - expect(screen.findByText('Foobar')).toBeTruthy(); + expect(await screen.findByText('Foobar')).toBeTruthy(); });src/BaseSelect/index.tsx (1)
800-800:prefix属性正确传递给 Selector 组件
prefix属性已正确传递给 Selector 组件，这确保了前缀将被正确渲染。实现方式与其他属性的传递保持一致。建议在 Selector 组件的文档中添加关于
prefix属性的说明，以便其他开发者了解如何使用这个新功能。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
tests/__snapshots__/Combobox.test.tsx.snapis excluded by!**/*.snaptests/__snapshots__/ssr.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
- README.md (1 hunks)
- assets/index.less (1 hunks)
- docs/examples/custom-icon.tsx (2 hunks)
- src/BaseSelect/index.tsx (3 hunks)
- src/Selector/SingleSelector.tsx (1 hunks)
- src/Selector/index.tsx (3 hunks)
- tests/Multiple.test.tsx (2 hunks)
🧰 Additional context used
🔇 Additional comments (9)
assets/index.less (1)
75-75: 更改看起来合理，但请验证其影响。为
.@{select-prefix}-selection-search类添加position: relative;是一个合理的更改，可能与新的前缀功能有关。这有助于更精确地定位子元素。然而，建议验证此更改对现有布局没有负面影响。请考虑在不同场景下测试选择器组件，以确保其行为符合预期。
src/Selector/index.tsx (3)
75-75: 新增的prefix属性符合预期新增的
prefix属性类型为React.ReactNode，这是一个很好的选择，因为它允许用户传入各种类型的内容（如文本、图标或自定义组件）作为前缀。这个改动与 PR 的目标一致，增强了 Select 组件的灵活性。
114-114:prefix属性的解构符合组件的一致性
prefix属性的解构与组件中其他属性的处理方式保持一致，并且它在解构列表中的位置也很合适，与其他视觉相关的属性放在一起。这种做法有助于保持代码的可读性和一致性。
274-274: 前缀元素的渲染实现得当，但需要注意样式影响前缀元素的条件渲染和类名设置都很合理。将前缀
div放置在selectNode之前也符合左侧前缀的设计要求。这种实现方式为样式和布局控制提供了灵活性。然而，建议考虑以下几点：
- 确保新增的前缀元素不会对现有的选择器布局产生负面影响。
- 可能需要更新相关的 CSS 样式，以确保前缀与选择器的其他部分能够正确对齐和间距。
- 考虑在组件的文档中添加关于如何正确使用和样式化前缀的指导。
为了验证样式的一致性，请运行以下脚本来检查相关的 CSS 文件是否已更新：
README.md (1)
124-124: 新属性 'prefix' 添加正确'prefix' 属性的添加符合拉取请求的目标，支持在 Select 组件中使用前缀。描述简洁明了，表格格式也正确。
不过，根据之前的评论，似乎曾就命名约定进行过讨论（'prefix' 与 'prefixIcon'）。建议确认 'prefix' 是否为最终商定的名称。如果是，那么这个更改是恰当的。
tests/Multiple.test.tsx (2)
22-22: 导入screen是个好的做法从 '@testing-library/react' 导入
screen是正确的，因为它在新的测试用例中被使用。这符合按需导入的最佳实践。
Line range hint
385-685: 现有测试保持不变，这是个好迹象在查看提供的代码时，没有发现对现有测试的重大修改。这表明新添加的前缀功能与现有行为兼容，不需要调整其他测试。这是保持向后兼容性的好迹象。
src/BaseSelect/index.tsx (2)
176-176: 新属性prefix添加正确
prefix属性的添加符合 PR 的目标，允许在 Select 组件左侧添加图标或文本。使用React.ReactNode类型提供了良好的灵活性。
Line range hint
1-863: 总体评价：prefix功能实现完善此 PR 成功地在 BaseSelect 组件中添加了
prefix功能，实现方式简洁有效：
- 在 BaseSelectProps 接口中添加了
prefix属性。- 将
prefix属性正确传递给 Selector 组件。这些更改与 PR 的目标一致，并保持了向后兼容性。代码风格和结构与文件的其余部分保持一致。
建议：
- 在 Selector 组件的文档中添加关于
prefix属性的使用说明。- 考虑添加一个简单的单元测试，以确保
prefix属性被正确传递和渲染。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
README.md (1)
124-124: 新属性prefix的添加是个很好的改进，但有一些建议这个新属性的添加确实增强了 Select 组件的功能，允许用户在选择框左侧添加图标或文本。然而，有几点需要考虑：
默认值 "-" 可能会引起混淆。建议将其改为 "undefined" 或 "-" 并添加说明，表示默认不显示前缀。
之前的讨论中提到了命名一致性的问题。考虑是否应该使用 "prefixIcon" 来与 "suffixIcon" 保持一致，或者将 "suffixIcon" 改为 "suffix"。这样可以提高 API 的一致性和可理解性。
您是否考虑过这些建议？如果需要，我可以协助您修改文档或实现代码。
tests/Multiple.test.tsx (1)
377-384: 新的测试用例有效验证了静态前缀功能这个测试用例很好地验证了 Select 组件中静态前缀的渲染。不过，为了增加测试的稳健性，我建议使用更具体的断言方法。
考虑使用以下方式改进测试：
- expect(screen.findByText('Foobar')).toBeTruthy(); + expect(screen.getByText('Foobar')).toBeInTheDocument();这样的修改可以提供更明确的错误信息，并确保元素确实在文档中。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
tests/__snapshots__/Combobox.test.tsx.snapis excluded by!**/*.snaptests/__snapshots__/ssr.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
- README.md (1 hunks)
- assets/index.less (1 hunks)
- docs/examples/custom-icon.tsx (2 hunks)
- src/BaseSelect/index.tsx (3 hunks)
- src/Selector/SingleSelector.tsx (1 hunks)
- src/Selector/index.tsx (3 hunks)
- tests/Multiple.test.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- assets/index.less
- docs/examples/custom-icon.tsx
- src/BaseSelect/index.tsx
- src/Selector/SingleSelector.tsx
- src/Selector/index.tsx
🧰 Additional context used
🔇 Additional comments (1)
tests/Multiple.test.tsx (1)
22-22: 导入screen是一个好的做法从 '@testing-library/react' 导入
screen是使用 React Testing Library 时的常见做法。这将使得在测试中查询渲染的组件元素变得更加容易和直观。
有时需要再 Select 左侧放图标或文字，类似 Input 的 prefix。
Summary by CodeRabbit
新功能
prefix属性，允许用户自定义选择器和基础选择组件的前缀图标或文本，增强了视觉表现和用户界面的灵活性。maxCount属性，定义可选择的最大项数，进一步增强了组件的自定义能力。prefix属性。样式改进
文档
README.md文件，增加了关于新prefix和maxCount属性的说明，帮助用户更好地理解和使用这些功能。测试
Select.Multiple组件的测试覆盖，新增了验证prefix属性功能的测试用例。