Modernize countdown component #9734
Conversation
lukasmasuch
added
security-assessment-completed
| public static defaultProps: Partial<Props> = { | ||
| endCallback: () => {}, | ||
| } | ||
| const Countdown: React.FC<Props> = ({ countdown, endCallback = () => {} }) => { |
There was a problem hiding this comment.
nit: instead of the defining a default function that does not do anything, we could just have it undefined and check for it here
There was a problem hiding this comment.
I made it a required prop again which should solve this as well.
| const { countdown } = this.state | ||
| const { endCallback } = this.props | ||
| const newCountdown = countdown - 1 | ||
| const onAnimationEnd = (): void => { |
There was a problem hiding this comment.
Do you know why onAnimationEnd was an async function before?
There was a problem hiding this comment.
Nope, not sure. But I changed it back to async.
There was a problem hiding this comment.
I have just had a look at the MDN web docs and it does not mention asynchronicity: https://developer.mozilla.org/en-US/docs/Web/API/Element/animationend_event
So in this case we should probably remove it to avoid confusion
There was a problem hiding this comment.
Oki, I removed it again 👍
## Describe your changes Migrate the countdown component from class-based to functional. ## Testing Plan - No logical changes, no tests required. --- **Contribution License Agreement** By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license. --------- Co-authored-by: Lukas Masuch <[email protected]>
Describe your changes
Migrate the countdown component from class-based to functional.
Testing Plan
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.