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

Core | Container: Improve ResizeObserver handling #463

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

rokotyan
Copy link
Contributor

This PR fixes #455, a problem with ResizeObserver throwing an error when scrollbars appear/disappear. Solving this by delaying the resize by one frame.

The only tradeoff is scrollbar flickering when changing the size, but it only appears when "Show scroll bars" setting is set to "Always" on macOS (not sure about the other platforms).

image
Screen.Recording.2024-10-11.at.3.18.15.PM.mp4

Changelog:

  • Use requestAnimationFrame to avoid multiple resize events when scrollbars appear/disappear
  • Cancel previous animation frame IDs before scheduling new ones
  • Renamed _requestedAnimationFrame to _renderAnimationFrameId

- Use requestAnimationFrame to avoid multiple resize events when scrollbars appear/disappear
- Cancel previous animation frame IDs before scheduling new ones
- Renamed `_requestedAnimationFrame` to `_renderAnimationFrameId`

Fixes f5#455
@lee00678
Copy link
Collaborator

@rokotyan how so I test this? I removed height={400} after a npm run install:clean. Still not resizing for me with vertical changes.

Screen.Recording.2024-10-21.at.3.33.57.AM.mov

- Adds a new example for a full height donut chart
- Tests the resize behavior of the donut chart component

f5#455
@rokotyan
Copy link
Contributor Author

@lee00678 I've pushed a new example for this

@lee00678
Copy link
Collaborator

@lee00678 I've pushed a new example for this

Thanks for the clarification.

@lee00678 lee00678 merged commit 42b5f28 into f5:main Oct 21, 2024
4 checks passed
@curran
Copy link

curran commented Oct 22, 2024

Hooray!

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.

Resize Observer Error on Vertical Resize
3 participants