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: Remove react resize detector #2912

Merged

Conversation

lee-chase
Copy link
Member

Contributes to #2744

V7 of ReactResizeObserver can result in the error shown in #2744 but the error appears to be a timing issue of some description.

The author fixed a similar looking issue in V8, but it persists in our storybook. In V9 there were more extensive changes that stopped our current usage working.

This PR removes ReactResizeObserver in favor of wrapping ResizeObserver (now standard) in a hook.

What did you change?

  • Removed react-resize-detector
  • Created useResizeObserver hook.
  • Checked it functioned as a replacement for all useResizeDetector hook

How did you test and verify your work?

  • Storybook changing the size of things and adding new elements
  • Ran tests

NOTE: Storybook 6 reports the following error which would appear to be related this issue fixed only in Storybook 7. storybookjs/storybook#18716

image

@lee-chase lee-chase requested a review from a team as a code owner April 21, 2023 17:06
@lee-chase lee-chase changed the base branch from main to carbon-v11 April 21, 2023 17:06
@netlify
Copy link

netlify bot commented Apr 21, 2023

Deploy Preview for carbon-for-ibm-products ready!

Name Link
🔨 Latest commit 7dd4d9a
🔍 Latest deploy log https://app.netlify.com/sites/carbon-for-ibm-products/deploys/6442c28e1740520007dcfa87
😎 Deploy Preview https://deploy-preview-2912--carbon-for-ibm-products.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@lee-chase lee-chase changed the title Remove react resize detector reat: Remove react resize detector Apr 21, 2023
@davidmenendez davidmenendez changed the title reat: Remove react resize detector feat: Remove react resize detector Apr 21, 2023
@netlify
Copy link

netlify bot commented Apr 25, 2023

Deploy Preview for v11-carbon-for-ibm-products ready!

Name Link
🔨 Latest commit 518d218
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-for-ibm-products/deploys/6447b65a84fc6c000816936e
😎 Deploy Preview https://deploy-preview-2912--v11-carbon-for-ibm-products.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Apr 25, 2023

Deploy Preview for v11-carbon-for-ibm-products ready!

Name Link
🔨 Latest commit e7fb955
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-for-ibm-products/deploys/6447d31c7104ac0008b2c1f1
😎 Deploy Preview https://deploy-preview-2912--v11-carbon-for-ibm-products.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@lee-chase lee-chase requested a review from matthewgallo April 25, 2023 12:00
Copy link
Member

@matthewgallo matthewgallo left a comment

Choose a reason for hiding this comment

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

👏 looks great! And nice to have one less dependency.

@kodiakhq kodiakhq bot merged commit 42f6d0a into carbon-design-system:carbon-v11 Apr 25, 2023
@lee-chase lee-chase deleted the removeReactResizeDetector branch May 2, 2023 08:54
@tonydiaz
Copy link

tonydiaz commented May 2, 2023

@lee-chase Can this also be removed in the main branch? I seeing this issue on @carbon/[email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants