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.

Sign up for GitHub

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

Jump to bottom

chore: optimize disabledDate logic #191

Merged
merged 11 commits into from
Dec 30, 2020
Merged

chore: optimize disabledDate logic #191

merged 11 commits into from
Dec 30, 2020

Conversation

kerm1it
Copy link
Member

@kerm1it kerm1it commented Dec 19, 2020 •
edited
Loading

fix ant-design/ant-design#28155
fix ant-design/ant-design#26697
fix ant-design/ant-design#27517

有可能还有别的 issue，欢迎补一下，之前见过好几次说 disabledDate 有问题。

之前的 disabledDate 判断逻辑是：

判断的基准时间都是当前选择的时间，或者当前时间，因此如果正好这个时间满足 disabledDate 的条件，切换到月份、季度、年、十年等面板的时候，此时的基准时间都是这个时间，因此当前月份、季度、年等都会被禁用

现在的逻辑：

切换到月份面板时，会判断这个月内的每一天是否都满足 disabledDate 条件，只有全部满足，当前月份才会被禁用
年的面板时，判断这一年中的每个月是否满足disabledDate 条件
以此类推

这样做了以后用户的 disabledDate 里面的逻辑可以任意写，都可以满足。

缺点时：判断年或者十年的时候运算量稍微有点大，但是应该不会有卡顿。

大家可以在本地测试一下，看看有什么问题，没有之后我补上测试。

@zombieJ @afc163 @xrkffgg

@vercel
Copy link

vercel bot commented Dec 19, 2020 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/react-component/picker/g32tjnkiy
✅ Preview: https://picker-git-fix-disabledate.react-component.vercel.app

@kerm1it kerm1it mentioned this pull request Dec 19, 2020
Closed
@codecov
Copy link

codecov bot commented Dec 19, 2020 •
edited
Loading

Codecov Report

Merging #191 (1329de9) into master (26726d2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #191   +/-   ##
=======================================
  Coverage   99.60%   99.61%           
=======================================
  Files          45       45           
  Lines        2044     2094   +50     
  Branches      601      616   +15     
=======================================
+ Hits         2036     2086   +50     
  Misses          6        6           
  Partials        2        2
Impacted Files Coverage Δ
src/PanelContext.tsx 100.00% <ø> (ø)
src/PickerPanel.tsx 100.00% <ø> (ø)
src/Picker.tsx 100.00% <100.00%> (ø)
src/RangePicker.tsx 100.00% <100.00%> (ø)
src/panels/DecadePanel/DecadeBody.tsx 100.00% <100.00%> (ø)
src/panels/PanelBody.tsx 100.00% <100.00%> (ø)
src/utils/dateUtil.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26726d2...1329de9. Read the comment docs.

@HelloAny
Copy link

HelloAny commented Dec 20, 2020 •
edited
Loading 

function disabledDate(current: Moment) {
  return current && current < moment().endOf('day') 
}

2

disableDate在这种逻辑下这种情况下decade可以点击，但是高亮消失

@kerm1it
Copy link
Member Author

kerm1it commented Dec 20, 2020 •
edited
Loading 


function disabledDate(current: Moment) {

  return current && current < moment().endOf('day') 

}

2

disableDate在这种逻辑下这种情况下decade可以点击，但是高亮消失

高亮不是有么？背景红不是高亮的样式么？

@HelloAny
Copy link 


function disabledDate(current: Moment) {

  return current && current < moment().endOf('day') 

}

2
disableDate在这种逻辑下这种情况下decade可以点击，但是高亮消失

高亮不是有么？北京红不是高亮的样式么？

DecadePanel/DecadeBody中有一段重复的逻辑，应该删去。这个会导致disabled执行错误
image
我不知道怎么贴代码😂

@kerm1it
Copy link
Member Author

kerm1it commented Dec 20, 2020

删除了，之前没注意到这。

@afc163
Copy link
Member

afc163 commented Dec 21, 2020

看看 ant-design/ant-design#27517 是否也解决了？

