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

scroll lag after opening and closing calendar (or any other overlay like autocomplete) #12341

Closed
fxck opened this issue Jul 24, 2018 · 21 comments · Fixed by #12414
Closed

scroll lag after opening and closing calendar (or any other overlay like autocomplete) #12341

fxck opened this issue Jul 24, 2018 · 21 comments · Fixed by #12414
Assignees
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@fxck
Copy link
Contributor

fxck commented Jul 24, 2018

See https://youtu.be/1VE5vHXnL84

It feels as if something is not properly cleaned up.

The popover component I'm using is https://github.com/ncstate-sat/popover and is internally using overlay as well.

Also worth noting is that it happens with or without dev tools open and nothing seems out of the ordinary in the performance and rendering tab.

I'll try to make a repro later today.

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

6.4.0 material, 6.0.9 angular

@fxck
Copy link
Contributor Author

fxck commented Jul 24, 2018

https://stackblitz.com/edit/material-scroll-lag-9qzbai?file=app%2Fsimple-example%2Fsimple-example.component.ts

could not replicate it, any ideas? I mean if it was for the sheer amount of components it contains, it wouldn't just start behaving like this after opening component with an overlay, right..

@crisbeto
Copy link
Member

Maybe this is happening as a result of a combination of components? It might be easier to see what's going on by doing a recording in the "Performance" tab.

@crisbeto
Copy link
Member

One more thing to try: once you've opened it a few times and it starts lagging, can you try finding the cdk-overlay-container in the DOM and adding a display: none to it, and seeing if the lag goes away?

@jelbourn jelbourn added the needs: clarification The issue does not contain enough information for the team to determine if it is a real bug label Jul 24, 2018
@fxck
Copy link
Contributor Author

fxck commented Jul 27, 2018

One more thing to try: once you've opened it a few times and it starts lagging, can you try finding the cdk-overlay-container in the DOM and adding a display: none to it, and seeing if the lag goes away?

Can't, it would hide the popover as well.

@crisbeto
Copy link
Member

It's a bit more work, but you should be able to hide the cdk-overlay-connected-position-bounding-box for the closed overlays.

@fxck
Copy link
Contributor Author

fxck commented Jul 27, 2018

Ok, that did fix the lag.

@crisbeto
Copy link
Member

Cool, thanks for checking. In that case we should move the detached overlays out of the DOM completely.

@fxck
Copy link
Contributor Author

fxck commented Jul 27, 2018

I still can't quite understand why is it happening, that cdk-overlay-connected-position-bounding-box seems harmless..

@crisbeto
Copy link
Member

It's because, even though they're transparent and unclickable, the elements are still being rendered over the page. Since they're position: fixed, they have to be redrawn while you're scrolling.

@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent has pr and removed needs: clarification The issue does not contain enough information for the team to determine if it is a real bug labels Jul 27, 2018
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 27, 2018
Removes the detached overlays from the DOM, rather than keeping their host element. Previously we kept the elements in the DOM, because they don't interfere with user interactions and it made it easier to handle animations, however doing so can cause scroll jank, because the host still has dimensions and is being rendered by the browser.

Fixes angular#12341.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 28, 2018
Removes the detached overlays from the DOM, rather than keeping their host element. Previously we kept the elements in the DOM, because they don't interfere with user interactions and it made it easier to handle animations, however doing so can cause scroll jank, because the host still has dimensions and is being rendered by the browser.

Fixes angular#12341.
jelbourn pushed a commit that referenced this issue Aug 1, 2018
Removes the detached overlays from the DOM, rather than keeping their host element. Previously we kept the elements in the DOM, because they don't interfere with user interactions and it made it easier to handle animations, however doing so can cause scroll jank, because the host still has dimensions and is being rendered by the browser.

Fixes #12341.
jelbourn pushed a commit that referenced this issue Aug 7, 2018
Removes the detached overlays from the DOM, rather than keeping their host element. Previously we kept the elements in the DOM, because they don't interfere with user interactions and it made it easier to handle animations, however doing so can cause scroll jank, because the host still has dimensions and is being rendered by the browser.

Fixes #12341.
jelbourn pushed a commit that referenced this issue Aug 7, 2018
Removes the detached overlays from the DOM, rather than keeping their host element. Previously we kept the elements in the DOM, because they don't interfere with user interactions and it made it easier to handle animations, however doing so can cause scroll jank, because the host still has dimensions and is being rendered by the browser.

Fixes #12341.
@fxck
Copy link
Contributor Author

fxck commented Sep 26, 2018

Was this really fixed? Experiencing the problem still using 6.4.7..

@fxck
Copy link
Contributor Author

fxck commented Sep 26, 2018

And it looks like cdk-overlay-container is still in DOM after closing the popover..

@crisbeto
Copy link
Member

The cdk-overlay-container is still in the DOM, but it's display: none so it won't have an effect on rendering.

@fxck
Copy link
Contributor Author

fxck commented Sep 26, 2018

But it does. Or something does. Nothing really changed since it was "fixed". See https://youtu.be/rvzkiTx4_Fk it's smooth the first time I open the popover, after I close it and open it again, it lags a lot..

@crisbeto
Copy link
Member

For what it's worth, that fix actually went into 7.0.0-beta.0, but judging by your video you're running Angular 6.1.8.

@fxck
Copy link
Contributor Author

fxck commented Sep 26, 2018

That's why I asked in the first place, looking the changelog it seemed like the fix was included in 6.4.3 monelite-meeple and I'm running "@angular/material": "6.4.7"..

image

@crisbeto
Copy link
Member

I'm not sure why it ended up in both versions, but an easy way to verify it is to open and close the overlay a couple of times, click around the page somewhere to trigger a round of change detection and see if the cdk-overlay-container is empty.

@fxck
Copy link
Contributor Author

fxck commented Sep 26, 2018

image

Also cdk-overlay-container is indeed display: none;.. but it doesn't fix the lag.. funnily enough it did, when I previously added it manually, as mentioned here #12341 (comment)

@crisbeto
Copy link
Member

Then it's most likely something different, but it's hard to say by looking at a video.

@fxck
Copy link
Contributor Author

fxck commented Sep 28, 2018

What can I do (to help you debug it)?

@crisbeto
Copy link
Member

Taking a performance recording while scrolling on a list that is laggy could help identify where it's coming from.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants