feat(icon-button): Add new package #2748

williamernest · 2018-05-16T18:34:08Z

codecov-io · 2018-05-16T19:05:05Z

Codecov Report

Merging #2748 into master will decrease coverage by 0.06%.
The diff coverage is 97.97%.

@@            Coverage Diff             @@
##           master    #2748      +/-   ##
==========================================
- Coverage   98.48%   98.42%   -0.07%     
==========================================
  Files          98       98              
  Lines        4232     4186      -46     
  Branches      538      532       -6     
==========================================
- Hits         4168     4120      -48     
- Misses         64       66       +2

Impacted Files	Coverage Δ
packages/mdc-icon-button/constants.js	`100% <100%> (ø)`
packages/mdc-icon-button/index.js	`100% <100%> (ø)`
packages/mdc-icon-button/foundation.js	`96.55% <96.55%> (ø)`
packages/mdc-icon-toggle/index.js
packages/mdc-icon-toggle/constants.js
packages/mdc-icon-toggle/foundation.js

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 16a6890...2a95b2c. Read the comment docs.

kfranqueiro

Sending partial review 'cause I have to jump into a meeting soon.

kfranqueiro · 2018-05-16T19:41:47Z

demos/card.html

@@ -58,7 +58,7 @@ <h3 class="demo-card__subtitle mdc-typography--subtitle2">by Kurt Wagner</h3>
              <button class="mdc-button mdc-card__action mdc-card__action--button">Bookmark</button>
            </div>
            <div class="mdc-card__action-icons">
-              <i class="mdc-icon-toggle material-icons mdc-card__action mdc-card__action--icon"
+              <i class="mdc-icon-button material-icons mdc-card__action mdc-card__action--icon"


Shouldn't we also add mdc-icon-button to the other mdc-card__action--icon elements to make sure it works for them? (Which would also affect MDC Card documentation, and might mean there are styles we can pull out of Card if mdc-icon-button already covers it now)

Need to be careful to only instantiate MDCIconButtonToggle for the actual toggle buttons though, and not all icon buttons... This might actually point to a developer ergonomics problem, if it becomes difficult to programmatically distinguish things we want to be toggle-able from things that aren't.

kfranqueiro · 2018-05-16T19:43:06Z

demos/icon-button.html

@@ -0,0 +1,271 @@
+<!DOCTYPE html>


