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 resize-observer-lite fork which uses different strategy #4761

Conversation

larsjohnsen
Copy link
Collaborator

@larsjohnsen larsjohnsen commented May 4, 2018

The scroll strategy which was used does not work when animations are globally disabled, causing media to not flow properly. The other available strategy, object, does not have this issue.

Replaces #4759

@erikdesjardins Can you fork the repo larsjohnsen/resize-observer-lite to the organization, like is done with jquery.tokeninput?

@erikdesjardins
Copy link
Collaborator

Done. Everyone in the org should have write access to https://github.com/Reddit-Enhancement-Suite/resize-observer-lite

Another potential option is to change the disableAnimations CSS to avoid applying to the relevant media elements; maybe you've already considered that and discarded it?

@larsjohnsen
Copy link
Collaborator Author

larsjohnsen commented May 4, 2018

Thanks.

I haven't really explored adding an expectation to the disableAnimations CSS as I am satisfied with just switching the resize detection strategy; there doesn't appear to be any significant performance difference between the two.

@larsjohnsen larsjohnsen force-pushed the resize-observer-lite-fork-object branch from bfbc84f to cee050b Compare May 4, 2018 22:01
@erikdesjardins
Copy link
Collaborator

Sounds good to me.

@erikdesjardins erikdesjardins merged commit 6b6308e into honestbleeps:master May 4, 2018
@larsjohnsen larsjohnsen deleted the resize-observer-lite-fork-object branch August 28, 2018 14:21
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.

2 participants