-
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -509,6 +509,82 @@ describe('MdDatepicker', () => { | |
.toThrowError(/MdDatepicker: No provider found for .*/); | ||
}); | ||
}); | ||
|
||
describe('popup positioning', () => { | ||
let fixture: ComponentFixture<StandardDatepicker>; | ||
let testComponent: StandardDatepicker; | ||
let input: HTMLElement; | ||
|
||
beforeEach(async(() => { | ||
TestBed.configureTestingModule({ | ||
imports: [MdDatepickerModule, MdInputModule, MdNativeDateModule, NoopAnimationsModule], | ||
declarations: [StandardDatepicker], | ||
}).compileComponents(); | ||
|
||
fixture = TestBed.createComponent(StandardDatepicker); | ||
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 commentThe reason will be displayed to describe this comment to others. Learn more. nifty way of testing this, I like it :) |
||
})); | ||
|
||
it('should be below and to the right when there is plenty of space', () => { | ||
input.style.top = input.style.left = '20px'; | ||
testComponent.datepicker.open(); | ||
fixture.detectChanges(); | ||
|
||
const overlayRect = document.querySelector('.cdk-overlay-pane').getBoundingClientRect(); | ||
const inputRect = input.getBoundingClientRect(); | ||
|
||
expect(Math.floor(overlayRect.top)) | ||
.toBe(Math.floor(inputRect.bottom), 'Expected popup to align to input bottom.'); | ||
expect(Math.floor(overlayRect.left)) | ||
.toBe(Math.floor(inputRect.left), 'Expected popup to align to input left.'); | ||
}); | ||
|
||
it('should be above and to the right when there is no space below', () => { | ||
input.style.bottom = input.style.left = '20px'; | ||
testComponent.datepicker.open(); | ||
fixture.detectChanges(); | ||
|
||
const overlayRect = document.querySelector('.cdk-overlay-pane').getBoundingClientRect(); | ||
const inputRect = input.getBoundingClientRect(); | ||
|
||
expect(Math.floor(overlayRect.bottom)) | ||
.toBe(Math.floor(inputRect.top), 'Expected popup to align to input top.'); | ||
expect(Math.floor(overlayRect.left)) | ||
.toBe(Math.floor(inputRect.left), 'Expected popup to align to input left.'); | ||
}); | ||
|
||
it('should be below and to the left when there is no space on the right', () => { | ||
input.style.top = input.style.right = '20px'; | ||
testComponent.datepicker.open(); | ||
fixture.detectChanges(); | ||
|
||
const overlayRect = document.querySelector('.cdk-overlay-pane').getBoundingClientRect(); | ||
const inputRect = input.getBoundingClientRect(); | ||
|
||
expect(Math.floor(overlayRect.top)) | ||
.toBe(Math.floor(inputRect.bottom), 'Expected popup to align to input bottom.'); | ||
expect(Math.floor(overlayRect.right)) | ||
.toBe(Math.floor(inputRect.right), 'Expected popup to align to input right.'); | ||
}); | ||
|
||
it('should be above and to the left when there is no space on the bottom', () => { | ||
input.style.bottom = input.style.right = '20px'; | ||
testComponent.datepicker.open(); | ||
fixture.detectChanges(); | ||
|
||
const overlayRect = document.querySelector('.cdk-overlay-pane').getBoundingClientRect(); | ||
const inputRect = input.getBoundingClientRect(); | ||
|
||
expect(Math.floor(overlayRect.bottom)) | ||
.toBe(Math.floor(inputRect.top), 'Expected popup to align to input top.'); | ||
expect(Math.floor(overlayRect.right)) | ||
.toBe(Math.floor(inputRect.right), 'Expected popup to align to input right.'); | ||
}); | ||
|
||
}); | ||
}); | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,8 @@ import { | |
Output, | ||
ViewChild, | ||
ViewContainerRef, | ||
ViewEncapsulation | ||
ViewEncapsulation, | ||
NgZone, | ||
} from '@angular/core'; | ||
import {Overlay} from '../core/overlay/overlay'; | ||
import {OverlayRef} from '../core/overlay/overlay-ref'; | ||
|
@@ -22,16 +23,19 @@ import {MdDialogRef} from '../dialog/dialog-ref'; | |
import {PositionStrategy} from '../core/overlay/position/position-strategy'; | ||
import { | ||
OriginConnectionPosition, | ||
OverlayConnectionPosition | ||
} from '../core/overlay/position/connected-position'; | ||
OverlayConnectionPosition, | ||
RepositionScrollStrategy, | ||
BlockScrollStrategy, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looks like There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
ScrollDispatcher, | ||
} from '../core/overlay/index'; | ||
import {MdDatepickerInput} from './datepicker-input'; | ||
import 'rxjs/add/operator/first'; | ||
import {Subscription} from 'rxjs/Subscription'; | ||
import {MdDialogConfig} from '../dialog/dialog-config'; | ||
import {DateAdapter} from '../core/datetime/index'; | ||
import {createMissingDateImplError} from './datepicker-errors'; | ||
import {ESCAPE} from '../core/keyboard/keycodes'; | ||
import {MdCalendar} from './calendar'; | ||
import 'rxjs/add/operator/first'; | ||
|
||
|
||
/** Used to generate a unique ID for each datepicker instance. */ | ||
|
@@ -155,8 +159,11 @@ export class MdDatepicker<D> implements OnDestroy { | |
|
||
private _inputSubscription: Subscription; | ||
|
||
constructor(private _dialog: MdDialog, private _overlay: Overlay, | ||
constructor(private _dialog: MdDialog, | ||
private _overlay: Overlay, | ||
private _ngZone: NgZone, | ||
private _viewContainerRef: ViewContainerRef, | ||
private _scrollDispatcher: ScrollDispatcher, | ||
@Optional() private _dateAdapter: DateAdapter<D>, | ||
@Optional() private _dir: Dir) { | ||
if (!this._dateAdapter) { | ||
|
@@ -253,6 +260,9 @@ export class MdDatepicker<D> implements OnDestroy { | |
let componentRef: ComponentRef<MdDatepickerContent<D>> = | ||
this._popupRef.attach(this._calendarPortal); | ||
componentRef.instance.datepicker = this; | ||
|
||
// Update the position once the calendar has rendered. | ||
this._ngZone.onStable.first().subscribe(() => this._popupRef.updatePosition()); | ||
} | ||
|
||
this._popupRef.backdropClick().first().subscribe(() => this.close()); | ||
|
@@ -265,15 +275,29 @@ export class MdDatepicker<D> implements OnDestroy { | |
overlayState.hasBackdrop = true; | ||
overlayState.backdropClass = 'md-overlay-transparent-backdrop'; | ||
overlayState.direction = this._dir ? this._dir.value : 'ltr'; | ||
overlayState.scrollStrategy = new RepositionScrollStrategy(this._scrollDispatcher); | ||
|
||
this._popupRef = this._overlay.create(overlayState); | ||
} | ||
|
||
/** Create the popup PositionStrategy. */ | ||
private _createPopupPositionStrategy(): PositionStrategy { | ||
let origin = {originX: 'start', originY: 'bottom'} as OriginConnectionPosition; | ||
let overlay = {overlayX: 'start', overlayY: 'top'} as OverlayConnectionPosition; | ||
return this._overlay.position().connectedTo( | ||
this._datepickerInput.getPopupConnectionElementRef(), origin, overlay); | ||
return this._overlay.position() | ||
.connectedTo(this._datepickerInput.getPopupConnectionElementRef(), | ||
{originX: 'start', originY: 'bottom'}, | ||
{overlayX: 'start', overlayY: 'top'} | ||
) | ||
.withFallbackPosition( | ||
{ originX: 'start', originY: 'top' }, | ||
{ overlayX: 'start', overlayY: 'bottom' } | ||
) | ||
.withFallbackPosition( | ||
{originX: 'end', originY: 'bottom'}, | ||
{overlayX: 'end', overlayY: 'top'} | ||
) | ||
.withFallbackPosition( | ||
{ originX: 'end', originY: 'top' }, | ||
{ overlayX: 'end', overlayY: 'bottom' } | ||
); | ||
} | ||
} |
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?Uh oh!
There was an error while loading. Please reload this page.
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.