I appreciate that this file was added (especially in this case where it's effectively replacing an existing component), but is it feasible to fast-follow this PR with screenshot testing page(s)?

kfranqueiro · 2018-05-16T19:43:20Z

demos/icon-button.html

@@ -0,0 +1,271 @@
+<!DOCTYPE html>
+<!--
+  Copyright 2016 Google Inc. All rights reserved.


Might want to do a copyright year check on new files

kfranqueiro · 2018-05-16T19:52:12Z

demos/icon-button.html

+            <div class="toggle-example">
+              <h2 class="mdc-typography--headline6">Disabled Buttons</h2>
+              <div class="demo-wrapper">
+                <span class="mdc-icon-button mdc-icon-button--disabled material-icons"


See previous comment RE potentially limiting to button and a elements. We shouldn't need to support a disabled CSS modifier class at all that way, and I suspect we wouldn't need aria-disabled then either (which is missing here but present on others below).

kfranqueiro · 2018-05-16T19:53:38Z

demos/icon-button.html

+                      role="button"
+                      aria-label="Transit directions"
+                      aria-pressed="false"
+                      tabindex="0">


These shouldn't have tabindex if they're disabled.

kfranqueiro · 2018-05-16T19:55:09Z

demos/icon-button.html

+                   role="button"
+                   aria-label="Add to favorites"
+                   aria-pressed="false"
+                   tabindex="0">


a and button elements shouldn't need tabindex, right?

a needs tabIndex if there isn't an href attribute.

kfranqueiro · 2018-05-16T19:56:11Z

demos/icon-button.html

+              <h2 class="mdc-typography--headline6">SVG Icon</h2>
+              <div class="demo-wrapper">
+                <button class="mdc-icon-button"
+                   role="button"


role="button" is redundant for button elements, right?

kfranqueiro · 2018-05-16T19:56:39Z

demos/icon-button.html

+                   aria-label="Add to favorites"
+                   aria-pressed="false"
+                   tabindex="0">
+                  <svg xmlns="http://www.w3.org/2000/svg" width="24px" height="24px" viewBox="0 0 24 24">


Do we need width and height hard-coded inline here or should CSS cover this?

kfranqueiro · 2018-05-16T20:22:29Z

package.json

@@ -153,6 +153,7 @@
          "floating-label",
          "form-field",
          "grid-list",
+          "icon-button",


Make sure your commit message is valid when merging (your PR title has a space instead of hyphen)

kfranqueiro · 2018-05-16T20:23:30Z

packages/mdc-button/_variables.scss

@@ -19,4 +19,7 @@ $mdc-button-horizontal-padding: 8px;
 $mdc-button-contained-horizontal-padding: 16px;
 $mdc-dense-button-height: 32px;

+$mdc-button-icon-padding: 12px;


I'm assuming these don't belong here anymore? They're never used in this PR AFAICT.

kfranqueiro

If we're going to literally move the icon-toggle tests, we should add a deprecation notice to its README a la toolbar.

kfranqueiro · 2018-05-16T21:15:57Z

packages/mdc-icon-button/README.md

+
+<ul class="icon-list">
+  <li class="icon-list-item icon-list-item--spec">
+    <a href="https://material.io/guidelines/components/buttons.html#buttons-toggle-buttons">Material Design guidelines: Toggle buttons</a>


This is an old link. AFAIK there's no new equivalent though... we might want to notify/ask designers about that.

kfranqueiro · 2018-05-16T21:18:25Z

packages/mdc-icon-button/README.md

+    <a href="https://material.io/guidelines/components/buttons.html#buttons-toggle-buttons">Material Design guidelines: Toggle buttons</a>
+  </li>
+  <li class="icon-list-item icon-list-item--link">
+    <a href="https://material-components.github.io/material-components-web-catalog/#/component/icon-toggle">Demo</a>


Should we enter an issue in the catalog repo to add a page for this?

(It would not get merged to master and deployed until we have a new MDC Web release we can update it to; I have thoughts about release process between these packages that I need to formalize and share with the team.)

kfranqueiro · 2018-05-16T21:19:18Z

packages/mdc-icon-button/README.md

+  favorite
+</button>
+```
+> Note: The MDC Icon Button can be used with `<button>`, `<span>`, `<i>`, and `<a>` tags.


As mentioned above, we may want to restrict this to button / a which make the most sense

kfranqueiro · 2018-05-16T21:19:36Z

packages/mdc-icon-button/README.md

+```
+### JavaScript Instantiation
+
+The icon button will work without Javascript, but you can enhance it to hav ea ripple effect by instantiating `MDCRipple` on the root element. 


have ea -> have a

kfranqueiro · 2018-05-16T21:21:50Z

packages/mdc-icon-button/README.md

+```
+
+```js
+var toggleButton = new mdc.iconButton.MDCIconToggle(document.getElementById('add-to-favorites'));


Replace MDCIconToggle with MDCIconButtonToggle across this readme

kfranqueiro · 2018-05-16T21:59:00Z

packages/mdc-icon-button/foundation.js

+   * @param {string} dataAttr
+   * @return {!IconToggleState}
+   */
+  parseJsonDataAttr_(dataAttr) {


I realize this was inherited from icon-toggle, but serious question: which do you think is better for developer ergonomics within HTML?

Current:

data-toggle-off='{"content": "favorite_border", "label": "Add to favorites"}'

This requires JSON within HTML, which in turn requires using single-quotes around the attribute value to easily have double-quotes in the value for valid JSON

Alternative:

data-toggle-off-content="favorite_border" data-toggle-off-label="Add to favorites"

This requires writing more separate attributes (and requires us to read more separate attributes in our code) but doesn't require the use of JSON within HTML and thus the atypical use of single quotes around attribute values.

The downside is this could lead to as many as 6 total attributes (up from 2), and converting existing icon-toggles to icon-button would no longer be a single find/replace. IMO the upside is it's both more readable and writeable.

Log the data attribute redesign as a follow-up issue... but we should really do that before merging this to master and publishing this package to avoid immediate breaking changes.

It should be possible for you to create a centralized branch for mdc-icon-button changes and edit this PR to target that branch.

kfranqueiro · 2018-05-16T22:03:31Z

packages/mdc-icon-button/index.js

+  initRipple_() {
+    const adapter = Object.assign(MDCRipple.createAdapter(this), {
+      isUnbounded: () => true,
+      isSurfaceActive: () => this.foundation_.isKeyboardActivated(),


If we think the keyboard code is safe to remove (see earlier comment), we only need to set unbounded, which we can do without needing to patch the adapter (just set ripple.unbounded = true immediately after instantiating).

kfranqueiro · 2018-05-16T22:04:59Z

packages/mdc-icon-button/mdc-icon-button.scss

+
+.mdc-icon-button {
+  @include mdc-icon-button-base_;
+  @include mdc-states(on-surface);


I'm not sure I understand why we're setting this to on-surface here? That seems like the correct setting for card, but wouldn't be the correct setting in general? Also, we're setting up states but not ink color?

kfranqueiro · 2018-05-16T22:07:41Z

packages/mdc-icon-button/package.json

+    "url": "https://github.com/material-components/material-components-web.git"
+  },
+  "dependencies": {
+    "@material/animation": "^0.34.0",


Is this being used?

kfranqueiro · 2018-05-16T22:14:48Z

packages/mdc-top-app-bar/_mixins.scss

-  color: inherit;
-  text-decoration: none;
-  cursor: pointer;
+  @include mdc-icon-button-base_();


Is there a reason for doing this rather than adding the mdc-button-icon class directly? I'm a bit iffy on depending on "internal" mixins across packages, though maybe there's precedent for that which I'm forgetting.

If there's a convincing argument for this being done this way, maybe we should rename that mixin to be public, sort of like mdc-ripple-surface is.

kfranqueiro · 2018-05-17T13:01:28Z

packages/mdc-top-app-bar/_mixins.scss

@@ -92,5 +91,23 @@
 // Both .mdc-top-app-bar__icon and .mdc-top-app-bar__navigation-icon share all styles except for
 // horizontal padding.
 @mixin mdc-top-app-bar-icon_() {
-  @include mdc-icon-button-base_();
+  @include mdc-ripple-surface;


Did you revert this with the intent of adopting mdc-icon-button within top app bar in a separate PR? I'm a bit confused since I do see card changes in this one.

Card was previously using icon-toggle, so I went ahead and updated that one. Top app bar doesn't need to change with this PR.

kfranqueiro · 2018-05-17T15:30:04Z

Just noticed an issue in IE 11 which only affects Icon Button (not Icon Toggle):

Icons are initially misaligned. When pressed, they move both vertically and horizontally.

Icon Button (new):

Icon Toggle (old):

I tried removing the display style in Icon Button for argument's sake (since in the old demo page, the display: inline-block from material-icons is actually taking precedence), but that didn't seem to make any difference.

(For that matter, is there anything we can do to add specificity to our selectors for styles that conflict with .material-icons to ensure ours take precedence? We've had to do that in e.g. MDC Button and Chips.)

kfranqueiro · 2018-05-17T15:49:58Z

packages/mdc-icon-button/mdc-icon-button.scss

+
+.mdc-icon-button {
+  @include mdc-icon-button-base_;
+  @include mdc-states;


Does this line make sense standalone? Should we include mdc-icon-button-ink-color with a default value instead?

I'm not following. We use mdc-states without passing a parameter in tab, list, card and ripple.

kfranqueiro · 2018-05-17T15:52:56Z

demos/theme/theme-dark.scss

@@ -20,6 +20,7 @@
 $mdc-theme-primary: $material-color-amber-300;
 $mdc-theme-secondary: $material-color-pink-400;
 $mdc-theme-background: $material-color-grey-900;
+$mdc-theme-surface: $material-color-grey-900;


Does this need to be in this PR? Do other themes also need updates?

kfranqueiro · 2018-05-17T15:53:08Z

demos/theme/_menu.scss

@@ -26,7 +26,8 @@
 }

 .demo-theme-list {
-  @include mdc-theme-prop(color, text-primary-on-light);
+  @include mdc-theme-prop(color, on-surface);


Does this need to be in this PR?

kfranqueiro · 2018-05-17T15:55:05Z

packages/mdc-icon-button/README.md

+    <a href="https://material.io/design/components/buttons.html#toggle-button">Material Design guidelines: Toggle buttons</a>
+  </li>
+  <li class="icon-list-item icon-list-item--link">
+    <a href="https://material-components.github.io/material-components-web-catalog/#/component/icon-toggle">Demo</a>


Should we go ahead and update this URL now under the assumption that the next time these docs are pushed to material.io, the new page will exist?

I think we can wait on updating the URL until after we get the catalog updated. It might not happen completely in sync, and a simple doc update change would be a fast PR.

You're referencing an appspot URL above in the article asset div; do you want to update that to match this?

kfranqueiro · 2018-05-17T15:55:54Z

packages/mdc-icon-button/README.md

+```
+### JavaScript Instantiation
+
+The icon button will work without Javascript, but you can enhance it to have a ripple effect by instantiating `MDCRipple` on the root element. 


Javascript -> JavaScript

kfranqueiro · 2018-05-17T16:05:12Z

demos/card.html

-                 tabindex="0"
-                 role="button"
+              </button>
+              <button class="material-icons mdc-icon-button mdc-card__action mdc-card__action--icon"


Nit: mdc-icon-button first everywhere

kfranqueiro · 2018-05-17T16:08:04Z

packages/mdc-icon-button/index.js

+
+  /** @return {!Element} */
+  get iconEl_() {
+    const {'iconInnerSelector': sel} = this.root_.dataset;


iconInnerSelector can be removed from here and tests; it's not even properly documented for icon-toggle and we've removed mention of nested elements. Either do it here or file a follow-up issue.

This is used to support font awesome. Without an inner icon element, font awesome wouldn't work correctly.

kfranqueiro · 2018-05-17T16:09:03Z

test/unit/mdc-icon-button/mdc-icon-toggle.test.js

@@ -21,7 +21,7 @@ import {assert} from 'chai';

 import {supportsCssVariables} from '../../../packages/mdc-ripple/util';
 import {createMockRaf} from '../helpers/raf';
-import {MDCIconToggle, MDCIconToggleFoundation} from '../../../packages/mdc-icon-toggle';
+import {MDCIconButtonToggle, MDCIconButtonToggleFoundation} from '../../../packages/mdc-icon-button';


Rename this file to mdc-icon-button-toggle.test.js?

kfranqueiro · 2018-05-17T16:15:38Z

packages/mdc-icon-button/foundation.js

+    this.isHandlingKeydown_ = false;
+
+    this.keydownHandler_ = /** @private {!EventListener} */ ((/** @type {!KeyboardKey} */ evt) => {
+      if (isSpace(evt)) {


Log removal of keyboard handling as a follow-up issue if you want. I think especially considering we now have both with-JS and no-JS variants of icon button, it makes more sense for the two to operate consistently anyway.

kfranqueiro · 2018-05-17T16:16:21Z

packages/mdc-icon-button/foundation.js

+   * @param {string} dataAttr
+   * @return {!IconToggleState}
+   */
+  parseJsonDataAttr_(dataAttr) {


Log the data attribute redesign as a follow-up issue... but we should really do that before merging this to master and publishing this package to avoid immediate breaking changes.

It should be possible for you to create a centralized branch for mdc-icon-button changes and edit this PR to target that branch.

kfranqueiro · 2018-05-17T19:48:46Z

packages/mdc-icon-button/foundation.js

+    super(Object.assign(MDCIconButtonToggleFoundation.defaultAdapter, adapter));
+
+    /** @private {boolean} */
+    /** @private {boolean} */


Duplicated annotation, FYI

kfranqueiro · 2018-05-18T18:01:39Z

packages/mdc-icon-button/_mixins.scss

+
+  display: inline-block;
+  position: relative;
+  align-items: center;


Remove align-items and justify-content since they no longer apply now that display is inline-block

kfranqueiro · 2018-05-18T18:03:04Z

packages/mdc-icon-button/_mixins.scss

+  }
+}
+
+@mixin mdc-icon-button-font-size($font-size) {


I'm still not sure I follow why we need this rather than just adding font-size to mdc-icon-button-size and making it all one mixin.

kfranqueiro · 2018-05-18T18:09:13Z

demos/icon-button.html

+            <div class="toggle-example">
+              <h2 class="mdc-typography--headline6">Using Material Icons</h2>
+              <div class="demo-wrapper">
+                <a href="#"


I'd suggest changing this to button like the rest in this section. It doesn't make sense for a link to be a toggle, because links navigate.

kfranqueiro · 2018-05-18T18:18:05Z

demos/theme/index.html

@@ -35,12 +35,12 @@
    <header class="mdc-toolbar mdc-toolbar--fixed demo-toolbar">
      <div class="mdc-toolbar__row">
        <section class="mdc-toolbar__section mdc-toolbar__section--align-start">
-          <button class="material-icons mdc-toolbar__menu-icon demo-drawer-toggle">menu</button>
+          <button class="material-icons mdc-toolbar__menu-icon demo-drawer-toggle mdc-icon-button" data-mdc-ripple-is-unbounded>menu</button>


I assume the data attribute here is needed specifically for the Ripple JS... we might want to add a note in the JS instantiation section of the README about this (and that it's not needed for Icon Toggle since that handles it itself).

This is used instead of .mdc-ripple-surface for just an icon-button. This is required for the Ripple to instantiate an unbounded ripple since we're using the same function to instantiate bounded and unbounded ripples.

kfranqueiro · 2018-05-18T18:22:44Z

packages/mdc-icon-button/README.md

+    <a href="https://material.io/design/components/buttons.html#toggle-button">Material Design guidelines: Toggle buttons</a>
+  </li>
+  <li class="icon-list-item icon-list-item--link">
+    <a href="https://material-components.github.io/material-components-web-catalog/#/component/icon-toggle">Demo</a>


You're referencing an appspot URL above in the article asset div; do you want to update that to match this?

kfranqueiro · 2018-05-18T18:25:06Z

packages/mdc-icon-button/README.md

+
+## Usage within Web Frameworks
+
+If you are using a JavaScript framework, such as React or Angular, you can create a Icon Toggle for your framework. Depending on your needs, you can use the _Simple Approach: Wrapping MDC Web Vanilla Components_, or the _Advanced Approach: Using Foundations and Adapters_. Please follow the instructions [here](../../docs/integrating-into-frameworks.md).


a -> an, Icon Toggle -> Icon Button Toggle

kfranqueiro · 2018-05-18T18:25:26Z

packages/mdc-icon-button/README.md

+
+If you are using a JavaScript framework, such as React or Angular, you can create a Icon Toggle for your framework. Depending on your needs, you can use the _Simple Approach: Wrapping MDC Web Vanilla Components_, or the _Advanced Approach: Using Foundations and Adapters_. Please follow the instructions [here](../../docs/integrating-into-frameworks.md).
+
+### `MDCIconToggleAdapter`


MDCIconButtonToggleAdapter

kfranqueiro · 2018-05-18T18:25:37Z

packages/mdc-icon-button/README.md

+`removeAttr(name: string) => void` | Removes the attribute `name` on the root element.
+`notifyChange(evtData: {isOn: boolean}) => void` | Broadcasts a change notification, passing along the `evtData` to the environment's event handling system. In our vanilla implementation, Custom Events are used for this.
+
+### Foundation: `MDCIconToggleFoundation`


MDCIconButtonToggleFoundation

kfranqueiro · 2018-05-18T18:26:56Z

packages/mdc-icon-button/_mixins.scss

+@mixin mdc-icon-button-base_() {
+  @include mdc-ripple-surface;
+  @include mdc-ripple-radius-unbounded;
+  @include mdc-icon-button-font-size(24px);


Reference variable here? Otherwise the variable is unused.

kfranqueiro · 2018-05-18T18:28:05Z

packages/mdc-icon-button/_variables.scss

+//
+
+$mdc-icon-button-size: 24px;
+$mdc-icon-button-padding: 12px;


This variable is unused because it's calculated in the default mixin parameter.

kfranqueiro · 2018-05-18T20:14:30Z

packages/mdc-icon-button/adapter.js

+/* eslint no-unused-vars: [2, {"args": "none"}] */
+
+/**
+ * Adapter for MDC Icon Toggle. Provides an interface for managing


MDC Icon Button Toggle

kfranqueiro · 2018-05-18T20:17:52Z

packages/mdc-icon-button/package.json

@@ -0,0 +1,25 @@
+{
+  "name": "@material/icon-button",
+  "description": "The Material Components for the icon button component",


missing "web" after "the"?

kfranqueiro · 2018-05-18T20:19:05Z

packages/mdc-icon-button/foundation.js

+  setDisabled(isDisabled) {
+    this.disabled_ = isDisabled;
+
+    const {ARIA_DISABLED} = MDCIconButtonToggleFoundation.strings;


We don't need to maintain aria-disabled anymore since we're stating that disabled state applies only to buttons directly, right? (Also affects tests, and probably renders this constant unused)

kfranqueiro · 2018-05-18T20:30:40Z

Can we add a deprecation notice to mdc-icon-toggle's readme in here? Perhaps we should also remove demo links from it in preparation for the demo being moved in catalog?

kfranqueiro · 2018-05-18T20:48:10Z

demos/icon-button.scss

+
+.demo-icon-button-large {
+  @include mdc-icon-button-font-size(36px);
+}


Missing newline at end of file

kfranqueiro · 2018-05-18T21:11:48Z

demos/icon-button.scss

  @include mdc-states-base-color(#de442c);
  @include mdc-states-hover-opacity(.09);
  @include mdc-states-focus-opacity(.26);
  @include mdc-states-press-opacity(.35);
 }
+
+.demo-icon-button-large {
+  @include mdc-icon-button-font-size(36px);


mdc-icon-button-size

kfranqueiro · 2018-05-21T14:29:23Z

test/unit/mdc-icon-button/mdc-icon-button-toggle.test.js

 import {MDCRipple} from '../../../packages/mdc-ripple';
 import {cssClasses} from '../../../packages/mdc-ripple/constants';

 function setupTest({tabIndex = undefined, useInnerIconElement = false} = {}) {
-  const root = document.createElement(useInnerIconElement ? 'span' : 'i');
+  const root = document.createElement(useInnerIconElement ? 'span' : 'button');


Shouldn't the outer element always be the button? I thought font-awesome required the inner icon element to be i.

kfranqueiro · 2018-05-21T14:29:52Z

demos/theme/index.html

+                   data-toggle-on-label="Remove from favorites"
+                   data-toggle-off-content="favorite_border"
+                   data-toggle-off-label="Add to favorites">
+                  favorite_borde


Typo: the last r got deleted

kfranqueiro · 2018-05-23T21:17:21Z

packages/material-components-web/README.md

-   aria-label="Add to favorites" tabindex="0"
-   data-toggle-on='{"label": "Remove from favorites", "content": "favorite"}'
-   data-toggle-off='{"label": "Add to favorites", "content": "favorite_border"}'
+<button class="mdc-icon-toggle material-icons" 


This still says mdc-icon-toggle; I think it should say mdc-icon-button?

Also, remove trailing space after favorite_border

kfranqueiro · 2018-05-23T21:19:42Z

packages/mdc-icon-button/README.md

+
+<!--<div class="article__asset">
+  <a class="article__asset-link"
+     href="https://material.io/design/components/buttons.html#toggle-button">


This is supposed to link to a demo, not a design article.

kfranqueiro · 2018-05-23T21:21:38Z

packages/mdc-icon-button/README.md

+--- | ---
+`data-toggle-<TOGGLE STATE>-label` | The value to apply to the element's "aria-label" attribute.
+`data-toggle-<TOGGLE STATE>-content` | The text content to set on the element. Note that if an inner icon is used, the text content will be set on that element instead.
+`data-toggle-<TOGGLE STATE>-class` | A css class to apply to the icon element. The same rules regarding inner icon elements described for `content` apply here as well.


Nit: capitalize CSS

kfranqueiro · 2018-05-23T21:22:53Z

packages/mdc-icon-button/README.md

+### Icons 
+
+The icon button can be used with a standard icon library or an `svg`. The icon button toggle should only be used with 
+an standard icon library. We recommend you use [Material Icons](https://material.io/icons/) from Google Fonts.


Update link to material.io/tools/icons to skip redirect

kfranqueiro · 2018-05-23T21:34:39Z

packages/mdc-icon-button/README.md

+Property | Value Type | Description
+--- | --- | ---
+`on` | Boolean | Sets the toggle state to the provided `isOn` value.
+`disabled` | Boolean | Sets the icon toggle to the `disabled` state.


AFAICT this no longer exists in the component and tests.

kfranqueiro · 2018-05-23T21:37:34Z

packages/mdc-icon-button/README.md

+```js
+import {MDCRipple} from '@material/ripple';
+
+const iconButtonRipple = new MDCRipple(document.querySelector('.mdc-icon-button'));


Should we include another line here for iconButtonRipple.unbounded = true in order to get the ripple to look correct?

williamernest added 9 commits April 27, 2018 13:55

WIP: New component

c90a152

WIP: New Component

2014853

WIP: Revert changes to package-lock.json

c219043

WIP:Temp

bd863c2

WIP: Adding button--icon to button package

a3f2411

WIP: Move icon button to it's own package

2379283

WIP: remove package-lock

ff6d902

Merge branch 'master' into feat/icon-button/add-new-package

7b6ce1e

feat(icon-toggle): Update tests, and various uses of icon toggle

3c65ab7

kfranqueiro self-requested a review May 16, 2018 18:59

kfranqueiro suggested changes May 16, 2018

View reviewed changes

feat(icon-button): Update for comments

da51dd9

williamernest changed the title ~~feat(icon button): Add new package~~ feat(icon-button): Add new package May 16, 2018

feat(icon-button): Update for comments. Replace theme uses. Add demo

02192df

kfranqueiro suggested changes May 16, 2018

View reviewed changes

feat(icon-button): Update for comments

21e3b83

trimox mentioned this pull request May 17, 2018

Add MDC Icon Button trimox/angular-mdc-web#984

Closed

kfranqueiro reviewed May 17, 2018

View reviewed changes

williamernest mentioned this pull request May 17, 2018

Make new icon-button demo page from 0.36.0-0 prerelease material-components/material-components-web-catalog#156

Closed

4 tasks

kfranqueiro suggested changes May 17, 2018

View reviewed changes

feat(icon-button): Update for comments

a3254d3

kfranqueiro reviewed May 17, 2018

View reviewed changes

williamernest and others added 5 commits May 17, 2018 13:19

feat(icon-button): Revert theme style changes

a27180a

feat(icon-button): Update for comments

b9d1890

feat(icon-button): Update tests

d16bd67

WIP Add common.js to avoid # jumping to top

b2f11f0

feat(icon-button): Change icon button to display inline block

23cccd7

kfranqueiro suggested changes May 18, 2018

View reviewed changes

feat(icon-button): Update for comments

8c99a1a

kfranqueiro suggested changes May 18, 2018

View reviewed changes

kfranqueiro reviewed May 18, 2018

View reviewed changes

williamernest added 2 commits May 18, 2018 15:06

WIP: Update for comments

27c8ae9

Merge branch 'master' into feat/icon-button/add-new-package

c7436f5

kfranqueiro suggested changes May 21, 2018

View reviewed changes

williamernest and others added 2 commits May 21, 2018 08:42

feat(icon-button): Update for commentS

1a12ff3

Update

9fae22c

kfranqueiro mentioned this pull request May 21, 2018

FAB: Material icon font not vertically centered in IE 11...sometimes? #2752

Closed

williamernest added 2 commits May 22, 2018 11:25

feat(icon-button): Remove trailing whitespace

ac281de

feat(icon-button): Update

d005b41

kfranqueiro suggested changes May 23, 2018

View reviewed changes

williamernest and others added 4 commits May 23, 2018 15:28

feat(icon-button): Update for comments

0c1a72a

feat(icon-button): Update

28307a6

feat(icon-button): Update

945d7a6

WIP Update a couple more icon-toggle references in docs

e09096e

kfranqueiro approved these changes May 24, 2018

View reviewed changes

kfranqueiro and others added 3 commits May 24, 2018 11:39

Merge branch 'master' into feat/icon-button/add-new-package

8721628

Merge branch 'master' into feat/icon-button/add-new-package

49b3be3

Merge branch 'master' into feat/icon-button/add-new-package

2a95b2c

williamernest merged commit 39a4815 into master May 25, 2018

williamernest deleted the feat/icon-button/add-new-package branch May 25, 2018 16:40

williamernest mentioned this pull request May 25, 2018

Card: Icon ripples are initialized with wrong size if material-icons font loads after scripts #2702

Closed

kfranqueiro mentioned this pull request Jul 17, 2018

feat(icon-toggle): Added support for SVG icons #1898

Closed


		## Usage within Web Frameworks

		If you are using a JavaScript framework, such as React or Angular, you can create a Icon Toggle for your framework. Depending on your needs, you can use the _Simple Approach: Wrapping MDC Web Vanilla Components_, or the _Advanced Approach: Using Foundations and Adapters_. Please follow the instructions [here](../../docs/integrating-into-frameworks.md).


		If you are using a JavaScript framework, such as React or Angular, you can create a Icon Toggle for your framework. Depending on your needs, you can use the _Simple Approach: Wrapping MDC Web Vanilla Components_, or the _Advanced Approach: Using Foundations and Adapters_. Please follow the instructions [here](../../docs/integrating-into-frameworks.md).

		### `MDCIconToggleAdapter`

feat(icon-button): Add new package #2748

feat(icon-button): Add new package #2748

Conversation

williamernest commented May 16, 2018

codecov-io commented May 16, 2018 • edited Loading

Codecov Report

kfranqueiro 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

kfranqueiro 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

kfranqueiro May 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kfranqueiro commented May 17, 2018 • 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

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

williamernest May 17, 2018 • 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

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

kfranqueiro commented May 18, 2018

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

codecov-io commented May 16, 2018 •

edited

Loading

kfranqueiro May 17, 2018 •

edited

Loading

kfranqueiro commented May 17, 2018 •

edited

Loading

williamernest May 17, 2018 •

edited

Loading