Skip to content

Conversation

@clxandstuff
Copy link
Contributor

@clxandstuff clxandstuff commented May 11, 2023

I created a new hook useFirstPaint to replace useIsFirstRender. It checks whether the component render function has triggered the first paint of DOM.

It has been used in Checkbox and Radio to prevent animation on the initial render and to replace the isPristine flag.

useIsFirstRender rewritten and used in Checkbox and Radio
@clxandstuff clxandstuff requested a review from a team as a code owner May 11, 2023 07:33
isFirstRender.current = false;
return true;
}
React.useLayoutEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

hey @clxandstuff , we need isomorphic version here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@coderitual
Copy link
Contributor

coderitual commented May 11, 2023

i see that you're still using state to communicate about first render (inside first render hook). I think this may cause some unexpected side effects. Let's take a look into tests.

@clxandstuff
Copy link
Contributor Author

@coderitual

Take a look now. I removed states related to animation from isFirstRender, Checkbox, and Radio.

@coderitual
Copy link
Contributor

coderitual commented May 12, 2023

thx @clxandstuff , could you please add separate story documentation for is first render hook specifically

@clxandstuff clxandstuff requested a review from coderitual May 15, 2023 05:28
@clxandstuff clxandstuff changed the title new useIsFirstRender use useFirstPaint to control animation in Checkbox and Radio May 15, 2023
@coderitual
Copy link
Contributor

hey @clxandstuff I see 2 problems with the example below:

  • using ref directly in render (we can discuss)
  • is naming of this ref correct?

CleanShot 2023-05-17 at 11 16 59

@clxandstuff clxandstuff enabled auto-merge (squash) May 19, 2023 08:24
@clxandstuff clxandstuff merged commit 2bbc791 into master May 19, 2023
@clxandstuff clxandstuff deleted the DS-5347-radio-animation-is-not-fired branch May 19, 2023 08:38
@domidomi domidomi mentioned this pull request Jun 5, 2023
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.

3 participants