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

bind:offsetHeight doesn't update in certain cases #4233

Open
plmrry opened this issue Jan 9, 2020 · 7 comments
Open

bind:offsetHeight doesn't update in certain cases #4233

plmrry opened this issue Jan 9, 2020 · 7 comments
Labels
bug good first issue

Comments

@plmrry
Copy link
Contributor

@plmrry plmrry commented Jan 9, 2020

Describe the bug
In Chrome, using bind:offsetHeight on a component that immediately changes size does not trigger a change in the offsetHeight variable:

https://codesandbox.io/s/upbeat-hill-ilkqt?fontsize=14&hidenavigation=1&module=%2FApp.svelte&theme=dark

This does not seem to happen in Firefox or Safari, but it does happen in Chrome.

To Reproduce
https://codesandbox.io/s/upbeat-hill-ilkqt?fontsize=14&hidenavigation=1&module=%2FApp.svelte&theme=dark

Expected behavior
offsetHeight should be something close to 800 (in the example above)

Notes
Is there a reason Svelte doesn't attempt to use a ResizeObserver for resize detection?

@Conduitry Conduitry added the bug label Jan 30, 2020
@Conduitry
Copy link
Member

@Conduitry Conduitry commented Jan 30, 2020

When this was first implemented in #1386, ResizeObserver had just come out from behind a flag in Chrome and wasn't available anywhere else yet. It'd probably make sense to use that now though when possible.

@arxpoetica
Copy link
Member

@arxpoetica arxpoetica commented Feb 8, 2020

@Conduitry would you be worried about the lack of Safari support here: https://caniuse.com/#search=ResizeObserver (esp. iOS)

@Conduitry
Copy link
Member

@Conduitry Conduitry commented Feb 9, 2020

I wasn't suggesting that we drop the existing resizing mechanism, just that ResizeObserver is widespread enough that it'd be worth it to use that when possible.

@Prinzhorn
Copy link
Contributor

@Prinzhorn Prinzhorn commented Mar 16, 2020

When ResizeObserver gets implemented in one way or another I'd love to see a compiler option with three possible options:

  1. Default: use ResizeObserver where possible, fall back to the current solution in other browsers
  2. Only use ResizeObserver
  3. Only use the current behavior

This would allow predictable results for people that need them. For example in an Electron application we know that ResizeObserver is supported and we don't need to ship the fallback at all. On the other hand, for people that run into issues with the new ResizeObserver implementation they can enable the legacy behavior in every browser.

@jacwright
Copy link
Contributor

@jacwright jacwright commented Nov 12, 2020

ResizeObserver is looking pretty good now. https://caniuse.com/#search=ResizeObserver

Probably still want to keep the fallback for older browsers, but it would be worth adding it now as most browsers support it.

This would be an easy ticket for anyone interested in contributing to core. Just one runtime method to update, add_resize_listener:

https://github.com/sveltejs/svelte/blob/master/src/runtime/internal/dom.ts#L262

@arxpoetica arxpoetica added the good first issue label Nov 12, 2020
@iamricard
Copy link

@iamricard iamricard commented Jan 14, 2021

I'd like to take this, if nobody's working on it!

iamricard added a commit to iamricard/svelte that referenced this issue Jan 15, 2021
As discussed in sveltejs#4233, ResizeObserver is now widely available (>90% as
of 15/01/2021) so we can use it instead of a custom-built solution to
listen to resizes.

I also needed to add @types/resize-observer-browser because the type
definition for `ResizeObserver` hasn't landed in TS yet (see
microsoft/TypeScript#28502).

Closes sveltejs#4233.
iamricard added a commit to iamricard/svelte that referenced this issue Jan 15, 2021
As discussed in sveltejs#4233, ResizeObserver is now widely available (~90% as
of 15/01/2021, [source](http://caniuse.com/?search=ResizeObserver)) so
we can use it instead of a custom-built solution to listen to resizes.

I also needed to add @types/resize-observer-browser because the type
definition for `ResizeObserver` hasn't landed in TS yet (see
microsoft/TypeScript#28502).

Closes sveltejs#4233.
@stale
Copy link

@stale stale bot commented Jun 26, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

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

Successfully merging a pull request may close this issue.

9 participants