Skip to content
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(CalendarPickerView): add renderTop and renderBottom hide logic #6735

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Layouwen
Copy link
Contributor

增加 renderTop 和 renderBottom false 隐藏

Copy link

codecov bot commented Aug 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.69%. Comparing base (b805b5a) to head (6f01d6b).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6735   +/-   ##
=======================================
  Coverage   92.68%   92.69%           
=======================================
  Files         334      334           
  Lines        7191     7200    +9     
  Branches     1795     1790    -5     
=======================================
+ Hits         6665     6674    +9     
+ Misses        517      490   -27     
- Partials        9       36   +27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

PR preview has been successfully built and deployed to https://antd-mobile-preview-pr-6735.surge.sh

@@ -21,8 +21,8 @@ Only the simplest content area is shown here, and other more usages can be consu
| max | Maximum value of a selectable range. | `Date` | - |
| min | Minimum value of a selectable range. | `Date` | - | - |
| onChange | Trigger when selected date changes. | `(val: Date \| null) => void` when selection mode is "single". `(val: [Date, Date] \| null) => void` when selection mode is "range". | - |
| renderTop | The top information of date render function. | `(date: Date) => ReactNode \| null \| undefined` | - |
| renderBottom | The bottom information of date render function. | `(date: Date) => ReactNode \| null \| undefined` | - |
| renderTop | The top information of date render function. | `((date: Date) => ReactNode \| null \| undefined) \| false` | - |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

false 这个加一下版本号说明,参考:
截屏2024-08-30 11 02 59

@@ -44,9 +44,9 @@ export type CalendarPickerViewProps = {
title?: React.ReactNode | false
confirmText?: string
weekStartsOn?: 'Monday' | 'Sunday'
renderTop?: (date: Date) => React.ReactNode
renderTop?: ((date: Date) => React.ReactNode) | false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

加一下对应的测试用例

@Layouwen Layouwen requested a review from zombieJ August 30, 2024 03:12
@@ -243,6 +245,8 @@ export const CalendarPickerView = forwardRef<
(minDay && d.isBefore(minDay, 'day'))

const renderTop = () => {
if (props.renderTop === false) return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个逻辑看起来没有用?下面的代码里已经写了

if (props.renderXXX === false) return

所有 renderXXX 走到的地方其实已经都判断过了

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renderXXX 类型为 function | false
这里如果不把 boolean 过滤掉 ts 会报错, 所以才加上的

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

那可以考虑调整一下代码结构,也不需要这个 renderTop 函数了,直接内联写。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants