feat(select): Initial Implementation #4918

traviskaufman · 2016-11-16T16:58:21Z

Remove restriction on menu roles. Simply look for any mdl-list-item
with a role attribute when querying for menu items.
Implement select UI with MDL simple menu.
Implement Pure CSS version on top of browser's select element
Bump up karma timeouts in a blind effort to decrease TravisCI
flakiness

NOTE: Auto-positioning the select menu still needs to be done.

Part of #4475
[Delivers #126819221]

traviskaufman · 2016-11-16T17:04:43Z

packages/mdl-select/mdl-select.scss

+    border-bottom: 1px dotted rgba(white, .38);
+  }
+}
+// TODO: disabled


sgomes

Excellent work! Left some minor comments, nothing that requires a re-review.

sgomes · 2016-11-17T11:59:34Z

demos/select.html

+        padding-left: 0;
+      }
+
+      /* Hack to work around style-loader asynchronously loading styles. */


Is there any way we can have these hacks only when serving from npm run dev? We should probably avoid having these once we start publishing builds to gh-pages.

Definitely worth investigating removing these hacks once we start publishing demo pages. It may be worth revisiting demos altogether and give them more structure and an actual sane architecture ;)

Sounds good, no need to tackle just yet.

sgomes · 2016-11-17T12:01:58Z

demos/select.html

+      <section>
+        <h2>Custom Menu + Native Menu on mobile</h2>
+        <em>
+          TODO: Present both, maybe have a wrapper element that switches between both and


sgomes · 2016-11-17T12:03:03Z

packages/mdl-menu/README.md

@@ -32,7 +32,7 @@ A simple menu is usually closed, appearing when opened. It is appropriate for an
  focused instead, remove `tabindex="-1"` from the root element.

 ```js
-let menu = new mdl.SimpleMenu(document.querySelector('.mdl-simple-menu'));
+let menu = new mdl.menu.SimpleMenu(document.querySelector('.mdl-simple-menu'));


Oops, thanks for the fix!

sgomes · 2016-11-17T12:11:49Z

packages/mdl-select/README.md

+| `makeUnfocusable() => void` | Disallows the root element to be focusable via keyboard. We achieve this by setting the root element's `tabIndex` property to `-1`. |
+| `getComputedStyleValue(propertyName: string) => string` | Get the root element's computed style value of the given dasherized css property `propertyName`. We achieve this via `getComputedStyle(...).getPropertyValue(propertyName). `|
+| `setStyle(propertyName: string, value: string) => void` | Sets a dasherized css property `propertyName` to the value `value` on the root element. We achieve this via `root.style.setProperty(propertyName, value)`. |
+| `create2dRenderingContext() => {font: string, measureText: (string) => {width: number}}` | Returns an object which has the shape of a CanvasRenderingContext2d instance. Namely, it has a string property `font` which is writable, and a method `measureText` which given a string of text returns an object containing a `width` property specifying how wide that text should be rendered in the `font` specified by the font property. An easy way to achieve this is simply `document.createElement('canvas').getContext('2d');`. |


I'm wondering if this isn't too low-level... This is just used for determining how big we should make the UI, right? Perhaps that should be done at the component/util level? WDYT?

I'd rather keep it at the foundation level, since it is an essential part of good UX for a select. I'd be hesitant to make a higher-level computeWidth or similar adapter method since that might be too high-level, even if we were to factor it into a util. Ideally, each adapter method is self-describing and can be written in a few lines or less of code

I guess the deciding factor is whether or not there is an idiomatic way of achieving the high-level behaviour within a framework. If we're assuming that the canvas technique can be considered idiomatic (or at least not an anti-idiom) on most frameworks, I'm happy keeping it low-level.

sgomes · 2016-11-17T12:14:04Z

packages/mdl-select/README.md

+| `getComputedStyleValue(propertyName: string) => string` | Get the root element's computed style value of the given dasherized css property `propertyName`. We achieve this via `getComputedStyle(...).getPropertyValue(propertyName). `|
+| `setStyle(propertyName: string, value: string) => void` | Sets a dasherized css property `propertyName` to the value `value` on the root element. We achieve this via `root.style.setProperty(propertyName, value)`. |
+| `create2dRenderingContext() => {font: string, measureText: (string) => {width: number}}` | Returns an object which has the shape of a CanvasRenderingContext2d instance. Namely, it has a string property `font` which is writable, and a method `measureText` which given a string of text returns an object containing a `width` property specifying how wide that text should be rendered in the `font` specified by the font property. An easy way to achieve this is simply `document.createElement('canvas').getContext('2d');`. |
+| `openMenuWithFocusedOptionAtIndex(index: string) => void` | Opens the select's menu with focus on the option at the given index. The index is guaranteed to be in bounds. |


This is the only adapter method that deals with opening the menu, right? You can probably just call it openMenu and explain the parameter as you already do.

sgomes · 2016-11-17T12:16:06Z

packages/mdl-select/README.md

+selects.native.addEventListener('change', changeHandler);
+```
+
+We are looking into building this functionality into `MDLSelect` in the future.


You answered my question before I even asked it :)

sgomes · 2016-11-17T12:27:51Z

packages/mdl-select/index.js

+      makeFocusable: () => {
+        this.root_.tabIndex = 0;
+      },
+      makeUnfocusable: () => {


Consider makeTabbable/makeUntabbable rather than makeFocusable/makeUnfocusable. It's still programmatically focusable if you set tabIndex to -1.

sgomes · 2016-11-17T13:46:50Z

packages/mdl-select/mdl-select.scss

+
+@mixin mdl-select-dd-arrow-svg-bg_($fill-hex-number: 000000, $opacity: .54) {
+  // stylelint-disable scss/dollar-variable-pattern
+  background-image: url(data:image/svg+xml,%3Csvg%20width%3D%2210px%22%20height%3D%225px%22%20viewBox%3D%227%2010%2010%205%22%20version%3D%221.1%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%20xmlns%3Axlink%3D%22http%3A%2F%2Fwww.w3.org%2F1999%2Fxlink%22%3E%0A%20%20%20%20%3Cpolygon%20id%3D%22Shape%22%20stroke%3D%22none%22%20fill%3D%22%23#{$fill-hex-number}%22%20fill-rule%3D%22evenodd%22%20opacity%3D%22#{$opacity}%22%20points%3D%227%2010%2012%2015%2017%2010%22%3E%3C%2Fpolygon%3E%0A%3C%2Fsvg%3E);


Huh, this is a nice technique! Does it work everywhere? If so, we may want to see if we can expand it to places like the checkbox, where the SVG is currently provided in the markup.

I'd love to but the issue with the checkbox is that we need to animate the stroke-dashoffset of the mark in order to "draw" it in. Any ideas of how we could do that if the checkbox was a bg image? Maybe with a SVG film-strip + masking? May get complicated doing different state transitions though.

I don't think there's any way of controlling an animation on a background image. You could do the film-reel technique you mentioned, or you could alternate between a static and a permanently animating background (although the timing might not necessarily match). Either way, that would be future research we're talking about.

traviskaufman · 2016-11-21T16:42:36Z

I'm going to temporarily disable Firefox because that seems to be what's flaking on TravisCI however our test surface is really low now :/ hence #4922

* Remove restriction on menu roles. Simply look for any `mdl-list-item` with a `role` attribute when querying for menu items. * Implement select UI with MDL simple menu. * Implement Pure CSS version on top of browser's select element * Bump up karma timeouts in a blind effort to decrease TravisCI flakiness * Disable Firefox in CI as it seems to be flaky :/ NOTE: Auto-positioning the select menu still needs to be done. Part of #4475 [Delivers #126819221]

sgomes · 2016-11-22T11:30:41Z

Added some more comments. As before, LGTM 👍

traviskaufman · 2016-11-22T15:24:05Z

Gonna go ahead and merge this.

piotr-cz · 2017-04-19T13:34:31Z

Too bad the implementation along with v2 has been moved to material-components-web.
ATM either MDL or MDC has complete implementations of MD

traviskaufman assigned sgomes Nov 16, 2016

googlebot added the cla: yes label Nov 16, 2016

traviskaufman commented Nov 16, 2016

View reviewed changes

packages/mdl-select/mdl-select.scss

border-bottom: 1px dotted rgba(white, .38);

}

}

// TODO: disabled

Copy link

Contributor Author

traviskaufman Nov 16, 2016

Choose a reason for hiding this comment

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

whoops

traviskaufman force-pushed the feat/select branch from 1211adf to 7c614b5 Compare November 16, 2016 17:08

sgomes approved these changes Nov 17, 2016

View reviewed changes

traviskaufman mentioned this pull request Nov 21, 2016

Implement js <-> native select switcher component #4923

Closed

traviskaufman force-pushed the feat/select branch from 7c614b5 to cc7c042 Compare November 21, 2016 17:14

traviskaufman merged commit 2bdb563 into master Nov 22, 2016

traviskaufman deleted the feat/select branch November 22, 2016 15:24

traviskaufman mentioned this pull request Dec 5, 2016

Implement js <-> native select switcher component material-components/material-components-web#4

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(select): Initial Implementation #4918

feat(select): Initial Implementation #4918

traviskaufman commented Nov 16, 2016

traviskaufman Nov 16, 2016

sgomes left a comment

sgomes Nov 17, 2016

traviskaufman Nov 21, 2016

sgomes Nov 22, 2016

sgomes Nov 17, 2016

traviskaufman Nov 21, 2016

sgomes Nov 22, 2016

sgomes Nov 17, 2016

sgomes Nov 17, 2016

traviskaufman Nov 21, 2016

sgomes Nov 22, 2016

sgomes Nov 17, 2016

traviskaufman Nov 21, 2016

sgomes Nov 17, 2016

sgomes Nov 17, 2016

sgomes Nov 17, 2016

traviskaufman Nov 21, 2016

sgomes Nov 22, 2016

traviskaufman commented Nov 21, 2016

sgomes commented Nov 22, 2016

traviskaufman commented Nov 22, 2016

piotr-cz commented Apr 19, 2017

feat(select): Initial Implementation #4918

feat(select): Initial Implementation #4918

Conversation

traviskaufman commented Nov 16, 2016

Choose a reason for hiding this comment

sgomes left a 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

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

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

Choose a reason for hiding this comment

traviskaufman commented Nov 21, 2016

sgomes commented Nov 22, 2016

traviskaufman commented Nov 22, 2016

piotr-cz commented Apr 19, 2017