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: add leftTime for useCountDown #1737

Merged
merged 39 commits into from
Jul 22, 2022
Merged

Conversation

li-jia-nan
Copy link
Collaborator

@li-jia-nan li-jia-nan commented Jul 5, 2022

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

💡 Background and solution

close #1727

📝 Changelog

useCountDown 增加 leftTime 参数,支持剩余时间的倒计时,同时优化了一些代码细节:

  • TDate 类型替换成 dayjs.ConfigType,因为dayjs已经定义过了,感觉没必要重复定义
  • new Date().getTime() 更新为ES6中的 Date.now(),性能更好
  • Math.floor()方法取整更新为位运算,性能更好
  • 我看到这个hook提供了 onEnd 回调,现在考虑到要不要加一个 onStart 回调?但是好像没啥必要,onStart 好像直接用useEffect即可实现?
Language Changelog
🇺🇸 English add leftTime for
🇨🇳 Chinese 增加 leftTime 参数

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

@li-jia-nan
Copy link
Collaborator Author

有点问题,jest没跑过,我先close掉

@li-jia-nan li-jia-nan closed this Jul 6, 2022
@li-jia-nan li-jia-nan reopened this Jul 6, 2022
@li-jia-nan li-jia-nan closed this Jul 6, 2022
@li-jia-nan li-jia-nan reopened this Jul 6, 2022
@li-jia-nan
Copy link
Collaborator Author

@hchlq @crazylxr 大佬帮忙看下,测试用例跑不过

@crazylxr
Copy link
Collaborator

crazylxr commented Jul 6, 2022

@hchlq @crazylxr 大佬帮忙看下,测试用例跑不过

你本地跑一下呢,看看能不能过。 不能的话你可能需要看一下是测试用例的问题,还是代码的问题了。

@li-jia-nan li-jia-nan closed this Jul 6, 2022
@li-jia-nan li-jia-nan reopened this Jul 6, 2022
@frontlich
Copy link

calcLeftTime 函数最好使用时间戳计算,因为

手机黑屏或者页面失焦的时候,有可能setInterval会暂停执行,当界面重新恢复的时候,pre -interval 计算出来的剩余时间就偏大了

const startTime = Date.now(); // 记录一下开始时的时间戳

calcLeftTime函数里
const passTime = Date.now() - startTime; // 倒计时走过的时间
const restTime = leftTime - passTime; // 倒计时剩余的时间

上面写法可获取到正确的剩余时间

@li-jia-nan
Copy link
Collaborator Author

li-jia-nan commented Jul 6, 2022

calcLeftTime 函数最好使用时间戳计算,因为

手机黑屏或者页面失焦的时候,有可能setInterval会暂停执行,当界面重新恢复的时候,pre -interval 计算出来的剩余时间就偏大了

const startTime = Date.now(); // 记录一下开始时的时间戳

calcLeftTime函数里 const passTime = Date.now() - startTime; // 倒计时走过的时间 const restTime = leftTime - passTime; // 倒计时剩余的时间

上面写法可获取到正确的剩余时间

用时间戳的话会出现比较严重的精度问题,时间间隔不一定能和 setInterval 的间隔同步,比如 5000ms 倒计时可能是3998,2997,1994,992……这样的情况,可能会带来无法预期的后果

@frontlich
Copy link

calcLeftTime 函数最好使用时间戳计算,因为
手机黑屏或者页面失焦的时候,有可能setInterval会暂停执行,当界面重新恢复的时候,pre -interval 计算出来的剩余时间就偏大了
const startTime = Date.now(); // 记录一下开始时的时间戳
calcLeftTime函数里 const passTime = Date.now() - startTime; // 倒计时走过的时间 const restTime = leftTime - passTime; // 倒计时剩余的时间
上面写法可获取到正确的剩余时间

用时间戳的话会出现比较严重的精度问题,时间间隔不一定能和 setInterval 的间隔同步,比如 5000ms 倒计时可能是3998,2997,1994,992……这样的情况,可能会带来无法预期的后果

Math.round(3998 / 1000) * 1000 这样可以吧?

@li-jia-nan
Copy link
Collaborator Author

calcLeftTime 函数最好使用时间戳计算,因为
手机黑屏或者页面失焦的时候,有可能setInterval会暂停执行,当界面重新恢复的时候,pre -interval 计算出来的剩余时间就偏大了
const startTime = Date.now(); // 记录一下开始时的时间戳
calcLeftTime函数里 const passTime = Date.now() - startTime; // 倒计时走过的时间 const restTime = leftTime - passTime; // 倒计时剩余的时间
上面写法可获取到正确的剩余时间

用时间戳的话会出现比较严重的精度问题,时间间隔不一定能和 setInterval 的间隔同步,比如 5000ms 倒计时可能是3998,2997,1994,992……这样的情况,可能会带来无法预期的后果

Math.round(3998 / 1000) * 1000 这样可以吧?

这样理论上来说对毫秒是可以的,但是不知道会不会影响到 dayshoursminutesseconds 这些单位,我还需要再测试一下

@li-jia-nan
Copy link
Collaborator Author

@hchlq @crazylxr 大佬们麻烦review一下 1740 这个pr,修复了测试用例的问题,我这边需要先merge一下才能跑通

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ li-jia-nan
❌ crazylxr
You have signed the CLA already but the status is still pending? Let us recheck it.

@crazylxr
Copy link
Collaborator

crazylxr commented Jul 7, 2022

@hchlq @crazylxr 大佬们麻烦review一下 1740 这个pr,修复了测试用例的问题,我这边需要先merge一下才能跑通

好的,已经 merge

@li-jia-nan
Copy link
Collaborator Author

@hchlq @crazylxr 大佬们麻烦review一下 1740 这个pr,修复了测试用例的问题,我这边需要先merge一下才能跑通

好的,已经 merge

OK了,我pull了

packages/hooks/src/useCountDown/index.ts Outdated Show resolved Hide resolved
packages/hooks/src/useCountDown/index.ts Outdated Show resolved Hide resolved
packages/hooks/src/useCountDown/index.ts Outdated Show resolved Hide resolved
@li-jia-nan li-jia-nan requested a review from hchlq July 7, 2022 09:11
@li-jia-nan
Copy link
Collaborator Author

现在代码里面好多地方判断是 leftTime 还是 targetDate

我提个建议:初始化的时候,把 targetDate 转换成 leftTime,这样是不是核心代码就只用处理 leftTime 了?

我改一下

@li-jia-nan
Copy link
Collaborator Author

@crazylxr @hchlq 根据砖家的建议又改了一版,把 leftTimetarget 统一转化成 leftTime 了,麻烦 review 一下

packages/hooks/src/useCountDown/index.ts Outdated Show resolved Hide resolved
packages/hooks/src/useCountDown/index.ts Outdated Show resolved Hide resolved
@li-jia-nan
Copy link
Collaborator Author

@crazylxr @hchlq @brickspert 根据砖家的建议又双叒改了一版,把 leftTime 和 target 统一转化成 targetDate了,麻烦再 review 一下,辛苦!

packages/hooks/src/useCountDown/index.ts Outdated Show resolved Hide resolved
packages/hooks/src/useCountDown/index.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@brickspert brickspert left a comment

Choose a reason for hiding this comment

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

LGTM

@brickspert brickspert merged commit e30d308 into alibaba:master Jul 22, 2022
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.

希望useCountdown增加一个参数
7 participants