@afc163
Copy link
Member

afc163 commented Dec 21, 2020

ant-design/ant-design#27796 (comment)

这种情况是怎么表现的：

使用 disabledDate 禁止选择今天之前的日期， 比如今天是 2020-12-21，先选定 2021-05-01，然后去选择年份，如果此时 2020 年是可选的（因为还有后面几天 12-21~12-21 可选）。这样就会把 2020-05-01 这个不可选的日期选上去。

@kerm1it
Copy link
Member Author

kerm1it commented Dec 21, 2020

ant-design/ant-design#27796 (comment)

这种情况是怎么表现的：

使用 disabledDate 禁止选择今天之前的日期， 比如今天是 2020-12-21，先选定 2021-05-01，然后去选择年份，如果此时 2020 年是可选的（因为还有后面几天 12-21~12-21 可选）。这样就会把 2020-05-01 这个不可选的日期选上去。

这种情况下，切换到年份时，2020 是可以选的，但是选择后会要求继续选择，月份，进入到日期后会发现5月1号不可选的？

@afc163
Copy link
Member

afc163 commented Dec 21, 2020 •
edited
Loading

相当于从 2021-05-01 选择年份 2020 时，如果你发现日期不合法，你会清空 05-01？使其重新选择日/月，是这样么？

@kerm1it
Copy link
Member Author

kerm1it commented Dec 21, 2020

明白了，这个应该还是有问题，这次修复的主要是面板里面的显示逻辑

因为选择了年份后，当前值的年份是直接被设置为对应的值，此时的日期有可能被禁用的，这块儿的逻辑我得看一下

@kerm1it
Copy link
Member Author

kerm1it commented Dec 21, 2020

ant-design/ant-design#27517

这个 case 解决了，修复后的逻辑是，今天之后的日期不能选，选择年份时除了 2019，2020 可以选，其余不能选，进入 2019 月份面板时只有 12 月可选，符合预期。

@afc163
Copy link
Member

afc163 commented Dec 21, 2020

这一块确实特别复杂，相关的 issue 也很多：https://github.com/ant-design/ant-design/search?q=disabledDate&state=open&type=issues

@kerm1it
Copy link
Member Author

kerm1it commented Dec 21, 2020

是的，就是因为之前看到了好多相关的 issue，所以这次才想好好改一下，重新梳理一下这块的逻辑

@kerm1it
Copy link
Member Author

kerm1it commented Dec 21, 2020 •
edited
Loading

ant-design/ant-design#27796 (comment)

这种情况是怎么表现的：

使用 disabledDate 禁止选择今天之前的日期， 比如今天是 2020-12-21，先选定 2021-05-01，然后去选择年份，如果此时 2020 年是可选的（因为还有后面几天 12-21~12-21 可选）。这样就会把 2020-05-01 这个不可选的日期选上去。

在这里试试basic第一个例子: https://picker-git-fix-disabledate.react-component.now.sh/?path=/story/rc-picker--basic

之前是 hover 的时候 placeholder 处理有问题

现在的表现是，选完 5月1号后，选择年份时，选择 2020 了后，月份面板只有12月是可用的，此时如果失去焦点，值还是 2021-05-01 只有完整的选完 年月日后，值才会更新。

@HelloAny
Copy link

HelloAny commented Dec 21, 2020 •
edited
Loading

ant-design/ant-design#27796 (comment)
这种情况是怎么表现的：
使用 disabledDate 禁止选择今天之前的日期， 比如今天是 2020-12-21，先选定 2021-05-01，然后去选择年份，如果此时 2020 年是可选的（因为还有后面几天 12-21~12-21 可选）。这样就会把 2020-05-01 这个不可选的日期选上去。

在这里试试basic第一个例子: https://picker-git-fix-disabledate.react-component.now.sh/?path=/story/rc-picker--basic

之前是 hover 的时候 placeholder 处理有问题

现在的表现是，选完 5月1号后，选择年份时，选择 2020 了后，月份面板只有12月是可用的，此时如果失去焦点，值还是 2021-05-01 只有完整的选完 年月日后，值才会更新。

这种方法并未执行PickerPanel中，如果单独引用PickerPanel组件在onPanelChange时还是会失效，因为Picker是基于PickerPanel的，所以我的办法是对PanelBody的点击事件进行判定拦截，如果不符合条件(disableDate)就会跳转到下一阶段：比如你在example中写的，选择了2021年5月1日后，如果这个时候点击2020年，就会跳转到2020年的月份表，月份表点击后就会进入日期，在这个过程中因为不执行onSelect方法，所以在失焦的时候还是会在原来的2021年5月1日。

如果有必要的我可以截图，代码似乎pr不上来

@kerm1it
Copy link
Member Author

kerm1it commented Dec 21, 2020 •
edited
Loading

没明白你说的什么意思？有什么问题么？

@HelloAny
Copy link

没明白你说的什么意思？有什么问题么？

在example/customize中的PickerPanel，上述问题还是会在onPanelChange中出现，可以看看打印出来的数据

@kerm1it
Copy link
Member Author

kerm1it commented Dec 21, 2020 •
edited
Loading

没明白你说的什么意思？有什么问题么？

在example/customize中的PickerPanel，上述问题还是会在onPanelChange中出现，可以看看打印出来的数据

明白了，这个没事，打印出来的那个值只是视图的值，并不是最终输出的值，value 并没有改变。

@zombieJ
Copy link
Member

zombieJ commented Dec 28, 2020

当时没有做全遍历就是因为日期一多 dev 模式会有明显的卡顿。我有点想搞个 disabledDate(date, panel) 这种形式但是有担心太繁琐了。

@kerm1it
Copy link
Member Author

kerm1it commented Dec 28, 2020

当时没有做全遍历就是因为日期一多 dev 模式会有明显的卡顿。我有点想搞个 disabledDate(date, panel) 这种形式但是有担心太繁琐了。

你意思暴露出去，让用户自己写逻辑？

@zombieJ
Copy link
Member

zombieJ commented Dec 28, 2020

有这个想法，但是觉得不好。对用户而言太难用了。

zombieJ
zombieJ reviewed Dec 28, 2020
View reviewed changes
src/Picker.tsx Outdated Show resolved Hide resolved
@kerm1it
Copy link
Member Author

kerm1it commented Dec 28, 2020

有这个想法，但是觉得不好。对用户而言太难用了。

对呀，对用户来说逻辑有点复杂了

要不先这样试试，后续看看反馈？

@zombieJ
Copy link
Member

zombieJ commented Dec 28, 2020

要不先这样试试，后续看看反馈？

嗯~~

zombieJ
zombieJ reviewed Dec 28, 2020
View reviewed changes
src/utils/dateUtil.ts Outdated Show resolved Hide resolved
@kerm1it
Copy link
Member Author

kerm1it commented Dec 28, 2020

测试用例加完了，没问题就可以合并了。

@kerm1it kerm1it requested a review from zombieJ December 28, 2020 14:40
@kerm1it

This comment has been minimized.

Sign in to view
@kerm1it kerm1it merged commit 42f853b into master Dec 30, 2020
@kerm1it kerm1it deleted the fix-disableDate branch December 30, 2020 03:59
@kerm1it
Copy link
Member Author

kerm1it commented Dec 30, 2020

@zombieJ @afc163 我先合并了，有时间发个版本试试

@kerm1it

This comment has been minimized.

Sign in to view
@HelloAny

This comment has been minimized.

Sign in to view
@kerm1it

This comment has been minimized.

Sign in to view
@HelloAny

This comment has been minimized.

Sign in to view
@afc163
Copy link
Member

afc163 commented Dec 30, 2020

@kerm1it npm username 是啥，我加你

This was referenced Mar 12, 2021
Closed
Closed
Closed
Merged
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afc163 afc163 afc163 approved these changes

@zombieJ zombieJ Awaiting requested review from zombieJ

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@kerm1it @HelloAny @afc163 @zombieJ