Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(autocomplete): double-clicking input shouldnt close the panel #2835

Merged
merged 1 commit into from
Feb 2, 2017
Merged

fix(autocomplete): double-clicking input shouldnt close the panel #2835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 17 additions & 6 deletions src/lib/autocomplete/autocomplete-trigger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ import {Observable} from 'rxjs/Observable';
import {MdOptionSelectEvent, MdOption} from '../core/option/option';
import {ActiveDescendantKeyManager} from '../core/a11y/activedescendant-key-manager';
import {ENTER, UP_ARROW, DOWN_ARROW} from '../core/keyboard/keycodes';
import {Dir} from '../core/rtl/dir';
import {Subscription} from 'rxjs/Subscription';
import {Subject} from 'rxjs/Subject';
import 'rxjs/add/observable/of';
import 'rxjs/add/observable/merge';
import {Dir} from '../core/rtl/dir';
import 'rxjs/add/operator/startWith';
import 'rxjs/add/operator/switchMap';

Expand Down Expand Up @@ -59,7 +60,7 @@ export const MD_AUTOCOMPLETE_VALUE_ACCESSOR: any = {
'[attr.aria-expanded]': 'panelOpen.toString()',
'[attr.aria-owns]': 'autocomplete?.id',
'(focus)': 'openPanel()',
'(blur)': '_onTouched()',
'(blur)': '_handleBlur($event.relatedTarget?.tagName)',
'(input)': '_handleInput($event.target.value)',
'(keydown)': '_handleKeydown($event)',
},
Expand All @@ -77,6 +78,9 @@ export class MdAutocompleteTrigger implements AfterContentInit, ControlValueAcce
private _keyManager: ActiveDescendantKeyManager;
private _positionStrategy: ConnectedPositionStrategy;

/** Stream of blur events that should close the panel. */
private _blurStream = new Subject<any>();

/** View -> model callback called when value changes */
_onChange: (value: any) => {};

Expand Down Expand Up @@ -132,12 +136,12 @@ export class MdAutocompleteTrigger implements AfterContentInit, ControlValueAcce

/**
* A stream of actions that should close the autocomplete panel, including
* when an option is selected and when the backdrop is clicked.
* when an option is selected, on blur, and when TAB is pressed.
*/
get panelClosingActions(): Observable<MdOptionSelectEvent> {
return Observable.merge(
...this.optionSelections,
this._overlayRef.backdropClick(),
this._blurStream.asObservable(),
this._keyManager.tabOut
);
}
Expand Down Expand Up @@ -201,6 +205,15 @@ export class MdAutocompleteTrigger implements AfterContentInit, ControlValueAcce
this.openPanel();
}

_handleBlur(newlyFocusedTag: string): void {
this._onTouched();

// Only emit blur event if the new focus is *not* on an option.
if (newlyFocusedTag !== 'MD-OPTION') {
this._blurStream.next(null);
}
}

/**
* Given that we are not actually focusing active options, we must manually adjust scroll
* to reveal options below the fold. First, we find the offset of the option from the top
Expand Down Expand Up @@ -283,8 +296,6 @@ export class MdAutocompleteTrigger implements AfterContentInit, ControlValueAcce
const overlayState = new OverlayState();
overlayState.positionStrategy = this._getOverlayPosition();
overlayState.width = this._getHostWidth();
overlayState.hasBackdrop = true;
overlayState.backdropClass = 'md-overlay-transparent-backdrop';
overlayState.direction = this._dir ? this._dir.value : 'ltr';
return overlayState;
}
Expand Down
105 changes: 53 additions & 52 deletions src/lib/autocomplete/autocomplete.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ describe('MdAutocomplete', () => {
it('should open the panel when the input is focused', () => {
expect(fixture.componentInstance.trigger.panelOpen)
.toBe(false, `Expected panel state to start out closed.`);

dispatchEvent('focus', input);
fixture.detectChanges();

Expand All @@ -74,6 +75,7 @@ describe('MdAutocomplete', () => {
it('should open the panel programmatically', () => {
expect(fixture.componentInstance.trigger.panelOpen)
.toBe(false, `Expected panel state to start out closed.`);

fixture.componentInstance.trigger.openPanel();
fixture.detectChanges();

Expand All @@ -85,22 +87,18 @@ describe('MdAutocomplete', () => {
.toContain('California', `Expected panel to display when opened programmatically.`);
});

it('should close the panel when a click occurs outside it', async(() => {
it('should close the panel when blurred', async(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you include a test checking that the panel is open after changing focus to an md-option?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean when you make the md-option active by the keyboard, right? Focus never changes to md-option unless you click on it, in which case the panel should close.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added another test in the keyboard events section for that

dispatchEvent('focus', input);
fixture.detectChanges();

fixture.whenStable().then(() => {
const backdrop =
overlayContainerElement.querySelector('.cdk-overlay-backdrop') as HTMLElement;
backdrop.click();
dispatchEvent('blur', input);
fixture.detectChanges();

fixture.whenStable().then(() => {
expect(fixture.componentInstance.trigger.panelOpen)
.toBe(false, `Expected clicking outside the panel to set its state to closed.`);
expect(overlayContainerElement.textContent)
.toEqual('', `Expected clicking outside the panel to close the panel.`);
});
expect(fixture.componentInstance.trigger.panelOpen)
.toBe(false, `Expected clicking outside the panel to set its state to closed.`);
expect(overlayContainerElement.textContent)
.toEqual('', `Expected clicking outside the panel to close the panel.`);
});
}));

Expand All @@ -113,12 +111,10 @@ describe('MdAutocomplete', () => {
option.click();
fixture.detectChanges();

fixture.whenStable().then(() => {
expect(fixture.componentInstance.trigger.panelOpen)
.toBe(false, `Expected clicking an option to set the panel state to closed.`);
expect(overlayContainerElement.textContent)
.toEqual('', `Expected clicking an option to close the panel.`);
});
expect(fixture.componentInstance.trigger.panelOpen)
.toBe(false, `Expected clicking an option to set the panel state to closed.`);
expect(overlayContainerElement.textContent)
.toEqual('', `Expected clicking an option to close the panel.`);
});
}));

Expand Down Expand Up @@ -148,31 +144,26 @@ describe('MdAutocomplete', () => {
options[1].click();
fixture.detectChanges();

fixture.whenStable().then(() => {
expect(fixture.componentInstance.trigger.panelOpen)
.toBe(false, `Expected clicking a new option to set the panel state to closed.`);
expect(overlayContainerElement.textContent)
.toEqual('', `Expected clicking a new option to close the panel.`);
});
expect(fixture.componentInstance.trigger.panelOpen)
.toBe(false, `Expected clicking a new option to set the panel state to closed.`);
expect(overlayContainerElement.textContent)
.toEqual('', `Expected clicking a new option to close the panel.`);
});
});
}));

it('should close the panel programmatically', async(() => {
it('should close the panel programmatically', () => {
fixture.componentInstance.trigger.openPanel();
fixture.detectChanges();

fixture.componentInstance.trigger.closePanel();
fixture.detectChanges();

fixture.whenStable().then(() => {
expect(fixture.componentInstance.trigger.panelOpen)
.toBe(false, `Expected closing programmatically to set the panel state to closed.`);
expect(overlayContainerElement.textContent)
.toEqual('', `Expected closing programmatically to close the panel.`);
});

}));
expect(fixture.componentInstance.trigger.panelOpen)
.toBe(false, `Expected closing programmatically to set the panel state to closed.`);
expect(overlayContainerElement.textContent)
.toEqual('', `Expected closing programmatically to close the panel.`);
});

it('should close the panel when the options list is empty', async(() => {
dispatchEvent('focus', input);
Expand All @@ -183,15 +174,13 @@ describe('MdAutocomplete', () => {
input.value = 'af';
dispatchEvent('input', input);
fixture.detectChanges();
fixture.whenStable().then(() => {
expect(fixture.componentInstance.trigger.panelOpen)
.toBe(false, `Expected panel to close when options list is empty.`);
expect(overlayContainerElement.textContent)
.toEqual('', `Expected panel to close when options list is empty.`);
});

expect(fixture.componentInstance.trigger.panelOpen)
.toBe(false, `Expected panel to close when options list is empty.`);
expect(overlayContainerElement.textContent)
.toEqual('', `Expected panel to close when options list is empty.`);
});
}));

});

it('should have the correct text direction in RTL', () => {
Expand Down Expand Up @@ -428,7 +417,7 @@ describe('MdAutocomplete', () => {
fixture.detectChanges();
});

it('should should not focus the option when DOWN key is pressed', async(() => {
it('should not focus the option when DOWN key is pressed', async(() => {
fixture.whenStable().then(() => {
spyOn(fixture.componentInstance.options.first, 'focus');

Expand All @@ -437,12 +426,26 @@ describe('MdAutocomplete', () => {
});
}));

it('should not close the panel when DOWN key is pressed', async(() => {
fixture.whenStable().then(() => {
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);

expect(fixture.componentInstance.trigger.panelOpen)
.toBe(true, `Expected panel state to stay open when DOWN key is pressed.`);
expect(overlayContainerElement.textContent)
.toContain('Alabama', `Expected panel to keep displaying when DOWN key is pressed.`);
expect(overlayContainerElement.textContent)
.toContain('California', `Expected panel to keep displaying when DOWN key is pressed.`);
});
}));

it('should set the active item to the first option when DOWN key is pressed', async(() => {
fixture.whenStable().then(() => {
const optionEls =
overlayContainerElement.querySelectorAll('md-option') as NodeListOf<HTMLElement>;

fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);

fixture.whenStable().then(() => {
fixture.detectChanges();
expect(fixture.componentInstance.trigger.activeOption)
Expand Down Expand Up @@ -567,22 +570,20 @@ describe('MdAutocomplete', () => {
fixture.componentInstance.trigger._handleKeydown(ENTER_EVENT);
fixture.detectChanges();

fixture.whenStable().then(() => {
expect(fixture.componentInstance.trigger.panelOpen)
.toBe(false, `Expected panel state to read closed after ENTER key.`);
expect(overlayContainerElement.textContent)
.toEqual('', `Expected panel to close after ENTER key.`);
expect(fixture.componentInstance.trigger.panelOpen)
.toBe(false, `Expected panel state to read closed after ENTER key.`);
expect(overlayContainerElement.textContent)
.toEqual('', `Expected panel to close after ENTER key.`);

input.value = 'Alabam';
dispatchEvent('input', input);
fixture.detectChanges();
input.value = 'Alabam';
dispatchEvent('input', input);
fixture.detectChanges();

expect(fixture.componentInstance.trigger.panelOpen)
.toBe(true, `Expected panel state to read open when typing in input.`);
expect(overlayContainerElement.textContent)
.toContain('Alabama', `Expected panel to display when typing in input.`);
expect(fixture.componentInstance.trigger.panelOpen)
.toBe(true, `Expected panel state to read open when typing in input.`);
expect(overlayContainerElement.textContent)
.toContain('Alabama', `Expected panel to display when typing in input.`);
});
});
}));

it('should scroll to active options below the fold', async(() => {
Expand Down