-
Notifications
You must be signed in to change notification settings - Fork 6.8k
feat(datepicker): popup positioning improvements #4696
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
feat(datepicker): popup positioning improvements #4696
Conversation
* Adds fallback positions to the datepicker popup, allowing it to reposition in the cases where it would've gone outside the viewport. * Fixes an issue that prevented the datepicker from the being positioned properly on the first open due to the calendar not being rendered yet. * Adds a scroll strategy to reposition the datepicker when the user scrolls away. * Switches to using `ngSwitch` instead of `ngIf` when determining which view to render. Fixes angular#4406.
@@ -16,17 +16,17 @@ | |||
</div> | |||
|
|||
<div class="mat-calendar-content" (keydown)="_handleCalendarBodyKeydown($event)" | |||
cdkMonitorSubtreeFocus> | |||
[ngSwitch]="_monthView" cdkMonitorSubtreeFocus> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did we switch to ngSwitch
(I'm fine with it, just curious) is it just a style thing to avoid repeating the condition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just one less watcher in the template. It'll also be simpler to change if we add more views.
fixture.detectChanges(); | ||
testComponent = fixture.componentInstance; | ||
input = fixture.debugElement.query(By.css('input')).nativeElement; | ||
input.style.position = 'fixed'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nifty way of testing this, I like it :)
src/lib/datepicker/datepicker.ts
Outdated
} from '../core/overlay/position/connected-position'; | ||
OverlayConnectionPosition, | ||
RepositionScrollStrategy, | ||
BlockScrollStrategy, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like BlockScrollStrategy
isn't needed, also it seems like the reposition isn't working? (if I give the last input on the datepicker demo a huge top margin, it still seems to block scroll)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The demo page actually ends up blocking scroll whenever a connected overlay is open, because the scrollable container is the md-sidenav-container
. I usually test it by tweaking the demo-app.html
.
Removed the unused imports @mmalerba. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
ngSwitch
instead ofngIf
when determining which view to render.Fixes #4406.