-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
feat(autocomplete): support for md-optgroup #5604
Conversation
|
||
it('should scroll to active options below the fold', fakeAsync(() => { | ||
tick(); | ||
const container = document.querySelector('.cdk-overlay-pane .mat-autocomplete-panel')!; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about querying that element in the beforeEach
? The tick()
and container
variable initialization is done in every test of the current describe
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Done.
4c47a25
to
8ad659f
Compare
<input mdInput placeholder="State" [mdAutocomplete]="auto" [(ngModel)]="selectedState"> | ||
</md-input-container> | ||
|
||
<md-autocomplete #auto="mdAutocomplete"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add an autocomplete like this to the autocomplete demo page?
const groups = this.autocomplete.optionGroups.toArray(); | ||
let groupCounter = 0; | ||
|
||
for (let i = 0; i < optionIndex + 1; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This process looks pretty similar to how select calculates option index here. Given this is a common operation for option groups, maybe we could move this into a reusable function in core/option? A bit DRY-er.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what that would even be called, countGroupLabelsBeforeOption
? I would rather put it as a static method on the MdOption
instead since having it float around as a function implies that it's a bit more reusable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
countGroupsBeforeOption
sounds reasonable, but open to having it as a static method on MdOption
too.
Can we also add a blurb to the autocomplete docs, so people know about this feature? |
8ad659f
to
fd5c261
Compare
Added all the changes as discussed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just needs a rebase.
1412a6f
to
985f74d
Compare
Adds support for `md-optgroup` in `md-autocomplete` by: * Changing the `@ViewChild` query to pick up descendant options. * Tweaking the keyboard scrolling to handle having group labels before options. Fixes angular#5581.
985f74d
to
d24822d
Compare
name: string; | ||
} | ||
|
||
interface StateGroup { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Failing presubmit because of TS error for "private name StateGroup
" and "private name State
". I think you need to export these interfaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also can you fix the lint failure? https://travis-ci.org/angular/material2/jobs/266064783
Fixed @kara. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Adds support for
md-optgroup
inmd-autocomplete
by:@ViewChild
query to pick up descendant options.Fixes #5581.