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

Use ResizeObserver instead of iframes if available #5524

Closed
wants to merge 3 commits into from

Conversation

pushkine
Copy link
Contributor

@pushkine pushkine commented Oct 13, 2020

@pushkine pushkine changed the title Use ResizeListener instead of iframes if available Use ResizeObserver instead of iframes if available Oct 13, 2020
@Rich-Harris
Copy link
Member

This is very cool, but I don't think we can switch between ResizeObserver and iframes for the same binding — you could much too easily fall into a situation where something works when you test it locally in Chrome, but doesn't work for all of your users (because you've used it on an element that can't contain an iframe, like an inline-level element or an SVG element).

Instead, perhaps we should add a new set of bindings...

<div bind:contentRect bind:contentBoxSize bind:borderBoxSize>...</div>

...which are documented with the caveat that they don't work in IE11 etc.

@swyxio
Copy link
Contributor

swyxio commented Nov 15, 2020

has anyone suggested adding an ie11 compat toggle to svelte:options yet?

@j3rem1e
Copy link

j3rem1e commented Jan 23, 2021

why is this PR closed ?

@pushkine
Copy link
Contributor Author

why is this PR closed ?

I'm sorry I was deleting some of my svelte branches, I assumed those linked to an open PR would prompt a confirm dialog

@pushkine pushkine restored the patch-2 branch January 24, 2021 06:25
@pushkine pushkine reopened this Jan 24, 2021
@jacwright
Copy link
Contributor

@pushkine are you interested in updating this PR for the contentRect etc. bindings as suggested by Rich?

jacwright added a commit to jacwright/svelte that referenced this pull request Feb 4, 2021
Implements ResizerObserver per @Rich-Harris sveltejs#5524 (comment)

Done poorly, looking for guidance on doing this correctly or for someone else to take it on.
@bluwy
Copy link
Member

bluwy commented Aug 8, 2021

Would love to has this merge too. I've recently encountered a performance issue when rendering large arrays of elements that uses bind:clientWidth (and friends). The iframes mounting and unmounting heavily lags the page.

Re: ResizeObserver types
The latest TypeScript dom lib should have the types now.

Re: Can't use iframes in some elements and bind:contentRect
I'd prefer keeping the bind:clientWidth and bind:clientHeight names, and instead check during runtime for these edge cases to warn users beforehand (dev: true only). Or we could point to using a polyfill instead (we're technically polyfilling it anyways)

FWIW ResizeObserver now has 92% global usage

zqianem added a commit to byrd-polar/fluid-earth that referenced this pull request Aug 31, 2021
Fixes an issue where gridded legend scale would be the wrong size on
initial load on Firefox when screen width was ~900px.

Also improves performance a little bit, in theory.

Based on sveltejs/svelte#5524 which hasn't
merged upstream because of older browser support that we don't have to
worry about (since some of the WebGL stuff requires a somewhat recent
browser anyways).
@tanhauhau
Copy link
Member

See #5524 (comment)

Closed in favour of #5963

@tanhauhau tanhauhau closed this Jan 8, 2022
RaiVaibhav pushed a commit to RaiVaibhav/svelte that referenced this pull request Nov 1, 2022
Implements ResizerObserver per @Rich-Harris sveltejs#5524 (comment)

Done poorly, looking for guidance on doing this correctly or for someone else to take it on.
dummdidumm added a commit that referenced this pull request Apr 11, 2023
Implements ResizeObserver bindings: #5524 (comment)
Continuation of: #5963
Related to #7583

---------

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
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.

None yet

7 participants