Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

fix(ripple): Ripple should not activate on disabled label click #532

Merged
merged 6 commits into from
May 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions demos/checkbox.html
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ <h2>With Javascript</h2>
</div>
<button type="button" id="make-ind">Make indeterminate</button>
</section>

<section class="example mdc-theme--dark">
<h2>Dark Theme</h2>
<div class="mdc-form-field">
Expand Down Expand Up @@ -192,6 +193,9 @@ <h2>Dark Theme</h2>
var checkbox = document.getElementById('mdc-js-checkbox');
var checkboxInstance = new MDCCheckbox(checkbox);

var disabledCheckbox = document.getElementById('mdc-disabled-js-checkbox');
var disabledCheckboxInstance = new MDCCheckbox(disabledCheckbox);

var formField = checkbox.parentElement;
var formFieldInstance = new MDCFormField(formField);
formFieldInstance.input = checkboxInstance;
Expand Down
1 change: 1 addition & 0 deletions packages/mdc-checkbox/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ The adapter for checkboxes must provide the following functions, with correct si
| `forceLayout() => void` | Force-trigger a layout on the root element. This is needed to restart animations correctly. If you find that you do not need to do this, you can simply make it a no-op. |
| `isAttachedToDOM() => boolean` | Returns true if the component is currently attached to the DOM, false otherwise.` |


#### MDCCheckboxFoundation API

##### MDCCheckboxFoundation.isChecked() => boolean
Expand Down
1 change: 1 addition & 0 deletions packages/mdc-radio/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ Since MDC Radio is primarily driven by its native control, the adapter API is ex
| `addClass(className: string) => void` | Adds a class to the root element. |
| `removeClass(className: string) => void` | Removes a class from the root element. |


#### The full foundation API

##### MDCRadioFoundation.isChecked() => boolean
Expand Down
1 change: 1 addition & 0 deletions packages/mdc-ripple/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ ripple to. The adapter API is as follows:
| `browserSupportsCssVars() => boolean` | Whether or not the given browser supports CSS Variables. When implementing this, please take the [Safari considerations](#caveat-safari) into account. We provide a `supportsCssVariables` function within the `util.js` which we recommend using, as it handles this for you. |
| `isUnbounded() => boolean` | Whether or not the ripple should be considered unbounded. |
| `isSurfaceActive() => boolean` | Whether or not the surface the ripple is acting upon is [active](https://www.w3.org/TR/css3-selectors/#useraction-pseudos). We use this to detect whether or not a keyboard event has activated the surface the ripple is on. This does not need to make use of `:active` (which is what we do); feel free to supply your own heuristics for it. |
| `isSurfaceDisabled() => boolean` | Whether or not the ripple is attached to a disabled component. If true, the ripple will not activate. |
| `addClass(className: string) => void` | Adds a class to the ripple surface |
| `removeClass(className: string) => void` | Removes a class from the ripple surface |
| `registerInteractionHandler(evtType: string, handler: EventListener) => void` | Registers an event handler that's invoked when the ripple is interacted with using type `evtType`. Essentially equivalent to `HTMLElement.prototype.addEventListener`. |
Expand Down
6 changes: 6 additions & 0 deletions packages/mdc-ripple/foundation.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export default class MDCRippleFoundation extends MDCFoundation {
browserSupportsCssVars: () => /* boolean - cached */ {},
isUnbounded: () => /* boolean */ {},
isSurfaceActive: () => /* boolean */ {},
isSurfaceDisabled: () => /* boolean */ {},
addClass: (/* className: string */) => {},
removeClass: (/* className: string */) => {},
registerInteractionHandler: (/* evtType: string, handler: EventListener */) => {},
Expand All @@ -67,6 +68,7 @@ export default class MDCRippleFoundation extends MDCFoundation {

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

this.layoutFrame_ = 0;
this.frame_ = {width: 0, height: 0};
this.activationState_ = this.defaultActivationState_();
Expand Down Expand Up @@ -142,6 +144,10 @@ export default class MDCRippleFoundation extends MDCFoundation {
}

activate_(e) {
if (this.adapter_.isSurfaceDisabled()) {
return;
}

const {activationState_: activationState} = this;
if (activationState.isActivated) {
return;
Expand Down
1 change: 1 addition & 0 deletions packages/mdc-ripple/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export class MDCRipple extends MDCComponent {
browserSupportsCssVars: () => supportsCssVariables(window),
isUnbounded: () => instance.unbounded,
isSurfaceActive: () => instance.root_[MATCHES](':active'),
isSurfaceDisabled: () => instance.disabled,
addClass: (className) => instance.root_.classList.add(className),
removeClass: (className) => instance.root_.classList.remove(className),
registerInteractionHandler: (evtType, handler) => instance.root_.addEventListener(evtType, handler),
Expand Down
14 changes: 14 additions & 0 deletions test/unit/mdc-ripple/foundation-activation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,20 @@ import {cssClasses, strings, numbers} from '../../../packages/mdc-ripple/constan

suite('MDCRippleFoundation - Activation Logic');

testFoundation('does nothing if component if isSurfaceDisabled is true',
({foundation, adapter, mockRaf}) => {
const handlers = captureHandlers(adapter);
foundation.init();
mockRaf.flush();

td.when(adapter.isSurfaceDisabled()).thenReturn(true);

handlers.mousedown();

td.verify(adapter.addClass(cssClasses.BG_ACTIVE_FILL), {times: 0});
td.verify(adapter.addClass(cssClasses.FG_ACTIVATION), {times: 0});
});

testFoundation('adds activation classes on mousedown', ({foundation, adapter, mockRaf}) => {
const handlers = captureHandlers(adapter);
foundation.init();
Expand Down
7 changes: 4 additions & 3 deletions test/unit/mdc-ripple/foundation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ test('numbers returns constants.numbers', () => {

test('defaultAdapter returns a complete adapter implementation', () => {
verifyDefaultAdapter(MDCRippleFoundation, [
'browserSupportsCssVars', 'isUnbounded', 'isSurfaceActive', 'addClass', 'removeClass',
'registerInteractionHandler', 'deregisterInteractionHandler', 'registerResizeHandler',
'deregisterResizeHandler', 'updateCssVariable', 'computeBoundingRect', 'getWindowPageOffset',
'browserSupportsCssVars', 'isUnbounded', 'isSurfaceActive', 'isSurfaceDisabled',
'addClass', 'removeClass', 'registerInteractionHandler', 'deregisterInteractionHandler',
'registerResizeHandler', 'deregisterResizeHandler', 'updateCssVariable',
'computeBoundingRect', 'getWindowPageOffset',
]);
});

Expand Down
6 changes: 6 additions & 0 deletions test/unit/mdc-ripple/mdc-ripple.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,12 @@ test('adapter#isSurfaceActive calls the correct :matches API method on the root
assert.isOk(component.getDefaultFoundation().adapter_.isSurfaceActive());
});

test('adapter#isSurfaceDisabled delegates to component\'s disabled getter', () => {
const {component} = setupTest();
component.disabled = true;
assert.isTrue(component.getDefaultFoundation().adapter_.isSurfaceDisabled());
});

test('adapter#addClass adds a class to the root', () => {
const {root, component} = setupTest();
component.getDefaultFoundation().adapter_.addClass('foo');
Expand Down