Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ripple): Implement subset of improved interaction response guide… #363

Closed
wants to merge 16 commits into from
Closed

feat(ripple): Implement subset of improved interaction response guide… #363

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
7eceb51
feat(ripple): Implement subset of improved interaction response guide…
Feb 19, 2017
7911662
Merge branch 'master' into feat/implement-improved-interaction-guidel…
traviskaufman Mar 2, 2017
1cc19ac
feat(ripple): Implement subset of improved interaction response guide…
Feb 19, 2017
ef7a680
feat(ripple): Implement subset of improved interaction response guide…
Feb 19, 2017
94b5edf
feat(ripple): Implement subset of improved interaction response guide…
Feb 19, 2017
645e58f
feat(ripple): Implement subset of improved interaction response guide…
Feb 19, 2017
d876879
test(infrastructure): More test coverage (#368)
BBosman Mar 2, 2017
e6d6064
docs(checkbox): Fix indeterminate button in JS checkbox demo (#374)
traviskaufman Mar 3, 2017
9bf03c9
fix(checkbox): Ensure correct positioning in RTL context (#381)
traviskaufman Mar 4, 2017
2a1df0d
chore(infrastructure): Publish
traviskaufman Mar 6, 2017
5ae91bd
docs(infrastructure): Update CHANGELOG.md
traviskaufman Mar 6, 2017
008f39b
docs(infrastructure): Fixup Changelog
traviskaufman Mar 6, 2017
b760901
chore(infrastructure): Update release scripts and docs (#384)
traviskaufman Mar 7, 2017
c6970ad
fix(infrastructure): Update publishConfig.ignore to commands.publish.…
yeelan0319 Mar 7, 2017
e1683f3
feat(grid-list): Implement mdc-grid-list (#47) (#359)
yeelan0319 Mar 10, 2017
e2beadf
feat(ripple): Implement subset of improved interaction response guide…
Feb 19, 2017
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
35 changes: 29 additions & 6 deletions packages/mdc-ripple/foundation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ const DEACTIVATION_ACTIVATION_PAIRS = {
blur: 'focus',
};

const DEACTIVATION_TIMEOUT_MS = 200;

export default class MDCRippleFoundation extends MDCFoundation {
static get cssClasses() {
return cssClasses;
Expand Down Expand Up @@ -110,6 +112,9 @@ export default class MDCRippleFoundation extends MDCFoundation {
wasElementMadeActive: false,
activationStartTime: 0,
activationEvent: null,
isProgrammatic: false,
isTimeoutDeactivated: false,
deactivationTimeout: null,
};
}

Expand Down Expand Up @@ -146,12 +151,20 @@ export default class MDCRippleFoundation extends MDCFoundation {

activationState.isActivated = true;
activationState.isProgrammatic = e === null;
activationState.isTimeoutDeactivated = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this, as well as isProgrammatic, need to be added to the return value of defaultActivationState_()

activationState.activationEvent = e;
activationState.wasActivatedByPointer = activationState.isProgrammatic ? false : (
e.type === 'mousedown' || e.type === 'touchstart' || e.type === 'pointerdown'
);

activationState.activationStartTime = Date.now();

activationState.deactivationTimeout = setTimeout(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment as above

activationState.isTimeoutDeactivated = true;
this.deactivate_(e);
activationState.isActivated = false;
}, DEACTIVATION_TIMEOUT_MS);

requestAnimationFrame(() => {
// This needs to be wrapped in an rAF call b/c web browsers
// report active states inconsistently when they're called within
Expand Down Expand Up @@ -215,16 +228,18 @@ export default class MDCRippleFoundation extends MDCFoundation {
this.activationState_ = this.defaultActivationState_();
return;
}

const actualActivationType = DEACTIVATION_ACTIVATION_PAIRS[e.type];
const expectedActivationType = activationState.activationEvent.type;
// NOTE: Pointer events are tricky - https://patrickhlauke.github.io/touch/tests/results/
// Essentially, what we need to do here is decouple the deactivation UX from the actual
// deactivation state itself. This way, touch/pointer events in sequence do not trample one
// another.
const needsDeactivationUX = actualActivationType === expectedActivationType;
const needsDeactivationUX = actualActivationType === expectedActivationType ||
activationState.isTimeoutDeactivated;
let needsActualDeactivation = needsDeactivationUX;
if (activationState.wasActivatedByPointer) {
needsActualDeactivation = e.type === 'mouseup';
needsActualDeactivation = activationState.isTimeoutDeactivated || e.type === 'mouseup';
}

const state = Object.assign({}, activationState);
Expand All @@ -234,22 +249,30 @@ export default class MDCRippleFoundation extends MDCFoundation {
if (needsActualDeactivation) {
this.activationState_ = this.defaultActivationState_();
}

clearTimeout(activationState.deactivationTimeout);
}

deactivate() {
this.deactivate_(null);
}

animateDeactivation_(e, {wasActivatedByPointer, wasElementMadeActive, activationStartTime, isProgrammatic}) {
animateDeactivation_(e, {wasActivatedByPointer, wasElementMadeActive, activationStartTime, isProgrammatic,
isTimeoutDeactivated}) {
const {BG_ACTIVE} = MDCRippleFoundation.cssClasses;
if (wasActivatedByPointer || wasElementMadeActive) {
this.adapter_.removeClass(BG_ACTIVE);
const isPointerEvent = isProgrammatic ? false : (
e.type === 'touchend' || e.type === 'pointerup' || e.type === 'mouseup'
);


if (this.adapter_.isUnbounded()) {
this.animateUnboundedDeactivation_(this.getUnboundedDeactivationInfo_(activationStartTime));
} else {
const isPointerEventOnDeactivation = isTimeoutDeactivated &&
(e.type === 'touchstart' || e.type === 'pointerdown' || e.type === 'mousedown');

const isPointerEvent = isProgrammatic ? false : (
e.type === 'touchend' || e.type === 'pointerup' || e.type === 'mouseup' || isPointerEventOnDeactivation
);
this.animateBoundedDeactivation_(e, isPointerEvent);
}
}
Expand Down
37 changes: 37 additions & 0 deletions test/unit/mdc-ripple/foundation-deactivation.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,43 @@ testFoundation('runs deactivation UX on public deactivate() call', ({foundation,
td.verify(adapter.addClass(cssClasses.FG_BOUNDED_ACTIVE_FILL));
});

testFoundation('runs deactivation UX mousedown pressed for longer than deactivation timeout',
Copy link
Contributor

Choose a reason for hiding this comment

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

so for both of these, the tests will actually exit before the setTimeout() code is executed. This is because the tests execute synchronously, so by the time that setTimeout code runs, the test will be finished and any exceptions will be swallowed.

To prevent this, you can use lolex, which we use in our codebase to mock JS timers. You can see examples of this in the following tests:

({foundation, adapter, mockRaf}) => {
const handlers = captureHandlers(adapter);
foundation.init();
mockRaf.flush();

handlers.mousedown();
mockRaf.flush();

setTimeout(() => {
td.verify(adapter.removeClass(cssClasses.BG_ACTIVE));
td.verify(adapter.addClass(cssClasses.BG_BOUNDED_ACTIVE_FILL));
td.verify(adapter.addClass(cssClasses.FG_BOUNDED_ACTIVE_FILL));
}, foundation.DEACTIVATION_TIMEOUT_MS);
});

testFoundation('does not run deactivation UX on mouseup after mousedown pressed for longer than deactivation timeout',
({foundation, adapter, mockRaf}) => {
const handlers = captureHandlers(adapter);
foundation.init();
mockRaf.flush();

handlers.mousedown();
mockRaf.flush();

setTimeout(() => {
td.verify(adapter.removeClass(cssClasses.BG_ACTIVE));
Copy link
Contributor

Choose a reason for hiding this comment

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

Not exactly sure how these tests are different from the ones above...

Should this be verifying that these adapter methods are not called before DEACTIVATION_TIMEOUT_MS?

td.verify(adapter.addClass(cssClasses.BG_BOUNDED_ACTIVE_FILL));
td.verify(adapter.addClass(cssClasses.FG_BOUNDED_ACTIVE_FILL));
handlers.mouseup();
mockRaf.flush();
td.verify(adapter.removeClass(cssClasses.BG_ACTIVE));
td.verify(adapter.addClass(cssClasses.BG_BOUNDED_ACTIVE_FILL));
td.verify(adapter.addClass(cssClasses.FG_BOUNDED_ACTIVE_FILL));
}, foundation.DEACTIVATION_TIMEOUT_MS);
});

testFoundation('only re-activates when there are no additional pointer events to be processed',
({foundation, adapter, mockRaf}) => {
const handlers = captureHandlers(adapter);
Expand Down