Skip to content

Commit

Permalink
fix(select): wrong panel width if element is hidden initially
Browse files Browse the repository at this point in the history
Fixes the select not having a proper width if it became visible after it was initialized.

Fixes angular#3639.
Fixes angular#3244.
  • Loading branch information
crisbeto committed Mar 21, 2017
1 parent aa3360a commit 17f7e82
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 5 deletions.
59 changes: 58 additions & 1 deletion src/lib/select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ describe('MdSelect', () => {
ThrowsErrorOnInit,
BasicSelectOnPush,
BasicSelectOnPushPreselected,
SelectWithPlainTabindex
SelectWithPlainTabindex,
BasicSelectInitiallyHidden,
BasicSelectNoPlaceholder
],
providers: [
{provide: OverlayContainer, useFactory: () => {
Expand Down Expand Up @@ -155,6 +157,25 @@ describe('MdSelect', () => {
});
}));

it('should set the width of the overlay if the element was hidden initially', async(() => {
let initiallyHidden = TestBed.createComponent(BasicSelectInitiallyHidden);

initiallyHidden.detectChanges();
trigger = initiallyHidden.debugElement.query(By.css('.mat-select-trigger')).nativeElement;
trigger.style.width = '200px';

initiallyHidden.componentInstance.isVisible = true;
initiallyHidden.detectChanges();

initiallyHidden.whenStable().then(() => {
trigger.click();
initiallyHidden.detectChanges();

const pane = overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement;
expect(pane.style.minWidth).toBe('200px');
});
}));

it('should not attempt to open a select that does not have any options', () => {
fixture.componentInstance.foods = [];
fixture.detectChanges();
Expand All @@ -165,6 +186,21 @@ describe('MdSelect', () => {
expect(fixture.componentInstance.select.panelOpen).toBe(false);
});

it('should set the width of the overlay if there is no placeholder', async(() => {
let noPlaceholder = TestBed.createComponent(BasicSelectNoPlaceholder);

noPlaceholder.detectChanges();
trigger = noPlaceholder.debugElement.query(By.css('.mat-select-trigger')).nativeElement;

noPlaceholder.whenStable().then(() => {
trigger.click();
noPlaceholder.detectChanges();

const pane = overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement;
expect(parseInt(pane.style.minWidth)).toBeGreaterThan(0);
});
}));

});

describe('selection logic', () => {
Expand Down Expand Up @@ -1893,6 +1929,27 @@ class MultiSelect {
})
class SelectWithPlainTabindex { }

@Component({
selector: 'basic-select-initially-hidden',
template: `
<md-select [style.display]="isVisible ? 'block' : 'none'">
<md-option value="value">There are no other options</md-option>
</md-select>
`
})
class BasicSelectInitiallyHidden {
isVisible = false;
}

@Component({
selector: 'basic-select-no-placeholder',
template: `
<md-select>
<md-option value="value">There are no other options</md-option>
</md-select>
`
})
class BasicSelectNoPlaceholder { }

class FakeViewportRuler {
getViewportRect() {
Expand Down
14 changes: 10 additions & 4 deletions src/lib/select/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr
this._placeholder = value;

// Must wait to record the trigger width to ensure placeholder width is included.
Promise.resolve(null).then(() => this._triggerWidth = this._getWidth());
Promise.resolve(null).then(() => this._setTriggerWidth());
}

/** Whether the component is disabled. */
Expand Down Expand Up @@ -341,6 +341,11 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr
if (this.disabled || !this.options.length) {
return;
}

if (!this._triggerWidth) {
this._setTriggerWidth();
}

this._calculateOverlayPosition();
this._placeholderState = this._floatPlaceholderState();
this._panelOpen = true;
Expand Down Expand Up @@ -423,11 +428,12 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr
return this._dir ? this._dir.value === 'rtl' : false;
}

/** The width of the trigger element. This is necessary to match
/**
* Sets the width of the trigger element. This is necessary to match
* the overlay width to the trigger width.
*/
_getWidth(): number {
return this._getTriggerRect().width;
private _setTriggerWidth(): void {
this._triggerWidth = this._getTriggerRect().width;
}

/** Ensures the panel opens if activated by the keyboard. */
Expand Down

0 comments on commit 17f7e82

Please sign in to comment.