From ffa9bd398a3c720df9c35b56f8c0cd7768f66323 Mon Sep 17 00:00:00 2001 From: "Kenneth G. Franqueiro" Date: Fri, 13 Apr 2018 12:54:06 -0400 Subject: [PATCH 1/4] fix(ripple): Call layout on each activation instead of at init Unbounded still needs layout called immediately to compute its position. --- packages/mdc-ripple/foundation.js | 5 ++++- test/unit/mdc-ripple/foundation.test.js | 29 ------------------------- 2 files changed, 4 insertions(+), 30 deletions(-) diff --git a/packages/mdc-ripple/foundation.js b/packages/mdc-ripple/foundation.js index 60045c8dc2e..54f03dc4cfd 100644 --- a/packages/mdc-ripple/foundation.js +++ b/packages/mdc-ripple/foundation.js @@ -209,8 +209,9 @@ class MDCRippleFoundation extends MDCFoundation { this.adapter_.addClass(ROOT); if (this.adapter_.isUnbounded()) { this.adapter_.addClass(UNBOUNDED); + // Unbounded ripples need layout logic applied immediately to set coordinates for both shade and ripple + this.layoutInternal_(); } - this.layoutInternal_(); }); } @@ -380,6 +381,8 @@ class MDCRippleFoundation extends MDCFoundation { const {FG_DEACTIVATION, FG_ACTIVATION} = MDCRippleFoundation.cssClasses; const {DEACTIVATION_TIMEOUT_MS} = MDCRippleFoundation.numbers; + this.layoutInternal_(); + let translateStart = ''; let translateEnd = ''; diff --git a/test/unit/mdc-ripple/foundation.test.js b/test/unit/mdc-ripple/foundation.test.js index e6522f7f046..a88079811ea 100644 --- a/test/unit/mdc-ripple/foundation.test.js +++ b/test/unit/mdc-ripple/foundation.test.js @@ -78,35 +78,6 @@ testFoundation('#init gracefully exits when css variables are not supported', fa td.verify(adapter.addClass(cssClasses.ROOT), {times: 0}); }); -testFoundation(`#init sets ${strings.VAR_FG_SIZE} to the circumscribing circle's diameter`, - ({foundation, adapter, mockRaf}) => { - const size = 200; - td.when(adapter.computeBoundingRect()).thenReturn({width: size, height: size / 2}); - foundation.init(); - mockRaf.flush(); - const initialSize = size * numbers.INITIAL_ORIGIN_SCALE; - - td.verify(adapter.updateCssVariable(strings.VAR_FG_SIZE, `${initialSize}px`)); - }); - -testFoundation(`#init centers via ${strings.VAR_LEFT} and ${strings.VAR_TOP} when unbounded`, - ({foundation, adapter, mockRaf}) => { - const width = 200; - const height = 100; - const maxSize = Math.max(width, height); - const initialSize = maxSize * numbers.INITIAL_ORIGIN_SCALE; - - td.when(adapter.computeBoundingRect()).thenReturn({width, height}); - td.when(adapter.isUnbounded()).thenReturn(true); - foundation.init(); - mockRaf.flush(); - - td.verify(adapter.updateCssVariable(strings.VAR_LEFT, - `${Math.round((width / 2) - (initialSize / 2))}px`)); - td.verify(adapter.updateCssVariable(strings.VAR_TOP, - `${Math.round((height / 2) - (initialSize / 2))}px`)); - }); - testFoundation('#init registers events for interactions on root element', ({foundation, adapter}) => { foundation.init(); From 0e341f4abab4f2e7ebd3f57354c8026cfc868147 Mon Sep 17 00:00:00 2001 From: "Kenneth G. Franqueiro" Date: Fri, 13 Apr 2018 14:07:28 -0400 Subject: [PATCH 2/4] chore(ripple): Only register window resize handler for unbounded --- packages/mdc-ripple/foundation.js | 10 +++++++-- .../foundation-general-events.test.js | 4 +++- test/unit/mdc-ripple/foundation.test.js | 21 +++++++++++++++++-- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/packages/mdc-ripple/foundation.js b/packages/mdc-ripple/foundation.js index 54f03dc4cfd..66eefedfc20 100644 --- a/packages/mdc-ripple/foundation.js +++ b/packages/mdc-ripple/foundation.js @@ -245,7 +245,10 @@ class MDCRippleFoundation extends MDCFoundation { }); this.adapter_.registerInteractionHandler('focus', this.focusHandler_); this.adapter_.registerInteractionHandler('blur', this.blurHandler_); - this.adapter_.registerResizeHandler(this.resizeHandler_); + + if (this.adapter_.isUnbounded()) { + this.adapter_.registerResizeHandler(this.resizeHandler_); + } } /** @@ -269,7 +272,10 @@ class MDCRippleFoundation extends MDCFoundation { }); this.adapter_.deregisterInteractionHandler('focus', this.focusHandler_); this.adapter_.deregisterInteractionHandler('blur', this.blurHandler_); - this.adapter_.deregisterResizeHandler(this.resizeHandler_); + + if (this.adapter_.isUnbounded()) { + this.adapter_.deregisterResizeHandler(this.resizeHandler_); + } } /** @private */ diff --git a/test/unit/mdc-ripple/foundation-general-events.test.js b/test/unit/mdc-ripple/foundation-general-events.test.js index e6bda259109..2214cc1b7e2 100644 --- a/test/unit/mdc-ripple/foundation-general-events.test.js +++ b/test/unit/mdc-ripple/foundation-general-events.test.js @@ -23,7 +23,8 @@ import {cssClasses, strings} from '../../../packages/mdc-ripple/constants'; suite('MDCRippleFoundation - General Events'); -testFoundation('re-lays out the component on resize event', ({foundation, adapter, mockRaf}) => { +testFoundation('re-lays out the component on resize event for unbounded ripple', ({foundation, adapter, mockRaf}) => { + td.when(adapter.isUnbounded()).thenReturn(true); td.when(adapter.computeBoundingRect()).thenReturn({ width: 100, height: 200, @@ -54,6 +55,7 @@ testFoundation('re-lays out the component on resize event', ({foundation, adapte }); testFoundation('debounces layout within the same frame on resize', ({foundation, adapter, mockRaf}) => { + td.when(adapter.isUnbounded()).thenReturn(true); td.when(adapter.computeBoundingRect()).thenReturn({ width: 100, height: 200, diff --git a/test/unit/mdc-ripple/foundation.test.js b/test/unit/mdc-ripple/foundation.test.js index a88079811ea..19a7b909182 100644 --- a/test/unit/mdc-ripple/foundation.test.js +++ b/test/unit/mdc-ripple/foundation.test.js @@ -84,12 +84,20 @@ testFoundation('#init registers events for interactions on root element', ({foun td.verify(adapter.registerInteractionHandler(td.matchers.isA(String), td.matchers.isA(Function))); }); -testFoundation('#init registers an event for when a resize occurs', ({foundation, adapter}) => { +testFoundation('#init registers a resize handler for unbounded ripple', ({foundation, adapter}) => { + td.when(adapter.isUnbounded()).thenReturn(true); foundation.init(); td.verify(adapter.registerResizeHandler(td.matchers.isA(Function))); }); +testFoundation('#init does not register a resize handler for bounded ripple', ({foundation, adapter}) => { + td.when(adapter.isUnbounded()).thenReturn(false); + foundation.init(); + + td.verify(adapter.registerResizeHandler(td.matchers.isA(Function)), {times: 0}); +}); + testFoundation('#init does not register events if CSS custom properties not supported', ({foundation, adapter}) => { td.when(adapter.browserSupportsCssVars()).thenReturn(false); foundation.init(); @@ -115,8 +123,9 @@ testFoundation('#destroy unregisters all bound interaction handlers', ({foundati td.verify(adapter.deregisterDocumentInteractionHandler(td.matchers.isA(String), td.matchers.isA(Function))); }); -testFoundation('#destroy unregisters the resize handler', ({foundation, adapter}) => { +testFoundation('#destroy unregisters the resize handler for unbounded ripple', ({foundation, adapter}) => { let resizeHandler; + td.when(adapter.isUnbounded()).thenReturn(true); td.when(adapter.registerResizeHandler(td.matchers.isA(Function))).thenDo((handler) => { resizeHandler = handler; }); @@ -126,6 +135,14 @@ testFoundation('#destroy unregisters the resize handler', ({foundation, adapter} td.verify(adapter.deregisterResizeHandler(resizeHandler)); }); +testFoundation('#destroy does not unregister resize handler for bounded ripple', ({foundation, adapter}) => { + td.when(adapter.isUnbounded()).thenReturn(false); + foundation.init(); + foundation.destroy(); + + td.verify(adapter.deregisterResizeHandler(td.matchers.isA(Function)), {times: 0}); +}); + testFoundation(`#destroy removes ${cssClasses.ROOT}`, ({foundation, adapter, mockRaf}) => { foundation.destroy(); mockRaf.flush(); From e4452aec3338626f408efe40b38b9112bb055efa Mon Sep 17 00:00:00 2001 From: "Kenneth G. Franqueiro" Date: Fri, 13 Apr 2018 13:52:08 -0400 Subject: [PATCH 3/4] chore(dialog): Remove logic for updating layout of button ripples Ripple now calls layout upon activation. The layoutFooterRipples adapter API is no longer used and can be removed. --- packages/mdc-dialog/foundation.js | 2 -- packages/mdc-dialog/index.js | 1 - test/unit/mdc-dialog/foundation.test.js | 11 ----------- test/unit/mdc-dialog/mdc-dialog.test.js | 25 ------------------------- 4 files changed, 39 deletions(-) diff --git a/packages/mdc-dialog/foundation.js b/packages/mdc-dialog/foundation.js index c93119c3688..c2c7a83fa65 100644 --- a/packages/mdc-dialog/foundation.js +++ b/packages/mdc-dialog/foundation.js @@ -46,7 +46,6 @@ export default class MDCDialogFoundation extends MDCFoundation { trapFocusOnSurface: () => {}, untrapFocusOnSurface: () => {}, isDialog: (/* el: Element */) => /* boolean */ false, - layoutFooterRipples: () => {}, }; } @@ -138,7 +137,6 @@ export default class MDCDialogFoundation extends MDCFoundation { this.adapter_.removeClass(MDCDialogFoundation.cssClasses.ANIMATING); if (this.isOpen_) { this.adapter_.trapFocusOnSurface(); - this.adapter_.layoutFooterRipples(); } else { this.enableScroll_(); }; diff --git a/packages/mdc-dialog/index.js b/packages/mdc-dialog/index.js index d75cd75b847..49db39b54ff 100644 --- a/packages/mdc-dialog/index.js +++ b/packages/mdc-dialog/index.js @@ -83,7 +83,6 @@ export class MDCDialog extends MDCComponent { trapFocusOnSurface: () => this.focusTrap_.activate(), untrapFocusOnSurface: () => this.focusTrap_.deactivate(), isDialog: (el) => el === this.dialogSurface_, - layoutFooterRipples: () => this.footerBtnRipples_.forEach((ripple) => ripple.layout()), }); } } diff --git a/test/unit/mdc-dialog/foundation.test.js b/test/unit/mdc-dialog/foundation.test.js index c174b4c7dbf..691dab9280e 100644 --- a/test/unit/mdc-dialog/foundation.test.js +++ b/test/unit/mdc-dialog/foundation.test.js @@ -40,7 +40,6 @@ test('default adapter returns a complete adapter implementation', () => { 'registerDocumentKeydownHandler', 'deregisterDocumentKeydownHandler', 'registerTransitionEndHandler', 'deregisterTransitionEndHandler', 'notifyAccept', 'notifyCancel', 'trapFocusOnSurface', 'untrapFocusOnSurface', 'isDialog', - 'layoutFooterRipples', ]); }); @@ -158,16 +157,6 @@ test('#close deactivates focus trapping on the dialog surface', () => { td.verify(mockAdapter.untrapFocusOnSurface()); }); -test('#open calls adapter method to re-layout footer ripples', () => { - const {foundation, mockAdapter} = setupTest(); - - td.when(mockAdapter.registerTransitionEndHandler(td.callback)).thenCallback({target: {}}); - td.when(mockAdapter.isDialog(td.matchers.isA(Object))).thenReturn(true); - foundation.open(); - - td.verify(mockAdapter.layoutFooterRipples()); -}); - test('#accept closes the dialog', () => { const {foundation} = setupTest(); diff --git a/test/unit/mdc-dialog/mdc-dialog.test.js b/test/unit/mdc-dialog/mdc-dialog.test.js index b80891ed0f7..f9db9413cc4 100644 --- a/test/unit/mdc-dialog/mdc-dialog.test.js +++ b/test/unit/mdc-dialog/mdc-dialog.test.js @@ -14,9 +14,6 @@ * limitations under the License. */ -// This suite requires hooks to stub (and clean up) MDCRipple#layout. -/* eslint mocha/no-hooks: "off" */ - import {assert} from 'chai'; import bel from 'bel'; import domEvents from 'dom-events'; @@ -73,22 +70,6 @@ function hasClassMatcher(className) { suite('MDCDialog'); -const originalLayout = MDCRipple.prototype.layout; -const stubbedLayout = td.func('MDCRipple#layout'); - -before(() => { - MDCRipple.prototype.layout = stubbedLayout; -}); - -afterEach(() => { - // Ensure that stubbedLayout's call count resets between tests - td.reset(); -}); - -after(() => { - MDCRipple.prototype.layout = originalLayout; -}); - test('attachTo returns a component instance', () => { assert.isOk(MDCDialog.attachTo(getFixture().querySelector('.mdc-dialog')) instanceof MDCDialog); }); @@ -332,9 +313,3 @@ test('adapter#isDialog returns false for a non-dialog surface element', () => { const {root, component} = setupTest(); assert.isNotOk(component.getDefaultFoundation().adapter_.isDialog(root)); }); - -test('adapter#layoutFooterRipples calls layout on each footer button\'s ripple instance', () => { - const {component} = setupTest(); - component.getDefaultFoundation().adapter_.layoutFooterRipples(); - td.verify(stubbedLayout(), {times: 2}); -}); From b091b53e808015be2222c8fe8e0357eaa4e4dd59 Mon Sep 17 00:00:00 2001 From: "Kenneth G. Franqueiro" Date: Mon, 16 Apr 2018 15:53:10 -0400 Subject: [PATCH 4/4] WIP Remove unused var --- test/unit/mdc-dialog/mdc-dialog.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/mdc-dialog/mdc-dialog.test.js b/test/unit/mdc-dialog/mdc-dialog.test.js index f9db9413cc4..6b28a16de65 100644 --- a/test/unit/mdc-dialog/mdc-dialog.test.js +++ b/test/unit/mdc-dialog/mdc-dialog.test.js @@ -21,7 +21,6 @@ import td from 'testdouble'; import {createMockRaf} from '../helpers/raf'; import {strings} from '../../../packages/mdc-dialog/constants'; import {MDCDialog, util} from '../../../packages/mdc-dialog'; -import {MDCRipple} from '../../../packages/mdc-ripple'; import {supportsCssVariables} from '../../../packages/mdc-ripple/util'; function getFixture() {