-
Notifications
You must be signed in to change notification settings - Fork 262
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(calendar): support deleteIcon props #2644
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #2644 +/- ##
=======================================
Coverage 83.88% 83.89%
=======================================
Files 218 218
Lines 17888 17890 +2
Branches 2609 2609
=======================================
+ Hits 15006 15008 +2
Misses 2877 2877
Partials 5 5 ☔ View full report in Codecov by Sentry. |
Walkthrough此拉取请求引入了一个新的可选属性 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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: 1
🧹 Outside diff range and nitpick comments (9)
src/packages/calendar/demos/h5/demo1.tsx (2)
43-43
: 实现了自定义关闭图标,但建议增加灵活性成功实现了支持自定义关闭图标的新功能,这与 PR 的目标相符。然而,有以下几点建议:
- 考虑使用更直观的图标作为关闭按钮,如 'Close' 或 'X' 图标,以提高用户体验。
- 建议添加一个条件渲染,允许在自定义图标和默认关闭图标之间切换。这样可以增加组件的灵活性。
建议修改如下:
<Calendar visible={isVisible} showTitle={false} defaultValue={date} onClose={closeSwitch} onConfirm={setChooseValue} onDayClick={select} - closeIcon={<Star />} + closeIcon={customCloseIcon ? <Star /> : undefined} />这里的
customCloseIcon
可以是一个新的 state 或 prop,用于控制是否使用自定义关闭图标。
Line range hint
1-47
: 总体评价:功能实现正确,但有改进空间该 PR 成功实现了支持自定义关闭图标的新功能,符合预期目标。代码质量良好,没有明显的错误或问题。
然而,为了进一步提高组件的灵活性和用户体验,我们提出了一些改进建议,主要涉及:
- 使用更直观的关闭图标
- 增加条件渲染以支持默认图标和自定义图标的切换
这些改进将使组件更加通用和易用。
如果您需要帮助实现这些改进或有任何疑问,请随时告诉我。我可以提供更详细的代码示例或解释。
src/packages/calendar/doc.md (1)
123-123
: 属性添加正确,建议优化描述新增的
closeIcon
属性正确地添加到了文档中,包括了类型和默认值。这与 PR 的目标一致。为了提高文档的清晰度,建议稍微扩展一下描述,例如:
- | closeIcon | 自定义 Icon | `ReactNode` | `close` | + | closeIcon | 自定义关闭图标,用于替换默认的关闭图标 | `ReactNode` | `close` |这样可以更清楚地说明该属性的用途。
src/packages/calendar/doc.zh-TW.md (1)
123-123
: 新屬性closeIcon
添加正確新增的
closeIcon
屬性描述清晰,類型和默認值都已正確提供。這個添加增強了 Calendar 組件的自定義能力,使用者現在可以自定義關閉圖標。建議考慮在描述中添加一個簡短的使用示例,以幫助使用者更好地理解如何使用這個新屬性。例如:
- | closeIcon | 自定義 Icon | `ReactNode` | `close` | + | closeIcon | 自定義關閉圖標,例如 `<CloseIcon />` | `ReactNode` | `close` |src/packages/calendar/calendar.tsx (2)
26-26
: 建议在 defaultProps 中为 closeIcon 添加默认值虽然
closeIcon
是一个可选属性,但建议在defaultProps
对象中为其添加一个默认值,即使是undefined
。这样可以使代码更加一致和可预测。例如:const defaultProps = { // ... 其他默认属性 closeIcon: undefined, // ... 其他默认属性 } as CalendarProps这个改动可以提高代码的可维护性和一致性。
92-92
: closeIcon 属性的使用正确,建议小改进
closeIcon
属性的解构和传递给Popup
组件的实现是正确的,符合接口修改和 PR 的目标。为了进一步提高代码的可维护性,建议使用对象展开运算符来传递 props。这样可以确保将来添加的属性也能正确传递给
Popup
组件。例如:<Popup {...props} className="nut-calendar-popup" visible={visible} position="bottom" round closeable destroyOnClose onOverlayClick={closePopup} onCloseIconClick={closePopup} style={{ height: '83%' }} > {renderItem()} </Popup>这个改动可以使代码更加灵活,减少未来可能出现的问题。
Also applies to: 177-177
src/packages/calendar/calendar.taro.tsx (3)
26-26
: 新属性closeIcon
添加正确
closeIcon
属性的添加符合PR的目标,并且类型选择恰当。这个可选属性增加了组件的灵活性,同时保持了向后兼容性。建议为这个新属性添加简短的注释,说明其用途,例如:
/** 自定义关闭图标 */ closeIcon?: ReactNode这将有助于其他开发者更快地理解该属性的作用。
181-181
:closeIcon
属性正确传递给 Popup 组件
closeIcon
属性已正确传递给 Popup 组件,这样可以在弹出窗口中使用自定义的关闭图标。为了提高代码的可读性,建议在 Popup 组件的属性中添加一个换行,使
closeIcon
属性更加醒目:<Popup className="nut-calendar-popup" visible={visible} position="bottom" round closeable destroyOnClose onOverlayClick={closePopup} onCloseIconClick={closePopup} style={{ height: '83%' }} closeIcon={closeIcon} > {renderItem()} </Popup>这样可以使属性列表更加整洁,便于未来的维护和阅读。
26-26
: 总体评价:closeIcon
功能实现完善此PR成功地在日历组件中添加了自定义关闭图标的支持。主要变更包括:
- 在
CalendarProps
接口中添加closeIcon
属性- 在组件的属性解构中包含
closeIcon
- 将
closeIcon
传递给 Popup 组件这些变更正确实现了PR的目标,增强了组件的灵活性和可定制性。代码质量良好,逻辑清晰。
为了进一步提高组件的可维护性和可测试性,可以考虑以下建议:
- 为新添加的
closeIcon
属性添加单元测试,确保其正确渲染和功能正常。- 在组件文档中详细说明
closeIcon
的用法和示例,帮助其他开发者更好地使用这个新功能。- 考虑添加一个默认的关闭图标,在用户未提供自定义图标时使用,这样可以确保始终有一个可用的关闭按钮。
这些建议可以在未来的迭代中考虑实施,以进一步完善该功能。
Also applies to: 92-92, 181-181
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
- src/packages/calendar/calendar.taro.tsx (3 hunks)
- src/packages/calendar/calendar.tsx (3 hunks)
- src/packages/calendar/demos/h5/demo1.tsx (2 hunks)
- src/packages/calendar/demos/taro/demo1.tsx (2 hunks)
- src/packages/calendar/doc.en-US.md (1 hunks)
- src/packages/calendar/doc.md (1 hunks)
- src/packages/calendar/doc.taro.md (1 hunks)
- src/packages/calendar/doc.zh-TW.md (1 hunks)
🧰 Additional context used
🔇 Additional comments (8)
src/packages/calendar/demos/h5/demo1.tsx (1)
3-3
: 导入 Star 图标以自定义日历关闭按钮新增的 Star 图标导入语句与 PR 的目标一致,为日历组件提供了自定义关闭图标的能力。这个改动增强了组件的灵活性。
src/packages/calendar/demos/taro/demo1.tsx (3)
3-3
: 导入 Star 图标符合新功能要求新增的
Star
图标导入语句与PR的目标一致,为日历组件提供了自定义关闭图标的支持。这个改动是合理的,并且为后续的组件属性修改做好了准备。
Line range hint
1-47
: 总体评价:新功能实现良好,建议澄清属性名称此PR成功实现了允许用户为日历组件自定义关闭图标的新功能。代码变更简洁明了,新增的
Star
图标导入和closeIcon
属性的使用都符合预期。这些更改增强了组件的灵活性,使用户能够更好地自定义界面。然而,建议澄清属性名称是
closeIcon
还是deleteIcon
,以确保代码、文档和PR描述之间的一致性。总的来说,这是一个有价值的功能添加,提高了组件的可定制性。
43-43
: 新增 closeIcon 属性实现了自定义关闭图标功能新增的
closeIcon
属性成功实现了允许用户自定义日历组件关闭图标的功能,这与PR的目标相符。使用Star
图标作为示例很好地展示了这个新特性的用法。然而,我注意到代码中使用的属性名是
closeIcon
,而AI生成的摘要中提到的是deleteIcon
。建议确认正确的属性名称,并在必要时更新相关文档和代码注释,以保持一致性。为了验证属性名称的一致性,请运行以下脚本:
✅ Verification successful
验证关闭图标属性名称一致性
在
Calendar
组件中,closeIcon
属性被正确使用。请确保相关文档和代码摘要中使用的属性名称与代码保持一致,建议将 AI 摘要中的deleteIcon
更新为closeIcon
以避免混淆。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述:验证 Calendar 组件中的属性名称是否一致 # 测试:搜索 Calendar 组件的定义和使用。预期:应该只出现 closeIcon 或 deleteIcon 中的一个。 rg --type typescript -e 'interface.*Calendar.*Props' -e 'const.*Calendar.*=' -A 10 -B 10Length of output: 125
Script:
#!/bin/bash # 描述:验证 Calendar 组件中的属性名称是否一致 # 测试:搜索 Calendar 组件的定义和使用。预期:应该只出现 closeIcon 或 deleteIcon 中的一个。 rg --type ts -e 'interface.*Calendar.*Props' -e 'const.*Calendar.*=' -A 10 -B 10 rg --type tsx -e 'interface.*Calendar.*Props' -e 'const.*Calendar.*=' -A 10 -B 10Length of output: 106488
Script:
#!/bin/bash # 描述:验证 Calendar 组件中的属性名称是否一致 # 搜索 .ts 和 .tsx 文件中的 CalendarProps 接口和 Calendar 组件的定义 rg -g "*.ts" -e 'interface\s+CalendarProps' -e 'const\s+Calendar\s*=' -A 10 -B 10 rg -g "*.tsx" -e 'interface\s+CalendarProps' -e 'const\s+Calendar\s*=' -A 10 -B 10Length of output: 5483
Script:
#!/bin/bash # 描述:搜索所有 .ts 和 .tsx 文件中 `closeIcon` 和 `deleteIcon` 的使用情况 rg -g "*.ts" -g "*.tsx" -e 'closeIcon' -e 'deleteIcon'Length of output: 12613
src/packages/calendar/doc.taro.md (1)
123-123
: 属性名称与 PR 描述不一致新添加的属性
closeIcon
看起来实现了 PR 中描述的功能。然而,PR 描述中提到的是deleteIcon
,而不是closeIcon
。建议:
- 确认属性的正确名称是
closeIcon
还是deleteIcon
。- 如果
closeIcon
是正确的,请更新 PR 描述以保持一致性。- 如果
deleteIcon
是正确的,请更新此文档中的属性名称。除此之外,新属性的添加看起来很好,并且与现有文档格式保持一致。
请运行以下脚本来验证属性名称的一致性:
src/packages/calendar/calendar.tsx (2)
26-26
: 新属性closeIcon
添加正确新增的
closeIcon
属性类型为ReactNode
,这允许用户灵活地自定义关闭图标。这个改动符合 PR 的目标,增强了组件的可定制性。
Line range hint
1-187
: 总体评估:代码变更符合预期且质量良好
- 新增的
closeIcon
属性成功实现了自定义关闭图标的功能,符合 PR 的目标。- 代码实现整体正确,与接口修改保持一致。
- 代码质量良好,只有少量改进建议:
- 在
defaultProps
中为closeIcon
添加默认值- 使用对象展开运算符传递 props 到
Popup
组件这些小改进可以进一步提高代码的可维护性和灵活性。总的来说,这个 PR 的实现是成功的,经过小幅调整后可以合并。
src/packages/calendar/calendar.taro.tsx (1)
92-92
: 属性解构正确添加closeIcon
closeIcon
属性已正确添加到组件的属性解构中,与接口的变更保持一致。
@@ -120,6 +120,7 @@ import { Calendar } from '@nutui/nutui-react' | |||
| showSubTitle | whether to show sub title for calendar | `boolean` | `true` | | |||
| scrollAnimation | whether to start scroll animation | `boolean` | `true` | | |||
| firstDayOfWeek | first day of week | `0-6` | `0` | | |||
| closeIcon | Custom Icon | `ReactNode` | `close` | |
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.
新属性 closeIcon
已正确添加,但存在一些问题需要注意。
-
新属性
closeIcon
已正确添加到 Props 表格中,包括描述、类型和默认值。这很好。 -
然而,我注意到文档中使用的属性名称是
closeIcon
,而 PR 目标中提到的是deleteIcon
。这里存在不一致性,需要确认正确的属性名称并在整个文档中保持一致。 -
建议在演示部分添加一个使用这个新属性的例子,以便用户更好地理解如何使用它。
🤔 这个变动的性质是?
Summary by CodeRabbit
closeIcon
属性,允许用户自定义关闭图标。closeIcon
属性及其用法说明。