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

don't use contentVisibility on Firefox #4164

Merged
merged 1 commit into from
May 22, 2024

Conversation

haileyok
Copy link
Contributor

Why

The content-visibility style on Firefox is causing problems with the List implementation - at least for the list we use for chat conversations.

This PR just checks the useragent for Firefox and does not use this style - the same way we do not use it for Safari - if it matches.

Test Plan

With this PR, the chat list should be fine on Firefox.

Screen.Recording.2024-05-21.at.11.40.56.PM.mov

Copy link

render bot commented May 22, 2024

@haileyok
Copy link
Contributor Author

@gaearon I mentioned in Slack that I'm not certain what we're using this style for. It isn't (visibly anyway?) causing problems elsewhere in the app that we know of, so if it is useful for some reason we can scope this down further, maybe with a prop on List.

@haileyok haileyok requested a review from gaearon May 22, 2024 06:44
Copy link

Old size New size Diff
6.87 MB 6.87 MB 50 B (0.00%)

@mary-ext
Copy link
Contributor

mary-ext commented May 22, 2024

content-visibility is a rendering optimization for skipping work on the element entirely when the browser thinks it isn't visible.

It's guaranteed to ruin layout/scroll animations, because unless the browser knows it should render it, then the elements are assumed to have no height/width initially, which is what I presume is happening.

It can be remedied with contain-intrinsic-size which tells the browser the size it should assume, but I'd honestly recommend just turning it off for chat entirely in this case (on all browsers), since I don't imagine the users are going to do a lot of back and forward navigation from chat screen.

@haileyok
Copy link
Contributor Author

Ah that does make sense. Kind of wonder if this is what causes the ghost scroll position issues for some people (the content flashing up and down the screen). We still get reports of that occasionally though have never been able to reproduce.

@mary-ext
Copy link
Contributor

mary-ext commented May 22, 2024

Hmm, it might be, and not sure if it's something that can be remedied easily too.

There is contain-intrinsic-size: auto which will make the element retain its previously-known size once it goes offscreen, but unfortunately that size measurement gets lost when it gets detached from DOM.

Copy link
Member

@mozzius mozzius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works like a charm

@gaearon gaearon merged commit 6522ee9 into main May 22, 2024
6 checks passed
@gaearon gaearon deleted the hailey/firefox-no-use-contentVisibility branch May 22, 2024 13:07
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