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: Statistic.Countdown pause and start api #26229

Closed
wants to merge 6 commits into from

Conversation

zhangchen915
Copy link
Contributor

@zhangchen915 zhangchen915 commented Aug 16, 2020

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style 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

close #26182

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English Statistic.Countdown pause and start
🇨🇳 Chinese Statistic.Countdown 增加开始和暂停

☑️ 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

View rendered components/statistic/index.en-US.md
View rendered components/statistic/index.zh-CN.md

@ant-design-bot
Copy link
Contributor

ant-design-bot commented Aug 16, 2020

@ant-design-bot
Copy link
Contributor

ant-design-bot commented Aug 16, 2020

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 16, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit cdbe286:

Sandbox Source
antd reproduction template Configuration

Copy link
Member

@xrkffgg xrkffgg left a comment

Choose a reason for hiding this comment

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

feature branch

@zhangchen915 zhangchen915 changed the base branch from master to feature August 16, 2020 13:10
@codecov
Copy link

codecov bot commented Aug 17, 2020

Codecov Report

Merging #26229 into feature will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           feature   #26229   +/-   ##
========================================
  Coverage    99.54%   99.54%           
========================================
  Files          373      373           
  Lines         7315     7318    +3     
  Branches      2033     2041    +8     
========================================
+ Hits          7282     7285    +3     
  Misses          33       33           
Impacted Files Coverage Δ
components/cascader/index.tsx 100.00% <100.00%> (ø)
components/space/index.tsx 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 d4ea5d2...d851e22. Read the comment docs.

@xrkffgg
Copy link
Member

xrkffgg commented Aug 17, 2020

rebase featrue 后
git push -f

@xrkffgg
Copy link
Member

xrkffgg commented Aug 17, 2020

另外建议改用 hooks

@zhangchen915
Copy link
Contributor Author

rebase 到 featrue 发现 CI 有点问题,改用 hooks 是改成函数式组件吗?

@xrkffgg
Copy link
Member

xrkffgg commented Aug 17, 2020

嗯,Statistic 已经改了,可以把 Countdown 也改一下

@zombieJ
Copy link
Member

zombieJ commented Aug 17, 2020

为啥需要 autoStart?

@zombieJ
Copy link
Member

zombieJ commented Aug 17, 2020

这样设计 value 就不受控了

const [countdownId, setCountdownId] = React.useState<number>();
const [pauseTime, setPauseTime] = React.useState<number>();
const [totalPauseTime, setTotalPauseTime] = React.useState<number>(0);
const [, forceUpdate] = React.useState({});
Copy link
Member

Choose a reason for hiding this comment

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

forceUpdate 可以等一下 #26270 结果

Copy link
Contributor Author

Choose a reason for hiding this comment

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

换成全局的 useForceUpdate 了

| Name | Description | Version |
| ------- | --------------- | ------- |
| start() | Start countdown | |
| pause() | Pause countdown | |
Copy link
Member

Choose a reason for hiding this comment

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

不要提供方法出来,会让 value 非受控。提供一个 disabled 属性,设置后停止计时器,让用户恢复时重新提供一个新的 value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的

Copy link
Member

Choose a reason for hiding this comment

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

paused 如何?

Copy link
Member

Choose a reason for hiding this comment

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

paused 感觉 更符合 这个操作一些

@zombieJ
Copy link
Member

zombieJ commented Aug 31, 2020

@zhangchen915

@xrkffgg
Copy link
Member

xrkffgg commented Jan 30, 2021

有冲突,rebase 下最新的

@afc163
Copy link
Member

afc163 commented Apr 25, 2021

Closing for no further progress, feel free to open new pull request: #26229 (comment)

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.

[Statistic.Countdown] Would be nice to have a pause/start api.
6 participants