feat(autocomplete): add autocomplete panel toggling #2452

Merged
merged 1 commit into from Jan 7, 2017

Projects

DONE in md-autocomplete

3 participants

@kara
Collaborator
kara commented Dec 28, 2016 edited

The autocomplete is a WIP. To keep PRs small, this PR only implements the panel toggling behavior. Future PRs will implement forms integration, positioning, and keyboard events.

r: @jelbourn

@kara kara requested a review from jelbourn Dec 28, 2016
@googlebot googlebot added the cla: yes label Dec 28, 2016
+ private _overlayRef: OverlayRef;
+ private _portal: TemplatePortal;
+ private _panelOpen: boolean = false;
+ private _closeWatcher: Subscription;
@jelbourn
jelbourn Jan 4, 2017 Member

_panelClosures? I'd like to avoid the term "watchers" to avoid confusion with the Angular 1 concept.

Also add a description comment for this since it's not immediately obvious what it's for.

+ @Input('mdAutocomplete') autocomplete: MdAutocomplete;
+
+ constructor(private _element: ElementRef, private _overlay: Overlay,
+ private _vcr: ViewContainerRef) {}
@jelbourn
jelbourn Jan 4, 2017 Member

_viewContainerRef

+ }
+
+ /** Destroys the autocomplete suggestion panel. */
+ destroyPanel(): void {
@jelbourn
jelbourn Jan 4, 2017 Member

Should this be an internal method? (Would the user ever have a reason to call it?)

@kara
kara Jan 4, 2017 edited Collaborator

Hmm, someone could possibly call it if they knew that the autocomplete wouldn't be used anymore for some reason and they had a ton of options. But I don't really think it's that useful. Removing. We can always add to the API later if needed

+ * This method will close the panel if it receives a selection event from any of the options
+ * or a click on the backdrop.
+ */
+ private _watchForClose() {
@jelbourn
jelbourn Jan 4, 2017 Member

Different name that doesn't use watch?

+ */
+ private _watchForClose() {
+ // TODO(kara): add tab event watcher when adding keyboard events
+ this._closeWatcher = Observable.merge(...this._getOptionObs(), this._overlayRef.backdropClick())
@jelbourn
jelbourn Jan 4, 2017 Member

Won't this get stale is more options are added while the panel is open?

@kara
kara Jan 4, 2017 Collaborator

This is addressed in the next PR, #2516, when filtering is added (and the options are often changing)

+ * of their selection events. This map will be used to merge the option events and
+ * the backdrop click into one observable.
+ */
+ private _getOptionObs(): Observable<any>[] {
@jelbourn
jelbourn Jan 4, 2017 Member

Make this a getter called optionSelections? Then the description is just something like

/** Stream of autocomplete option selections. */
+ * the backdrop click into one observable.
+ */
+ private _getOptionObs(): Observable<any>[] {
+ return this.autocomplete.options.map((option) => option.onSelect);
@jelbourn
jelbourn Jan 4, 2017 Member

Don't need parens around option

@kara
Collaborator
kara commented Jan 4, 2017

@jelbourn Comments addressed!

@jelbourn

Just a last few things I noticed

+
+ if (!this._overlayRef.hasAttached()) {
+ this._overlayRef.attach(this._portal);
+ this._panelClosingSub = this.panelClosingActions.subscribe(() => this.closePanel());
@jelbourn
jelbourn Jan 4, 2017 Member

_closingActionsSubscription?

+
+ // remove body padding to keep consistent cross-browser
+ document.body.style.padding = '0';
+ document.body.style.margin = '0';
@jelbourn
jelbourn Jan 4, 2017 Member

I don't think these styles are necessary any more since 38700e0

@kara
kara Jan 4, 2017 Collaborator

Oh right! Forgot to update.

+ dispatchEvent('focus', trigger);
+ fixture.detectChanges();
+
+ const backdrop = <HTMLElement>overlayContainerElement.querySelector('.cdk-overlay-backdrop');
@jelbourn
jelbourn Jan 4, 2017 Member

Prefer as HTMLElement
(here and elsewhere)

src/lib/core/option/option.scss
+@import '../style/menu-common';
+@import '../a11y/a11y';
+
+@mixin md-option() {
@jelbourn
jelbourn Jan 4, 2017 Member

Add comment explaining what this mixin is for (i.e., used by both select and autocomplete)

@kara
Collaborator
kara commented Jan 4, 2017

@jelbourn Updated

@jelbourn
Member
jelbourn commented Jan 5, 2017

LGTM

@kara kara feat(autocomplete): add autocomplete panel toggling
21abcc2
@kara kara merged commit d4ab3d3 into angular:master Jan 7, 2017

2 checks passed

cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment