ant-design / ant-design-mobile

Essential UI blocks for building mobile web apps.
https://mobile.ant.design
MIT License
11.62k stars 2.39k forks source link

feat: calendar picker view scroll #6521

Open Yueyanc opened 8 months ago

Yueyanc commented 8 months ago

测试有两个未通过,不太清楚为什么。 image image

Yueyanc commented 8 months ago

6425

Yueyanc commented 8 months ago

@zombieJ 这块这样实现可行吗,可行的话我添加文档和修复用例报错

github-actions[bot] commented 8 months ago

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

Yueyanc commented 8 months ago

@zombieJ 帮忙review一下呢

bravepg commented 8 months ago

感觉这个 api 有点奇怪。是不是打开弹窗之后最好可以直接定位到选择的日期,而不是再调用个 api 来滚动到相应的位置?

Yueyanc commented 8 months ago

感觉这个 api 有点奇怪。是不是打开弹窗之后最好可以直接定位到选择的日期,而不是再调用个 api 来滚动到相应的位置?

弹窗打开的时候内部调用了这个api 不需要再手动调用了

bravepg commented 8 months ago

感觉这个 api 有点奇怪。是不是打开弹窗之后最好可以直接定位到选择的日期,而不是再调用个 api 来滚动到相应的位置?

弹窗打开的时候内部调用了这个api 不需要再手动调用了

image 那这里就可以不用暴露了吧?

Yueyanc commented 8 months ago

感觉这个 api 有点奇怪。是不是打开弹窗之后最好可以直接定位到选择的日期,而不是再调用个 api 来滚动到相应的位置?

弹窗打开的时候内部调用了这个api 不需要再手动调用了

image 那这里就可以不用暴露了吧?

这里是因为考虑到可能有用户想要定位到其他日期才添加上去的。需要去掉吗

Yueyanc commented 8 months ago

感觉这个 api 有点奇怪。是不是打开弹窗之后最好可以直接定位到选择的日期,而不是再调用个 api 来滚动到相应的位置?

弹窗打开的时候内部调用了这个api 不需要再手动调用了

image 那这里就可以不用暴露了吧?

scrollTo这个api是通过ref传出来的,如果文档里面不写,但是用户在ref里面能看见这个api,会不会有疑惑

codecov[bot] commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (b431a18) 92.27% compared to head (45cd1d1) 92.29%. Report is 1 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #6521 +/- ## ========================================== + Coverage 92.27% 92.29% +0.02% ========================================== Files 316 316 Lines 6897 6906 +9 Branches 1728 1733 +5 ========================================== + Hits 6364 6374 +10 + Misses 497 496 -1 Partials 36 36 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

zombieJ commented 8 months ago

@bravepg,印象里渲染面板数量有限。这个 scrollTo 感觉容易超出范围经常不生效。确认一下。

Jarryxin commented 8 months ago

这个实现有些缺陷,理由如下:

  1. scrollTo后只是将要展示的日期显示到屏幕中间(调用 scrollIntoView({ block: 'center' })),展示的截断区域比较奇怪,如下,最好是将当前月完整显示 image
  2. data-date加cell日期的方式只改了默认cell显示的属性,renderDate自定义日期渲染的情况未处理
  3. 当设置了min后,默认未从defaultValue/value设置的日期开始渲染,当 visible 为 true 时,calendarRef.current不能立刻取到,需要加setImmediate获取 calendarRef

@zombieJ @Yueyanc

Yueyanc commented 8 months ago

这个实现有些缺陷,理由如下:

  1. scrollTo后只是将要展示的日期显示到屏幕中间(调用 scrollIntoView({ block: 'center' })),展示的截断区域比较奇怪,如下,最好是将当前月完整显示

image

  1. data-date加cell日期的方式只改了默认cell显示的属性,renderDate自定义日期渲染的情况未处理

  2. 当设置了min后,默认未从defaultValue/value设置的日期开始渲染,当 visible 为 true 时,calendarRef.current不能立刻取到,需要加setImmediate获取 calendarRef

@zombieJ @Yueyanc

🤡我当时没想这么多 @bravepg 老哥怎么看