Skip to content

Commit

Permalink
fix(select): prevent the panel from going outside the viewport horizo…
Browse files Browse the repository at this point in the history
…ntally (#3864)

Fixes #3504.
Fixes #3831.
  • Loading branch information
crisbeto authored and kara committed Apr 21, 2017
1 parent 5983a2b commit e10bb18
Show file tree
Hide file tree
Showing 3 changed files with 180 additions and 69 deletions.
2 changes: 1 addition & 1 deletion src/lib/select/select.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

<ng-template cdk-connected-overlay [origin]="origin" [open]="panelOpen" hasBackdrop (backdropClick)="close()"
backdropClass="cdk-overlay-transparent-backdrop" [positions]="_positions" [minWidth]="_triggerWidth"
[offsetY]="_offsetY" [offsetX]="_offsetX" (attach)="_setScrollTop()" (detach)="close()">
[offsetY]="_offsetY" (attach)="_onAttached()" (detach)="close()">
<div class="mat-select-panel" [@transformPanel]="'showing'" (@transformPanel.done)="_onPanelDone()"
(keydown)="_keyManager.onKeydown($event)" [style.transformOrigin]="_transformOrigin"
[class.mat-select-panel-done-animating]="_panelDoneAnimating" [ngClass]="'mat-' + color">
Expand Down
185 changes: 133 additions & 52 deletions src/lib/select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,23 @@ import {TAB} from '../core/keyboard/keycodes';
import {ScrollDispatcher} from '../core/overlay/scroll/scroll-dispatcher';


class FakeViewportRuler {
getViewportRect() {
return {
left: 0, top: 0, width: 1014, height: 686, bottom: 686, right: 1014
};
}

getViewportScrollPosition() {
return {top: 0, left: 0};
}
}

describe('MdSelect', () => {
let overlayContainerElement: HTMLElement;
let dir: {value: 'ltr'|'rtl'};
let scrolledSubject = new Subject();
let fakeViewportRuler = new FakeViewportRuler();

beforeEach(async(() => {
TestBed.configureTestingModule({
Expand Down Expand Up @@ -69,10 +82,10 @@ describe('MdSelect', () => {

return {getContainerElement: () => overlayContainerElement};
}},
{provide: ViewportRuler, useValue: fakeViewportRuler},
{provide: Dir, useFactory: () => {
return dir = { value: 'ltr' };
}},
{provide: ViewportRuler, useClass: FakeViewportRuler},
{provide: ScrollDispatcher, useFactory: () => {
return {scrolled: (delay: number, callback: () => any) => {
return scrolledSubject.asObservable().subscribe(callback);
Expand Down Expand Up @@ -942,6 +955,91 @@ describe('MdSelect', () => {

});

describe('limited space to open horizontally', () => {
beforeEach(() => {
select.style.position = 'absolute';
select.style.top = '200px';
});

it('should stay within the viewport when overflowing on the left in ltr', fakeAsync(() => {
select.style.left = '-100px';
trigger.click();
tick(400);
fixture.detectChanges();

const panelLeft = document.querySelector('.mat-select-panel')
.getBoundingClientRect().left;
expect(panelLeft).toBeGreaterThan(0,
`Expected select panel to be inside the viewport in ltr.`);
}));

it('should stay within the viewport when overflowing on the left in rtl', fakeAsync(() => {
dir.value = 'rtl';
select.style.left = '-100px';
trigger.click();
tick(400);
fixture.detectChanges();

const panelLeft = document.querySelector('.mat-select-panel')
.getBoundingClientRect().left;

expect(panelLeft).toBeGreaterThan(0,
`Expected select panel to be inside the viewport in rtl.`);
}));

it('should stay within the viewport when overflowing on the right in ltr', fakeAsync(() => {
select.style.right = '-100px';
trigger.click();
tick(400);
fixture.detectChanges();

const viewportRect = fakeViewportRuler.getViewportRect().right;
const panelRight = document.querySelector('.mat-select-panel')
.getBoundingClientRect().right;

expect(viewportRect - panelRight).toBeGreaterThan(0,
`Expected select panel to be inside the viewport in ltr.`);
}));

it('should stay within the viewport when overflowing on the right in rtl', fakeAsync(() => {
dir.value = 'rtl';
select.style.right = '-100px';
trigger.click();
tick(400);
fixture.detectChanges();

const viewportRect = fakeViewportRuler.getViewportRect().right;
const panelRight = document.querySelector('.mat-select-panel')
.getBoundingClientRect().right;

expect(viewportRect - panelRight).toBeGreaterThan(0,
`Expected select panel to be inside the viewport in rtl.`);
}));

it('should keep the position within the viewport on repeat openings', async(() => {
select.style.left = '-100px';
trigger.click();
fixture.detectChanges();

let panelLeft = document.querySelector('.mat-select-panel').getBoundingClientRect().left;

expect(panelLeft).toBeGreaterThan(0, `Expected select panel to be inside the viewport.`);

fixture.componentInstance.select.close();
fixture.detectChanges();

fixture.whenStable().then(() => {
trigger.click();
fixture.detectChanges();
panelLeft = document.querySelector('.mat-select-panel').getBoundingClientRect().left;

expect(panelLeft).toBeGreaterThan(0,
`Expected select panel continue being inside the viewport.`);
});
}));

});

describe('when scrolled', () => {

// Need to set the scrollTop two different ways to support
Expand Down Expand Up @@ -1063,42 +1161,38 @@ describe('MdSelect', () => {
select.style.marginRight = '30px';
});

it('should align the trigger and the selected option on the x-axis in ltr', async(() => {
it('should align the trigger and the selected option on the x-axis in ltr', fakeAsync(() => {
trigger.click();
tick(400);
fixture.detectChanges();

fixture.whenStable().then(() => {
const triggerLeft = trigger.getBoundingClientRect().left;
const firstOptionLeft = document.querySelector('.cdk-overlay-pane md-option')
.getBoundingClientRect().left;
const triggerLeft = trigger.getBoundingClientRect().left;
const firstOptionLeft = document.querySelector('.cdk-overlay-pane md-option')
.getBoundingClientRect().left;

// Each option is 32px wider than the trigger, so it must be adjusted 16px
// to ensure the text overlaps correctly.
expect(firstOptionLeft.toFixed(2)).toEqual((triggerLeft - 16).toFixed(2),
`Expected trigger to align with the selected option on the x-axis in LTR.`);
});
// Each option is 32px wider than the trigger, so it must be adjusted 16px
// to ensure the text overlaps correctly.
expect(firstOptionLeft.toFixed(2)).toEqual((triggerLeft - 16).toFixed(2),
`Expected trigger to align with the selected option on the x-axis in LTR.`);
}));

it('should align the trigger and the selected option on the x-axis in rtl', async(() => {
it('should align the trigger and the selected option on the x-axis in rtl', fakeAsync(() => {
dir.value = 'rtl';
fixture.whenStable().then(() => {
fixture.detectChanges();
fixture.detectChanges();

trigger.click();
fixture.detectChanges();
trigger.click();
tick(400);
fixture.detectChanges();

fixture.whenStable().then(() => {
const triggerRight = trigger.getBoundingClientRect().right;
const firstOptionRight =
document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().right;

// Each option is 32px wider than the trigger, so it must be adjusted 16px
// to ensure the text overlaps correctly.
expect(firstOptionRight.toFixed(2))
.toEqual((triggerRight + 16).toFixed(2),
`Expected trigger to align with the selected option on the x-axis in RTL.`);
});
});
const triggerRight = trigger.getBoundingClientRect().right;
const firstOptionRight =
document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().right;

// Each option is 32px wider than the trigger, so it must be adjusted 16px
// to ensure the text overlaps correctly.
expect(firstOptionRight.toFixed(2))
.toEqual((triggerRight + 16).toFixed(2),
`Expected trigger to align with the selected option on the x-axis in RTL.`);
}));
});

Expand All @@ -1111,8 +1205,8 @@ describe('MdSelect', () => {
trigger = multiFixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement;
select = multiFixture.debugElement.query(By.css('md-select')).nativeElement;

select.style.marginLeft = '20px';
select.style.marginRight = '20px';
select.style.marginLeft = '60px';
select.style.marginRight = '60px';
});

it('should adjust for the checkbox in ltr', async(() => {
Expand All @@ -1131,21 +1225,20 @@ describe('MdSelect', () => {
});
}));

it('should adjust for the checkbox in rtl', async(() => {
it('should adjust for the checkbox in rtl', fakeAsync(() => {
dir.value = 'rtl';
trigger.click();
tick(400);
multiFixture.detectChanges();

multiFixture.whenStable().then(() => {
const triggerRight = trigger.getBoundingClientRect().right;
const firstOptionRight =
document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().right;
const triggerRight = trigger.getBoundingClientRect().right;
const firstOptionRight =
document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().right;

// 48px accounts for the checkbox size, margin and the panel's padding.
expect(firstOptionRight.toFixed(2))
.toEqual((triggerRight + 48).toFixed(2),
`Expected trigger label to align along x-axis, accounting for the checkbox.`);
});
// 48px accounts for the checkbox size, margin and the panel's padding.
expect(firstOptionRight.toFixed(2))
.toEqual((triggerRight + 48).toFixed(2),
`Expected trigger label to align along x-axis, accounting for the checkbox.`);
}));
});

Expand Down Expand Up @@ -2126,15 +2219,3 @@ class BasicSelectWithTheming {
@ViewChild(MdSelect) select: MdSelect;
theme: string;
}

class FakeViewportRuler {
getViewportRect() {
return {
left: 0, top: 0, width: 1014, height: 686, bottom: 686, right: 1014
};
}

getViewportScrollPosition() {
return {top: 0, left: 0};
}
}
62 changes: 46 additions & 16 deletions src/lib/select/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,13 +194,6 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
/** Whether the panel's animation is done. */
_panelDoneAnimating: boolean = false;

/**
* The x-offset of the overlay panel in relation to the trigger's top start corner.
* This must be adjusted to align the selected option text over the trigger text when
* the panel opens. Will change based on LTR or RTL text direction.
*/
_offsetX = 0;

/**
* The y-offset of the overlay panel in relation to the trigger's top start corner.
* This must be adjusted to align the selected option text over the trigger text.
Expand Down Expand Up @@ -505,6 +498,7 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
} else {
this.onClose.emit();
this._panelDoneAnimating = false;
this.overlayDir.offsetX = 0;
}
}

Expand All @@ -526,12 +520,20 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
}
}

/**
* Callback that is invoked when the overlay panel has been attached.
*/
_onAttached(): void {
this._calculateOverlayOffsetX();
this._setScrollTop();
}

/**
* Sets the scroll position of the scroll container. This must be called after
* the overlay pane is attached or the scroll container element will not yet be
* present in the DOM.
*/
_setScrollTop(): void {
private _setScrollTop(): void {
const scrollContainer =
this.overlayDir.overlayRef.overlayElement.querySelector('.mat-select-panel');
scrollContainer.scrollTop = this._scrollTop;
Expand Down Expand Up @@ -729,12 +731,6 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal

/** Calculates the scroll position and x- and y-offsets of the overlay panel. */
private _calculateOverlayPosition(): void {
this._offsetX = this.multiple ? SELECT_MULTIPLE_PANEL_PADDING_X : SELECT_PANEL_PADDING_X;

if (!this._isRtl()) {
this._offsetX *= -1;
}

const panelHeight =
Math.min(this.options.length * SELECT_OPTION_HEIGHT, SELECT_PANEL_MAX_HEIGHT);
const scrollContainerHeight = this.options.length * SELECT_OPTION_HEIGHT;
Expand All @@ -748,7 +744,7 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
// center of the overlay panel rather than the top.
const scrollBuffer = panelHeight / 2;
this._scrollTop = this._calculateOverlayScroll(selectedIndex, scrollBuffer, maxScroll);
this._offsetY = this._calculateOverlayOffset(selectedIndex, scrollBuffer, maxScroll);
this._offsetY = this._calculateOverlayOffsetY(selectedIndex, scrollBuffer, maxScroll);
} else {
// If no option is selected, the panel centers on the first option. In this case,
// we must only adjust for the height difference between the option element
Expand Down Expand Up @@ -810,12 +806,46 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
return this.ariaLabelledby ? null : this.ariaLabel || this.placeholder;
}

/**
* Sets the x-offset of the overlay panel in relation to the trigger's top start corner.
* This must be adjusted to align the selected option text over the trigger text when
* the panel opens. Will change based on LTR or RTL text direction. Note that the offset
* can't be calculated until the panel has been attached, because we need to know the
* content width in order to constrain the panel within the viewport.
*/
private _calculateOverlayOffsetX(): void {
const overlayRect = this.overlayDir.overlayRef.overlayElement.getBoundingClientRect();
const viewportRect = this._viewportRuler.getViewportRect();
const isRtl = this._isRtl();
let offsetX = this.multiple ? SELECT_MULTIPLE_PANEL_PADDING_X : SELECT_PANEL_PADDING_X;

if (!isRtl) {
offsetX *= -1;
}

const leftOverflow = 0 - (overlayRect.left + offsetX
- (isRtl ? SELECT_PANEL_PADDING_X * 2 : 0));
const rightOverflow = overlayRect.right + offsetX - viewportRect.width
+ (isRtl ? 0 : SELECT_PANEL_PADDING_X * 2);

if (leftOverflow > 0) {
offsetX += leftOverflow + SELECT_PANEL_VIEWPORT_PADDING;
} else if (rightOverflow > 0) {
offsetX -= rightOverflow + SELECT_PANEL_VIEWPORT_PADDING;
}

// Set the offset directly in order to avoid having to go through change detection and
// potentially triggering "changed after it was checked" errors.
this.overlayDir.offsetX = offsetX;
this.overlayDir.overlayRef.updatePosition();
}

/**
* Calculates the y-offset of the select's overlay panel in relation to the
* top start corner of the trigger. It has to be adjusted in order for the
* selected option to be aligned over the trigger when the panel opens.
*/
private _calculateOverlayOffset(selectedIndex: number, scrollBuffer: number,
private _calculateOverlayOffsetY(selectedIndex: number, scrollBuffer: number,
maxScroll: number): number {
let optionOffsetFromPanelTop: number;

Expand Down

0 comments on commit e10bb18

Please sign in to comment.