feat(select): add md-select-header directive #3211

crisbeto · 2017-02-20T21:06:52Z

Adds a md-select-header component, which is a fixed header above the select's options. It allows for the user to project an input to be used for filtering long lists of options.

Note: This component only handles the positioning, styling and exposes the panel id for a11y. The functionality is up to the user to handle.

Fixes #2812.

fxck · 2017-02-21T11:31:17Z

Would it be worth adding an example in the demo showing a bit more reactive way of doing that filtering (could also be used as async search example)?

I could try putting something together.

Also I think md-select-header should take care of autofocusing the search input.

// edit this could be a start fxck@f0dd2ec

fxck · 2017-03-21T15:44:03Z

@kara hi, I know you are probably busy with whatever you are currently working on on material2 and form improvements in angular itself as well as preparing for upcoming conferences, but could you perhaps please take a look at these two #3211 #3341 select-related PRs from @crisbeto? They are not big changes in the codebase itself and both are very desirable features of the select box I'm sure many many people would appreciate.. Thank you.

Eernie · 2017-03-24T15:38:35Z

src/lib/select/select.spec.ts

@@ -19,7 +19,7 @@ import {
 } from '@angular/forms';
 import {ViewportRuler} from '../core/overlay/position/viewport-ruler';

-describe('MdSelect', () => {
+fdescribe('MdSelect', () => {


I believe the build is failing because of the f before describe

It is, but it's not the only issue. I'll get it passing once I get some feedback on it.

Eernie · 2017-03-24T15:39:26Z

@kara Do you have an ETA for this pull request. I'd love see this merged 👍

Jagpreet16 · 2017-03-25T07:24:51Z

@kara By when can we expect this feature to be available?

gedclack · 2017-04-11T05:12:55Z

excuse me, is md-select-header implemented on beta.3 ?

fxck · 2017-04-11T09:26:28Z

@gedclack as you can see, this PR is not merged, not even reviewed, so no

badre429 · 2017-04-12T21:41:15Z

please work on this pr

fxck · 2017-04-23T19:34:33Z

Any chance this will get implemented in the next like.. 4 months? It's a little unpleasant to have to maintain my own build of material2 because of these two(#3211 #3341) rather small, but pretty important for interfaces of any serious application, features that are like 20 LOC of actual non breaking changes to the codebase in total.

@kara @crisbeto @jelbourn could these please get some attention?

fxck · 2017-05-25T11:21:51Z

😰

zyarnold · 2017-05-30T14:48:28Z

any news for this yet?

fxck · 2017-06-09T17:15:16Z

Any updates? It's been four months.

jelbourn · 2017-06-09T19:54:05Z

We still plan on doing this, it's just been on the back-burner as other things have been higher priority

fxck · 2017-06-10T18:47:01Z

@jelbourn while I totally understand that there were/are things with higher priority, wouldn't it be worth dedicating, say, one day in a month to review and merge these small PRs that are low in effort and pretty damn high in value for consumers?

What are you using to decide the priority? Are you using one of these (1, 2) by any chance? Are you planning on involving the community in the decision in any way in the future?

jelbourn · 2017-06-12T17:18:51Z

The reason this one in particular was back-burnered was that we needed to figure out what the a11y story for this feature is. It would be very easy for someone to break the a11y of the select this this, which is something we want to put some effort into avoiding.

irackley-mc · 2017-06-12T18:56:01Z

@jelbourn Thanks for the update on this. Had been waiting a while wondering why there was no traction on this. Guess I'll have to do a custom build for now but hopefully some priority can be added to this, as I agree with @fxck and it is a great value add.

Adds a `md-select-header` component, which is a fixed header above the select's options. It allows for the user to project an input to be used for filtering long lists of options. **Note:** This component only handles the positioning, styling and exposes the panel id for a11y. The functionality is up to the user to handle. Fixes angular#2812.

crisbeto · 2017-06-14T21:51:51Z

@jelbourn I've cleaned up and rebased this one.

jelbourn · 2017-06-14T22:04:01Z

src/demo-app/select/select-demo.html

+  <md-card *ngIf="showSelect">
+    <md-select placeholder="Drink" [(ngModel)]="currentDrink" #selectWitHeader="mdSelect">
+      <md-select-header>
+        <input type="search" [(ngModel)]="searchTerm" role="combobox" [attr.aria-owns]="selectWitHeader.panelId"


In this demo, if I select a value and then try to type into the input then the focus leaves the text input on every keypress

Hmm you're right. This complicates things a lot, because it means that we'd need two different key handling strategies depending on whether the select has a header (FocusKeyManager by default and ActiveDescendantKeyManager if there's a header), which in turn will complicate the select even more, because it would have to delegate to the header in some cases but not in others. I can see how it can turn into a huge mess pretty quickly since we'd need to sprinkle if (this.header) all over the place and there would be a MdSelect <-> MdSelectHeader dependency. It might be best if we park and rethink whether we want to do this or recommend that people use md-autocomplete instead.

or recommend that people use md-autocomplete instead.

just not this, please, searching is not the sole use case for the select header and even if it was, autocomplete has it's own problems that make it hard to use as a select.. #3334

jelbourn · 2017-06-14T22:12:21Z

src/demo-app/select/select-demo.html

+  <md-card *ngIf="showSelect">
+    <md-select placeholder="Drink" [(ngModel)]="currentDrink" #selectWitHeader="mdSelect">
+      <md-select-header>
+        <input type="search" [(ngModel)]="searchTerm" role="combobox" [attr.aria-owns]="selectWitHeader.panelId"


Did you try this out in a screen reader w/ this role/aria-owns? I'm not sure it will work as expected
(what I had been talking about the other day was that to do this property you would need to revamp how the roles on select are set up completely)

jelbourn · 2017-06-14T22:16:06Z

src/lib/select/select.html

+      <ng-content select="md-select-header, mat-select-header"></ng-content>
+    </div>
+
+    <div class="mat-select-content" [attr.id]="panelId" [@fadeInContent]="'showing'"


I think just [id] should work here

jelbourn · 2017-06-14T22:16:45Z

src/lib/select/select.html

@@ -35,7 +35,12 @@
    [style.transformOrigin]="_transformOrigin"
    [class.mat-select-panel-done-animating]="_panelDoneAnimating">

-    <div class="mat-select-content" [@fadeInContent]="'showing'" (@fadeInContent.done)="_onFadeInDone()">
+    <div [@fadeInContent]="'showing'">


I think you could replace this div with an ng-container

jelbourn · 2017-06-14T22:18:59Z

src/lib/select/select.scss

+  border-bottom: solid 1px;
+  box-sizing: border-box;
+
+  input {


I'd lean toward omitting this style in the select itself, since the select header is mean to be a generic container that's not necessarily used for this style of filter (even if that is the most common use case)

The issue with allowing anything inside the header is that, with the way it is currently set up, the positioning won't work with anything that isn't 48px high. Alternatively, I can hard-code the header height.

I'd rather do that for now and then revisit the positioning to make it more flexible in the future.

fxck · 2017-07-17T11:59:42Z

So is this on hold now again? @jelbourn @crisbeto

crisbeto · 2017-07-17T12:02:37Z

It is until we can figure out a decent way to handle the accessibility.

benb7760 · 2017-08-17T08:48:38Z

@crisbeto Have found a few more issues that haven't been mentioned above when using this;

With two or more options with the same key, and one is selected, when option is filtered out and then unfiltered again, the first option with that value is selected and the other(s) ignored. This behaviour doesn't present in normal select as the options are never removed. Should an error be thrown when providing multiple options with same value?
In multiple mode, if I select some options then apply a filter which hides some of the selected options, then select a new option, when i remove the filter, any that were selected but hidden are no longer selected. This is probably because the selection model is updated based on the options property, which does not contain options when filtered out

I am keeping my own branch for this PR, updated to master, with a basic fix for the focus issue mentioned by jelbourn. I intend to fix the other two issues also, as I need the feature. Would it be useful for me to make a PR for this when done?

crisbeto · 2017-08-17T09:04:19Z

@benb7760 it's not the functionality that stopped this from being pushed through, but rather the accessibility story. I have a task on my list to refactor the select focus management so it can handle having a header.

* Refactors the select to use `aria-activedescendant` to announce the highlighted item to screen readers. Previously we would this through focus, however using focus prevents us from being able to do things like angular#3211. * Fixes a hack that was used to get a hold of the panel element using `querySelector`. Now it properly uses a `ViewChild` query, however this meant some tests had to be updated. Relates to angular#3211. Fixes angular#6690.

Relates to #3211. Fixes #6690.

fxck · 2017-10-06T16:05:52Z

Looks like this can now be done, now that #6856 is in?

crisbeto · 2017-10-06T16:10:38Z

Yes the a11y shouldn't be an issue anymore. I might have to resubmit it though since it'll take a while to rebase this.

badre429 · 2017-10-08T15:07:27Z

@crisbeto is there any timeline for this feature

crisbeto · 2017-10-16T20:40:28Z

Closing in favor of #7835.

angular-automatic-lock-bot · 2019-09-08T12:03:17Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Feb 20, 2017

crisbeto added the pr: needs review label Feb 20, 2017

crisbeto assigned kara Feb 20, 2017

crisbeto added the pr: needs cleanup / tests label Feb 20, 2017

crisbeto force-pushed the 2812/select-header branch 2 times, most recently from 4c37b61 to 04f1c07 Compare February 23, 2017 19:45

fxck mentioned this pull request Mar 12, 2017

[Autocomplete] Restrict selection to given set of items #3334

Closed

Eernie reviewed Mar 24, 2017

View reviewed changes

fxck mentioned this pull request Mar 27, 2017

request: explain the issues labelling / review / merge process in CONTRIBUTING.md #3799

Closed

fxck mentioned this pull request Apr 16, 2017

Request : md-option autoHeight support for <md-select> and/or <md-autocomplete> #4071

Closed

crisbeto force-pushed the 2812/select-header branch from bea22ae to b049c8e Compare June 13, 2017 20:04

crisbeto assigned crisbeto and unassigned kara Jun 13, 2017

crisbeto added 2 commits June 14, 2017 23:37

chore: try to fix edge tests

6df224b

crisbeto force-pushed the 2812/select-header branch from 02c961b to 6f6297e Compare June 14, 2017 21:38

crisbeto removed the pr: needs cleanup / tests label Jun 14, 2017

crisbeto assigned jelbourn and unassigned crisbeto Jun 14, 2017

jelbourn reviewed Jun 15, 2017

View reviewed changes

crisbeto added the needs: discussion Further discussion with the team is needed before proceeding label Jun 18, 2017

crisbeto mentioned this pull request Sep 5, 2017

refactor(select): use aria-activedescendant to manage focus #6856

Merged

crisbeto mentioned this pull request Sep 13, 2017

Feature Request: Multiselect with Checkboxes and Search/Filter Options #7052

Closed

crisbeto mentioned this pull request Sep 26, 2017

feat(select): Add select header to the md-select #2812

Closed

kara pushed a commit that referenced this pull request Oct 3, 2017

refactor(select): use aria-activedescendant to manage focus (#6856)

b5ada20

Relates to #3211. Fixes #6690.

crisbeto mentioned this pull request Oct 16, 2017

feat(select): add mat-select-header component #7835

Closed

crisbeto closed this Oct 16, 2017

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019

feat(select): add md-select-header directive #3211

feat(select): add md-select-header directive #3211

Conversation

crisbeto commented Feb 20, 2017

fxck commented Feb 21, 2017 • edited Loading

fxck commented Mar 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Eernie commented Mar 24, 2017

Jagpreet16 commented Mar 25, 2017

gedclack commented Apr 11, 2017

fxck commented Apr 11, 2017

badre429 commented Apr 12, 2017

fxck commented Apr 23, 2017

fxck commented May 25, 2017

zyarnold commented May 30, 2017

fxck commented Jun 9, 2017 • edited Loading

jelbourn commented Jun 9, 2017

fxck commented Jun 10, 2017

jelbourn commented Jun 12, 2017

irackley-mc commented Jun 12, 2017

crisbeto commented Jun 14, 2017

Choose a reason for hiding this comment

crisbeto Jun 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crisbeto Jun 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fxck commented Jul 17, 2017

crisbeto commented Jul 17, 2017

benb7760 commented Aug 17, 2017 • edited Loading

crisbeto commented Aug 17, 2017

fxck commented Oct 6, 2017

crisbeto commented Oct 6, 2017

badre429 commented Oct 8, 2017

crisbeto commented Oct 16, 2017

angular-automatic-lock-bot bot commented Sep 8, 2019

fxck commented Feb 21, 2017 •

edited

Loading

fxck commented Mar 21, 2017 •

edited

Loading

fxck commented Jun 9, 2017 •

edited

Loading

crisbeto Jun 18, 2017 •

edited

Loading

crisbeto Jun 15, 2017 •

edited

Loading

benb7760 commented Aug 17, 2017 •

edited

Loading