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

Align useResizeObserver with EuiResizeObserver component behavior #7458

Closed
drewdaemon opened this issue Jan 11, 2024 · 3 comments · Fixed by #7518
Closed

Align useResizeObserver with EuiResizeObserver component behavior #7458

drewdaemon opened this issue Jan 11, 2024 · 3 comments · Fixed by #7518
Labels
feature request good first issue low hanging fruit An issue, often a bug, that is lower effort and clearly ought to be fixed

Comments

@drewdaemon
Copy link

Describe the bug
I'm not sure if this is a bug, but the discrepancy can be seen on the docs page.

Both EuiResizeObserver and useResizeObserver give initial container dimensions, but they are always [0, 0] for useResizeObserver.

Screenshot 2024-01-11 at 9 38 30 AM

correct initial dimensions

Screenshot 2024-01-11 at 9 37 59 AM

incorrect initial dimensions

Impact and severity
This isn't directly impacting an end user. There is a workaround—get the initial size from the ref in a useEffect hook. It works, but it would be much nicer to just have to worry about a single hook: useResizeObserver.

Environment and versions

  • EUI version: v92.0.0, probably much older
  • React version: 11.11
  • Browser: Brave
  • Operating System: OSX

To Reproduce
Look at the initial values show in the docs page.

Expected behavior
As soon as the ref is populated, useResizeObserver should return its initial dimensions.

@drewdaemon drewdaemon added bug ⚠️ needs validation For bugs that need confirmation as to whether they're reproducible labels Jan 11, 2024
@JasonStoltz JasonStoltz added feature request low hanging fruit An issue, often a bug, that is lower effort and clearly ought to be fixed good first issue and removed bug ⚠️ needs validation For bugs that need confirmation as to whether they're reproducible labels Jan 29, 2024
@JasonStoltz
Copy link
Member

Thanks for raising this issue. I changed the label to "feature request" since it has been this way for quite some time.

@cee-chen
Copy link
Contributor

cee-chen commented Feb 8, 2024

Hey @drewdaemon! After talking through the fix for this with @Juneezee, we realized that the issue/confusion is actually in our docs example. For useResizeObserver to correctly update on mount, consumers need to store their DOM ref in a useState (in order to trigger a rerender) and not in a useRef.

We've updated our docs example accordingly and added an inline comment to note that. We've left the source code alone to reduce breaking changes, and also to allow for devs who intentionally want useRef over useState (they may want to receive updates on resize event change only, and not on mount).

@drewdaemon
Copy link
Author

@cee-chen awesome. Thanks for looking into this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request good first issue low hanging fruit An issue, often a bug, that is lower effort and clearly ought to be fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants