feat(select): Adding md-select-header directive to md-select. #7782

DerekLouie · 2016-03-28T23:26:16Z

@jelbourn @ErinCoughlan @gmoothart @KarenParker Please review.

------ NO LONGER AN ISSUE: KEPT FOR POSTERITY ------

Currently facing an issue where the style.css in the demoSelectHeader folder is not getting compiled into the demo site and subsequently not being applied. Screenshot above only work because I put the css in select.scss to get it to look correct..

This is what it looks like currently without any of the demo css:

devversion · 2016-03-29T08:46:54Z

src/components/select/select.js

@@ -1122,12 +1140,12 @@ function SelectProvider($$interimElementProvider) {
      function watchAsyncLoad() {
        if (opts.loadingAsync && !opts.isRemoved) {
          scope.$$loadingAsyncDone = false;
-          scope.progressMode = 'indeterminate';
+          scope.$mdProgressMode = 'indeterminate';


Why are you prefixing that with $md?

In the SelectMenuDirective I changed the scope from true to false. Adding the $md insures the variable does not get clobbered by angular I believe? @jelbourn for clarification?

It's to make sure that we don't overwrite anything within the user scope (because we are now using the same one). $md- variables should not be used in anyone apps.

👍 Yes, I agree. Good point.

DerekLouie · 2016-03-31T20:56:52Z

Hello,

I have addressed the aforementioned comments and also taken the keydown logic out of the md-select directive code, as to make the header more generic.

Please Review.

EricHeath-AI · 2016-04-04T23:52:31Z

src/components/select/demoSelectHeader/index.html

+                   class="demo-header-searchbox _md-text">
+          </md-select-header>
+          <md-optgroup label="vegetables">
+            <md-option ng-value="vegetable" ng-repeat="vegetable in vegetables |


If I want to perform the search on the server side rather than in memory, then I just do:
ng-repeat="item in getMatches(searchTerm)"
similar to https://material.angularjs.org/latest/demo/autocomplete ?
An example with an xhr, or fake xhr might be nice in the docs.

Hello Eric,

Solid point, I am actually working on #7385 which will mimic the autocomplete directive more closely and will also replace this code.

My intention for this demo was really just to give an example of how the md-select-header could be used. I hope it, in no way, serves as an example of what a dedicated search bar component would look like (but that is coming soon 👍 ).

This demo will change after the search component PR is finished.

ThomasBurleson · 2016-04-05T02:17:56Z

@topherfangio - can you review?

topherfangio · 2016-04-05T20:04:24Z

src/components/select/demoSelectHeader/style.css

+  display: flex;
+  align-items: center;
+  width: auto;
+}


@DerekLouie The reason this CSS is not applied is because we prepend the .selectdemoBasicUsage class to the front of every selector so that it is "scoped" to only this demo.

The problem with this approach is that the menu container which houses the header is actually appended to the body, which is outside of our scoped demo. As far as I know, there is not currently a way to achieve what you want without moving your CSS somewhere more global like the site's app.css which I would recommend against.

If you want to do this, perhaps we can update the build system to recognize a global-styles.css file in the demos and append that to the generated styles.css without the demo scoping. We would just have to be very careful to always add some manual CSS classes (like you've done above) to ensure we don't affect other parts of the app.

Hmmm, the build system change sounds great, but a lot of work for the material team so for the sake of everyone, let's table that.

I discovered that I can use a data- attribute to add a class to the menu-container element. So I think I'll just go that route. Good call adding that hook!

topherfangio · 2016-04-05T20:07:10Z

If you're adding a new directive, I think we should document it's use. Since it is so simple, it probably does not warrant it's own directive docs, but if you could add something to the Select's docs, I think it would help.

Other than that and my two comments, it looks good to me 👍

topherfangio · 2016-04-05T20:12:09Z

src/components/select/select.js

  return {
    restrict: 'E',
    require: ['mdSelectMenu'],
-    scope: true,
+    scope: false,


I lied...one more thought 😳

Can you add a quick comment about why this is being changed? If I recall from the Slack conversation, we don't expect it to affect anyone, but it would be good to have a comment in case another dev comes back later and wants to swap it back to true to fix something else.

DerekLouie · 2016-04-06T01:27:43Z

Hello I have fixed all the aforementioned comments, please review.

Thank you @topherfangio for debugging the styling issue, it's all good now.

topherfangio · 2016-04-06T18:08:34Z

src/components/select/select.js

@@ -69,6 +70,18 @@ angular.module('material.components.select', [
 *   </md-input-container>
 * </hljs>
 *
+ * With a select-header


Can you add a small paragraph here describing potential use-cases and what happens to the original label?

DerekLouie · 2016-04-07T18:53:42Z

Hello,

@topherfangio I have fixed the aforementioned comments, please take another look over at your earliest convenience.

topherfangio · 2016-04-07T19:11:00Z

@DerekLouie Thanks so much for all the updates and putting up with the back-and-forth!

@ThomasBurleson LGTM 👍

ThomasBurleson · 2016-04-14T16:50:10Z

@topherfangio - thx.

DerekLouie · 2016-04-14T17:19:44Z

Hello @ThomasBurleson

I changed some css to align the search input's placeholder text and tightened up the height of the select menu.

Please merge at your earliest convenience!

Best,

Derek

Closes #7782

DerekLouie self-assigned this Mar 28, 2016

DerekLouie added needs: review This PR is waiting on review from the team EOC labels Mar 28, 2016

DerekLouie added this to the EOC - Q1 milestone Mar 28, 2016

DerekLouie force-pushed the mdSelectHeaderTemplate branch 5 times, most recently from 5bfc92e to 8645a11 Compare March 28, 2016 23:46

devversion reviewed Mar 29, 2016
View reviewed changes

DerekLouie force-pushed the mdSelectHeaderTemplate branch 2 times, most recently from 3c3e931 to f4e53f4 Compare March 31, 2016 18:16

DerekLouie force-pushed the mdSelectHeaderTemplate branch from f4e53f4 to a7b517d Compare March 31, 2016 20:58

EricHeath-AI reviewed Apr 4, 2016
View reviewed changes

ThomasBurleson assigned topherfangio and unassigned DerekLouie Apr 5, 2016

DerekLouie force-pushed the mdSelectHeaderTemplate branch 2 times, most recently from 0d8fd14 to 773271b Compare April 5, 2016 17:46

topherfangio reviewed Apr 5, 2016
View reviewed changes

DerekLouie force-pushed the mdSelectHeaderTemplate branch from 773271b to a9f207c Compare April 6, 2016 01:25

DerekLouie force-pushed the mdSelectHeaderTemplate branch from a9f207c to cbdbfbc Compare April 6, 2016 01:39

DerekLouie mentioned this pull request Apr 6, 2016

feat(menuFilter): Adding a menu filter directive. #7906

Closed

topherfangio reviewed Apr 6, 2016
View reviewed changes

DerekLouie force-pushed the mdSelectHeaderTemplate branch from cbdbfbc to 35ed068 Compare April 7, 2016 18:52

topherfangio added pr: merge ready This PR is ready for a caretaker to review and removed needs: review This PR is waiting on review from the team labels Apr 7, 2016

topherfangio assigned ThomasBurleson and unassigned topherfangio Apr 7, 2016

topherfangio mentioned this pull request Apr 11, 2016

Is there a way to enable keyboard-input on md-select? like a combobox #7970

Closed

feat(select): Adding md-select-header directive to md-select.

8682802

DerekLouie force-pushed the mdSelectHeaderTemplate branch from 35ed068 to 8682802 Compare April 14, 2016 17:18

ThomasBurleson closed this in 6275424 Apr 14, 2016

jelbourn pushed a commit that referenced this pull request Apr 28, 2016

feat(select): Adding md-select-header directive to md-select.

c6d08bb

Closes #7782

ThomasBurleson pushed a commit that referenced this pull request May 17, 2016

feat(select): Adding md-select-header directive to md-select.

28ee435

Closes #7782

feat(select): Adding md-select-header directive to md-select. #7782

feat(select): Adding md-select-header directive to md-select. #7782

Uh oh!

Conversation

DerekLouie commented Mar 28, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DerekLouie commented Mar 31, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThomasBurleson commented Apr 5, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

topherfangio commented Apr 5, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DerekLouie commented Apr 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DerekLouie commented Apr 7, 2016

Uh oh!

topherfangio commented Apr 7, 2016

Uh oh!

ThomasBurleson commented Apr 14, 2016

Uh oh!

DerekLouie commented Apr 14, 2016

Uh oh!

Uh oh!