Skip to content

Conversation

Maistho
Copy link
Contributor

@Maistho Maistho commented Jul 5, 2021

When the positioning of an overlay is calculated, the overlay pane is
moved to the top right corner, and given an unbounded size. Based on the
calculated size, a position is attempted where the entire element can
fit next to the origin element without needing to scroll.

If an overlay pane is created with a size large enough that it needs
scrolling in any valid position (top/bottom/left/right) but small enough
that it can fit in the viewport without needing to scroll, the scrolling
of that pane will break. When the pane is moved to the top left corner
for size calculation, the pane will lose it's scroll position since it
no longer has a scrollbar.

This fix introduces a maximum constraint on the size of the overlay pane
during size calculation, where the maximum available vertical and
horizontal space is used as the constraint.

This is a quick approach and there are probably some things I've missed.
I haven't written any tests for this fix, and would like some help regarding
how to best write those tests, as well as fix two tests that broke because
of these changes.


Edit: Added a stackblitz with a reproduction

https://stackblitz.com/edit/components-issue-yusmdo?file=src/app/example-component.html

Open the 75vh overlay, scroll a bit in it and then scroll the body. The scroll in the overlay will reset to the top.

Open the 125vh overlay, scroll a bit in it and then scroll the body. The scroll in the overlay will not reset.

When the positioning of an overlay is calculated, the overlay pane is
moved to the top right corner, and given an unbounded size. Based on the
calculated size, a position is attempted where the entire element can
fit next to the origin element without needing to scroll.

If an overlay pane is created with a size large enough that it needs
scrolling in any valid position (top/bottom/left/right) but small enough
that it can fit in the viewport without needing to scroll, the scrolling
of that pane will break. When the pane is moved to the top left corner
for size calculation, the pane will lose it's scroll position since it
no longer has a scrollbar.

This fix introduces a maximum constraint on the size of the overlay pane
during size calculation, where the maximum available vertical and
horizontal space is used as the constraint.
@Maistho Maistho requested review from crisbeto and jelbourn as code owners July 5, 2021 15:04
@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jul 5, 2021
@crisbeto
Copy link
Member

crisbeto commented Jul 6, 2021

Is there an example of the issue that this fix is addressing?

@Maistho
Copy link
Contributor Author

Maistho commented Jul 6, 2021

Yes, sorry should've added a stackblitz right away

https://stackblitz.com/edit/components-issue-yusmdo?file=src/app/example-component.html

Open the 75vh overlay, scroll a bit in it and then scroll the body. The scroll in the overlay will reset to the top.

Open the 125vh overlay, scroll a bit in it and then scroll the body. The scroll in the overlay will not reset.

@andrewseguin andrewseguin removed the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 28, 2021
@josephperrott josephperrott added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed needs rebase labels Nov 16, 2022
@josephperrott josephperrott requested a review from a team as a code owner December 18, 2024 17:40
@wagnermaciel
Copy link
Contributor

Looking into this, there are a lot of failures internally that need to be debugged. It seems like landing this PR would require a lot of attention which we can't afford at this time. If anyone still wants to see this issue resolved, please file a bug.

@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 Feb 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews area: cdk/overlay

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants