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

fix(Calendar): fix the issue where disabled dates can be selected und… #13080

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pany-ang
Copy link
Contributor

switch-modenone 时不存在该问题。

当为 monthyear-month 时,如果用户选择的「开始日期」和「结束日期」中间包含了「禁用日期」,那么该范围依旧能被选中,预期是无论如何都不能选中「禁用日期」

该 PR 保证了 nonemonthyear-month 三个模式下 UI 行为是一致的。

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 89.68%. Comparing base (eef8e0b) to head (90cbfd1).
Report is 2 commits behind head on main.

Files Patch % Lines
packages/vant/src/calendar/Calendar.tsx 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13080      +/-   ##
==========================================
+ Coverage   89.66%   89.68%   +0.01%     
==========================================
  Files         257      257              
  Lines        6987     6988       +1     
  Branches     1724     1725       +1     
==========================================
+ Hits         6265     6267       +2     
  Misses        382      382              
+ Partials      340      339       -1     

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

Copy link
Member

@chenjiahan chenjiahan left a comment

Choose a reason for hiding this comment

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

方便加个单元测试嘛

@inottn
Copy link
Collaborator

inottn commented Aug 24, 2024

感谢 PR,因为切换和非切换的实现原理不同,当 switch-mode 不为 none 时,从底层实现上来看,我们已经做不到选择的时间范围内不包含「禁用日期」,基于这个 PR 改动,用户在切换月份后,仍可以轻松地绕开这一限制。

平铺的模式下会渲染所有的月份,所以可以获取对应的数据,但代价是增加了性能开销,这也是我们引入 switch-mode 的原因。因为交互相似,参考了其它组件库的 RangePicker 组件对于这个场景的处理,它们确保起始和结束日期不能为「禁用日期」,现在的交互也是如此。考虑到不同 switch-mode 的交互和真实的适用场景不同,部分功能有所差异是可以接受的。

如果你有更好的实现方案,也可以在此讨论。

@pany-ang
Copy link
Contributor Author

用户在切换月份后,仍可以轻松地绕开这一限制

嗯对,该 PR 只解决了用户在当前显示月份进行范围选择时不能选中禁用日期的问题。

我想的话,好像可以用一个顶层 map 来缓存一下已经渲染过的月份的禁用日期,并且 onPanelChange 每次切换月份时,都去更新一下该 map 中对应月份的数据。

然后我们用该 map 来限制用户不能选中禁用日期即可。

const disabledDaysCache = ref<{ [key: string]: CalendarDayItem[] }>({});

@inottn
Copy link
Collaborator

inottn commented Aug 27, 2024

用户在切换月份后,仍可以轻松地绕开这一限制

嗯对,该 PR 只解决了用户在当前显示月份进行范围选择时不能选中禁用日期的问题。

我想的话,好像可以用一个顶层 map 来缓存一下已经渲染过的月份的禁用日期,并且 onPanelChange 每次切换月份时,都去更新一下该 map 中对应月份的数据。

然后我们用该 map 来限制用户不能选中禁用日期即可。

const disabledDaysCache = ref<{ [key: string]: CalendarDayItem[] }>({});

用户应该可以通过切换年,再切换月绕开这一限制 🤔

@pany-ang
Copy link
Contributor Author

😱

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

Successfully merging this pull request may close these issues.

4 participants