Skip to content

Commit

Permalink
fix(icon-button): Remove unused styles, update docs, code cleanup (#2957
Browse files Browse the repository at this point in the history
)

Fixes a bug with the foundation always being initialized "on".
  • Loading branch information
williamernest authored Jun 20, 2018
1 parent 01abc11 commit 32b5b9d
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 51 deletions.
24 changes: 21 additions & 3 deletions packages/mdc-icon-button/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ npm install @material/icon-button
```scss
@import "@material/icon-button/mdc-icon-button";
```

### 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.
Expand Down Expand Up @@ -81,7 +82,7 @@ The icon button can be used to toggle between an on and off icon. To style an ic
data-toggle-on-content="favorite"
data-toggle-on-label="Remove from favorites"
data-toggle-off-content="favorite_border"
data-toggle-off-label="Add to favorites">favorites_border</button>
data-toggle-off-label="Add to favorites">favorite_border</button>
```

```js
Expand All @@ -101,10 +102,27 @@ Attribute | Description
`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.

#### Icon Button Toggle with Font Awesome

The icon button toggle can be used with other font libraries such as Font Awesome that use an inner icon element.

```html
<button id="star-this-item"
class="mdc-icon-button"
aria-label="Unstar this item"
aria-hidden="true"
aria-pressed="true"
data-toggle-on-class="fa-star"
data-toggle-on-label="Unstar this item"
data-toggle-off-class="fa-star-o"
data-toggle-off-label="Star this item"><i class="fa fa-2x fa-star"></i></button>
```

### 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/tools/icons) from Google Fonts.
The icon button can be used with a standard icon library such as Material Icons or Font Awesome, or with an `svg`.
The icon button toggle should only be used with an standard icon library. We recommend you use
[Material Icons](https://material.io/tools/icons) from Google Fonts.

### Disabled

Expand Down
9 changes: 0 additions & 9 deletions packages/mdc-icon-button/adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,6 @@ class MDCIconButtonToggleAdapter {
/** @param {string} text */
setText(text) {}

/** @return {number} */
getTabIndex() {}

/** @param {number} tabIndex */
setTabIndex(tabIndex) {}

/**
* @param {string} name
* @return {string}
Expand All @@ -76,9 +70,6 @@ class MDCIconButtonToggleAdapter {
*/
setAttr(name, value) {}

/** @param {string} name */
removeAttr(name) {}

/** @param {!IconButtonToggleEvent} evtData */
notifyChange(evtData) {}
}
Expand Down
11 changes: 3 additions & 8 deletions packages/mdc-icon-button/foundation.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,27 +39,23 @@ class MDCIconButtonToggleFoundation extends MDCFoundation {
registerInteractionHandler: (/* type: string, handler: EventListener */) => {},
deregisterInteractionHandler: (/* type: string, handler: EventListener */) => {},
setText: (/* text: string */) => {},
getTabIndex: () => /* number */ 0,
setTabIndex: (/* tabIndex: number */) => {},
getAttr: (/* name: string */) => /* string */ '',
setAttr: (/* name: string, value: string */) => {},
removeAttr: (/* name: string */) => {},
notifyChange: (/* evtData: IconButtonToggleEvent */) => {},
};
}

constructor(adapter) {
super(Object.assign(MDCIconButtonToggleFoundation.defaultAdapter, adapter));

const {ARIA_PRESSED} = MDCIconButtonToggleFoundation.strings;

/** @private {boolean} */
this.on_ = false;
this.on_ = this.adapter_.getAttr(ARIA_PRESSED) === 'true';

/** @private {boolean} */
this.disabled_ = false;

/** @private {number} */
this.savedTabIndex_ = -1;

/** @private {?IconButtonToggleState} */
this.toggleOnData_ = null;

Expand All @@ -72,7 +68,6 @@ class MDCIconButtonToggleFoundation extends MDCFoundation {

init() {
this.refreshToggleData();
this.savedTabIndex_ = this.adapter_.getTabIndex();
this.adapter_.registerInteractionHandler('click', this.clickHandler_);
}

Expand Down
5 changes: 1 addition & 4 deletions packages/mdc-icon-button/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,8 @@ class MDCIconButtonToggle extends MDCComponent {
registerInteractionHandler: (type, handler) => this.root_.addEventListener(type, handler),
deregisterInteractionHandler: (type, handler) => this.root_.removeEventListener(type, handler),
setText: (text) => this.iconEl_.textContent = text,
getTabIndex: () => /* number */ this.root_.tabIndex,
setTabIndex: (tabIndex) => this.root_.tabIndex = tabIndex,
getAttr: (name, value) => this.root_.getAttribute(name, value),
getAttr: (name) => this.root_.getAttribute(name),
setAttr: (name, value) => this.root_.setAttribute(name, value),
removeAttr: (name) => this.root_.removeAttribute(name),
notifyChange: (evtData) => this.emit(MDCIconButtonToggleFoundation.strings.CHANGE_EVENT, evtData),
});
}
Expand Down
6 changes: 0 additions & 6 deletions packages/mdc-icon-button/mdc-icon-button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,4 @@
@include mdc-states;
}

.mdc-icon-button--disabled {
@include mdc-theme-prop(color, text-disabled-on-light);

pointer-events: none;
}

// postcss-bem-linter: end
15 changes: 12 additions & 3 deletions test/unit/mdc-icon-button/foundation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,24 @@ test('exports cssClasses', () => {
test('defaultAdapter returns a complete adapter implementation', () => {
verifyDefaultAdapter(MDCIconButtonToggleFoundation, [
'addClass', 'removeClass', 'registerInteractionHandler', 'deregisterInteractionHandler',
'setText', 'getTabIndex', 'setTabIndex', 'getAttr', 'setAttr', 'removeAttr', 'notifyChange',
'setText', 'getAttr', 'setAttr', 'notifyChange',
]);
});

const setupTest = () => setupFoundationTest(MDCIconButtonToggleFoundation);

test('#constructor sets on to false', () => {
const {foundation} = setupTest();
assert.isNotOk(foundation.isOn());
const {mockAdapter} = setupTest();
td.when(mockAdapter.getAttr(strings.ARIA_PRESSED)).thenReturn('false');
const foundation = new MDCIconButtonToggleFoundation(mockAdapter);
assert.isFalse(foundation.isOn());
});

test('#constructor sets on to true if the toggle is pressed', () => {
const {mockAdapter} = setupTest();
td.when(mockAdapter.getAttr(strings.ARIA_PRESSED)).thenReturn('true');
const foundation = new MDCIconButtonToggleFoundation(mockAdapter);
assert.isTrue(foundation.isOn());
});

test('#toggle flips on', () => {
Expand Down
18 changes: 0 additions & 18 deletions test/unit/mdc-icon-button/mdc-icon-button-toggle.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,17 +158,6 @@ test('#adapter.setText sets the text content of the inner icon element when used
assert.equal(root.querySelector('#icon').textContent, 'foo');
});

test('#adapter.getTabIndex returns the tabIndex of the element', () => {
const {component} = setupTest({tabIndex: 4});
assert.equal(component.getDefaultFoundation().adapter_.getTabIndex(), 4);
});

test('#adapter.setTabIndex sets the tabIndex of the element', () => {
const {root, component} = setupTest({tabIndex: 4});
component.getDefaultFoundation().adapter_.setTabIndex(2);
assert.equal(root.tabIndex, 2);
});

test('#adapter.getAttr retrieves an attribute from the root element', () => {
const {root, component} = setupTest();
root.setAttribute('aria-label', 'hello');
Expand All @@ -181,13 +170,6 @@ test('#adapter.setAttr sets an attribute on the root element', () => {
assert.equal(root.getAttribute('aria-label'), 'hello');
});

test('#adapter.removeAttr removes an attribute from the root element', () => {
const {root, component} = setupTest();
root.setAttribute('aria-label', 'hello');
component.getDefaultFoundation().adapter_.removeAttr('aria-label');
assert.isNotOk(root.hasAttribute('aria-label'));
});

test(`#adapter.notifyChange broadcasts a ${MDCIconButtonToggleFoundation.strings.CHANGE_EVENT} custom event`, () => {
const {root, component} = setupTest();
const handler = td.func('custom event handler');
Expand Down

0 comments on commit 32b5b9d

Please sign in to comment.