Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

fix(dialog): set md-dialog-container css position attribute to fixed #2631

Closed
wants to merge 2 commits into from
Closed

fix(dialog): set md-dialog-container css position attribute to fixed #2631

wants to merge 2 commits into from

Conversation

Kiougar
Copy link
Contributor

@Kiougar Kiougar commented Apr 30, 2015

This fixes the #1967. Example 1 (comment the first css line to see the problem)

Also, it fixes another issue with the sidenav toggle where the backdrop element caused the body to overflow. Example 2 (comment the second css line to see the problem)

Kiougar added 2 commits April 30, 2015 15:00
fix(dialog): set `md-dialog-container` css `position` attribute to
`fixed`

`position: fixed` ensures the dialog will always be center in the
viewport even when scrolling
fix(backdrop): set `md-backdrop` css `position` attribute to `fixed

`position: fixed` ensures the backdrop element will always cover the
entire viewport
@Kiougar Kiougar changed the title Backdrop dialog fix (fixes #1967) fix(dialog): set md-dialog-container css position attribute to fixed May 2, 2015
@Kiougar Kiougar closed this May 2, 2015
@Kiougar
Copy link
Contributor Author

Kiougar commented May 2, 2015

Will remake the pull request when I fix the commit messages.

@ThomasBurleson
Copy link
Contributor

@Kiougar - whenever I see CSS changes to use 'position: fixed' I worry about sideaffects.
@rschmukler - what are your thoughts on this PR?

@Kiougar
Copy link
Contributor Author

Kiougar commented May 2, 2015

@ThomasBurleson The second example I provided can be solved by setting top: 0 instead of position: fixed. But that won't solve the #1967 issue.
Another solution for the #1967 could be to manually update the top attribute of both the .md-dialog-container and md-backdrop element when a user scrolls vertically.

@Kiougar
Copy link
Contributor Author

Kiougar commented May 6, 2015

@ThomasBurleson, @rschmukler Reopened in #2754. We can discuss there if there are any side effects.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants