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

fix(colorpicker): prevent infinite rerender in React v18 Legacy mode #1616

Merged
merged 1 commit into from
Aug 31, 2023

Conversation

tgohn
Copy link
Contributor

@tgohn tgohn commented Aug 31, 2023

Description

With React v18 - Legacy mode (mounting with ReactDOM.render()), there is a chance that colorpicker will rerender infinitely when a new color is picked via ColorWell.

Reproducible:
https://codesandbox.io/s/quizzical-franklin-fxq9zy?file=/src/Example.tsx

Detail

We fix this by updating local state when color props changed in the same rendering cycle, instead of using useEffect.
Docs:
https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes

These changes can be tested via this sandbox https://codesandbox.io/s/garden-color-picker-new-lgm7s7:

  • this is the fork of https://codesandbox.io/s/quizzical-franklin-fxq9zy sandbox above
  • I updated the sandbox with some changes:
    • the new src/new-garden-color-picker.js file is copied from packages/colorpickers/dist/index.esm.js , which is built from this branch
    • import ColorPicker from the above file instead of @zendeskgarden/react-colorpickers

Checklist

  • πŸ‘Œ design updates will be Garden Designer approved (add the designer as a reviewer)
  • 🌐 demo is up-to-date (yarn start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • 🀘 renders as expected with Bedrock CSS (?bedrock)
  • πŸ’‚β€β™‚οΈ includes new unit tests. Maintain existing coverage (always >= 96%)
  • β™Ώ tested for WCAG 2.1 AA accessibility compliance
  • πŸ“ tested in Chrome, Firefox, Safari, and Edge

With React v18 - Legacy mode (mounting with `ReactDOM.render()`), there
is a chance that colorpicker will rerender infintely when a new color is
picked via ColorWell.

We fix this by updating local state when `color` props changed in the
same rendering cycle, instead of using `useEffect`.
Docs:
https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes

Reproducible:
https://codesandbox.io/s/quizzical-franklin-fxq9zy?file=/src/Example.tsx
@tgohn tgohn requested a review from a team as a code owner August 31, 2023 03:26
@coveralls
Copy link

Coverage Status

coverage: 95.476% (-0.005%) from 95.481% when pulling 5ac2fc9 on john/color-picker into 08af7bb on main.

Copy link
Member

@jzempel jzempel left a comment

Choose a reason for hiding this comment

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

thanks for this fix, @tgohn πŸ™Œ

@jzempel jzempel merged commit b7280ed into main Aug 31, 2023
6 checks passed
@jzempel jzempel deleted the john/color-picker branch August 31, 2023 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants