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

WIP fix resize observer bug #458

Closed
wants to merge 1 commit into from
Closed

Conversation

curran
Copy link
Contributor

@curran curran commented Oct 8, 2024

WIP towards #455

@curran
Copy link
Contributor Author

curran commented Oct 8, 2024

Current status: uncovered the fact that multiple resize observers are set up on the same element.

image

It looks like the constructor is invoked twice for the exact same element.

This could be related to the resize observer error, since each instance of the class assumes that it is the only instance associated with the given element.

@rokotyan
Copy link
Contributor

rokotyan commented Oct 8, 2024

It looks like the constructor is invoked twice for the exact same element.

This is probably due to React rendering components twice in the dev StrictMode, which is a known behavior and usually not an issue — Unovis container components should disconnect the observer when destroyed.

@curran
Copy link
Contributor Author

curran commented Oct 9, 2024

Ah yes, great point!

image

Confirmed - the resize observer gets removed on cleanup.

Maybe the error could be coming from somewhere else...

@curran
Copy link
Contributor Author

curran commented Oct 9, 2024

resizeError.mov

The error seems to trigger when the vertical scrollbar gets added or removed.

@rokotyan
Copy link
Contributor

I was able to reproduce this problem after setting "Show scroll bars" to "Always" in the macOS settings.

image

@rokotyan
Copy link
Contributor

#463 should fix this

@50rayn
Copy link
Contributor

50rayn commented Nov 25, 2024

Is this PR still actual ?

@rokotyan
Copy link
Contributor

@lee00678 I think we can close this one. I've fixed this issue in another PR.

@lee00678 lee00678 closed this Nov 26, 2024
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.

4 participants