From 9c9ad82670512ec36fce110aaa5d8b57d52035c9 Mon Sep 17 00:00:00 2001 From: Travis Kaufman Date: Wed, 11 Jan 2017 10:42:32 -0500 Subject: [PATCH] fix(ripple): Use correct start point for unbounded ripple expansion (#165) Fixes a bug where the start point for the foreground unbounded ripple would emanate from a different point than where the surface was interacted with. This was occurring because the positioning offsets were not being taken into account when computing this point. Fixes #138 [Delivers #132116569] --- packages/mdc-ripple/foundation.js | 18 ++++++++++++------ .../mdc-ripple/foundation-activation.test.js | 15 ++++++++------- test/unit/mdc-ripple/foundation.test.js | 8 ++++---- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/packages/mdc-ripple/foundation.js b/packages/mdc-ripple/foundation.js index a9aefa8f0a3..f4a30446bba 100644 --- a/packages/mdc-ripple/foundation.js +++ b/packages/mdc-ripple/foundation.js @@ -95,6 +95,10 @@ export default class MDCRippleFoundation extends MDCFoundation { this.cancelBgBounded_ = () => {}; this.cancelFgBounded_ = () => {}; this.cancelFgUnbounded_ = () => {}; + this.unboundedCoords_ = { + left: 0, + top: 0, + }; } defaultActivationState_() { @@ -203,8 +207,8 @@ export default class MDCRippleFoundation extends MDCFoundation { } const {left, top} = startPoint; const {VAR_XF_ORIGIN_X, VAR_XF_ORIGIN_Y} = MDCRippleFoundation.strings; - this.adapter_.updateCssVariable(VAR_XF_ORIGIN_X, `${left}px`); - this.adapter_.updateCssVariable(VAR_XF_ORIGIN_Y, `${top}px`); + this.adapter_.updateCssVariable(VAR_XF_ORIGIN_X, `${left - this.unboundedCoords_.left}px`); + this.adapter_.updateCssVariable(VAR_XF_ORIGIN_Y, `${top - this.unboundedCoords_.top}px`); this.adapter_.addClass(FG_UNBOUNDED_ACTIVATION); } @@ -384,10 +388,12 @@ export default class MDCRippleFoundation extends MDCFoundation { this.adapter_.updateCssVariable(VAR_FG_UNBOUNDED_TRANSFORM_DURATION, `${this.xfDuration_}ms`); if (this.adapter_.isUnbounded()) { - const left = -(fgSize / 2) + (this.frame_.width / 2); - const top = -(fgSize / 2) + (this.frame_.height / 2); - this.adapter_.updateCssVariable(VAR_LEFT, `${left}px`); - this.adapter_.updateCssVariable(VAR_TOP, `${top}px`); + this.unboundedCoords_ = { + left: Math.round(-(fgSize / 2) + (this.frame_.width / 2)), + top: Math.round(-(fgSize / 2) + (this.frame_.height / 2)), + }; + this.adapter_.updateCssVariable(VAR_LEFT, `${this.unboundedCoords_.left}px`); + this.adapter_.updateCssVariable(VAR_TOP, `${this.unboundedCoords_.top}px`); } } } diff --git a/test/unit/mdc-ripple/foundation-activation.test.js b/test/unit/mdc-ripple/foundation-activation.test.js index 049b2acf782..f3cc0758502 100644 --- a/test/unit/mdc-ripple/foundation-activation.test.js +++ b/test/unit/mdc-ripple/foundation-activation.test.js @@ -131,7 +131,8 @@ testFoundation('displays the foreground ripple on activation when unbounded', (t t.end(); }); -testFoundation('sets unbounded FG xf origin to the coords within surface on pointer activation', (t) => { +testFoundation('sets unbounded FG xf origin to the coords within surface on pointer activation, ' + + 'accounting for FG ripple offset', (t) => { const {foundation, adapter, mockRaf} = t.data; const handlers = captureHandlers(adapter); td.when(adapter.computeBoundingRect()).thenReturn({width: 100, height: 100, left: 50, top: 50}); @@ -142,8 +143,8 @@ testFoundation('sets unbounded FG xf origin to the coords within surface on poin handlers.mousedown({pageX: 100, pageY: 75}); mockRaf.flush(); - t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_XF_ORIGIN_X, '50px'))); - t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_XF_ORIGIN_Y, '25px'))); + t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_XF_ORIGIN_X, '71px'))); + t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_XF_ORIGIN_Y, '46px'))); t.end(); }); @@ -159,8 +160,8 @@ testFoundation('takes scroll offset into account when computing transform origin handlers.mousedown({pageX: 100, pageY: 75}); mockRaf.flush(); - t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_XF_ORIGIN_X, '50px'))); - t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_XF_ORIGIN_Y, '25px'))); + t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_XF_ORIGIN_X, '71px'))); + t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_XF_ORIGIN_Y, '46px'))); t.end(); }); @@ -176,7 +177,7 @@ testFoundation('sets unbounded FG xf origin to center on non-pointer activation' handlers.keydown(); mockRaf.flush(); - t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_XF_ORIGIN_X, '50px'))); - t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_XF_ORIGIN_Y, '50px'))); + t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_XF_ORIGIN_X, '71px'))); + t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_XF_ORIGIN_Y, '71px'))); t.end(); }); diff --git a/test/unit/mdc-ripple/foundation.test.js b/test/unit/mdc-ripple/foundation.test.js index f8ea00cd6a8..d8aac24a2a4 100644 --- a/test/unit/mdc-ripple/foundation.test.js +++ b/test/unit/mdc-ripple/foundation.test.js @@ -144,8 +144,8 @@ testFoundation(`#init centers via ${strings.VAR_LEFT} and ${strings.VAR_TOP} whe const expectedDiameter = Math.sqrt(2) * 200; const offset = (expectedDiameter / 2); - t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_LEFT, `${-offset + 50}px`))); - t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_TOP, `${-offset + 100}px`))); + t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_LEFT, `${Math.round(-offset + 50)}px`))); + t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_TOP, `${Math.round(-offset + 100)}px`))); t.end(); }); @@ -286,8 +286,8 @@ testFoundation(`#layout centers via ${strings.VAR_LEFT} and ${strings.VAR_TOP} w const expectedDiameter = Math.sqrt(2) * 200; const offset = (expectedDiameter / 2); - t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_LEFT, `${-offset + 50}px`))); - t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_TOP, `${-offset + 100}px`))); + t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_LEFT, `${Math.round(-offset + 50)}px`))); + t.doesNotThrow(() => td.verify(adapter.updateCssVariable(strings.VAR_TOP, `${Math.round(-offset + 100)}px`))); t.end(); });