Skip to content

fix(select): prevent the panel from going outside the viewport horizontally #3864

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

Merged
merged 1 commit into from
Apr 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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