From c8af69b1da53726f4856fafc64d88644fe29bfa0 Mon Sep 17 00:00:00 2001 From: "Kenneth G. Franqueiro" Date: Tue, 14 Aug 2018 18:12:49 -0400 Subject: [PATCH] feat(tab-indicator): Remove transitionend event handling (#3337) BREAKING CHANGE: Removes handleTransitionEnd foundation API. Removes [de]registerEventHandler adapter APIs. --- packages/mdc-tab-indicator/README.md | 2 - packages/mdc-tab-indicator/adapter.js | 14 ---- packages/mdc-tab-indicator/constants.js | 4 +- .../mdc-tab-indicator/fading-foundation.js | 19 ------ packages/mdc-tab-indicator/foundation.js | 2 - packages/mdc-tab-indicator/index.js | 2 - .../mdc-tab-indicator/mdc-tab-indicator.scss | 14 ++-- .../mdc-tab-indicator/sliding-foundation.js | 34 ++-------- .../fading-foundation.test.js | 49 -------------- .../unit/mdc-tab-indicator/foundation.test.js | 9 ++- .../mdc-tab-indicator.test.js | 18 ----- .../sliding-foundation.test.js | 66 ++----------------- 12 files changed, 26 insertions(+), 207 deletions(-) diff --git a/packages/mdc-tab-indicator/README.md b/packages/mdc-tab-indicator/README.md index 24a4daebe6e..e91895786d4 100644 --- a/packages/mdc-tab-indicator/README.md +++ b/packages/mdc-tab-indicator/README.md @@ -146,8 +146,6 @@ Method Signature | Description --- | --- `addClass(className: string) => void` | Adds a class to the root element. `removeClass(className: string) => void` | Removes a class from the root element. -`registerEventHandler(evtType: string, handler: EventListener) => void` | Registers an event listener on the root element. -`deregisterEventHandler(evtType: string, handler: EventListener) => void` | Deregisters an event listener on the root element. `setContentStyleProp(property: string, value: string) => void` | Sets the style property of the content element. `computeContentClientRect() => ClientRect` | Returns the content element's bounding client rect. diff --git a/packages/mdc-tab-indicator/adapter.js b/packages/mdc-tab-indicator/adapter.js index b3f3edf474b..9fb53a0a29e 100644 --- a/packages/mdc-tab-indicator/adapter.js +++ b/packages/mdc-tab-indicator/adapter.js @@ -28,20 +28,6 @@ * @record */ class MDCTabIndicatorAdapter { - /** - * Registers an event listener on the root element for a given event. - * @param {string} evtType - * @param {function(!Event): undefined} handler - */ - registerEventHandler(evtType, handler) {} - - /** - * Deregisters an event listener on the root element for a given event. - * @param {string} evtType - * @param {function(!Event): undefined} handler - */ - deregisterEventHandler(evtType, handler) {} - /** * Adds the given className to the root element. * @param {string} className The className to add diff --git a/packages/mdc-tab-indicator/constants.js b/packages/mdc-tab-indicator/constants.js index 0fcc30e4543..38fc6f09e49 100644 --- a/packages/mdc-tab-indicator/constants.js +++ b/packages/mdc-tab-indicator/constants.js @@ -19,9 +19,7 @@ const cssClasses = { ACTIVE: 'mdc-tab-indicator--active', FADE: 'mdc-tab-indicator--fade', - FADING_ACTIVATE: 'mdc-tab-indicator--fading-activate', - FADING_DEACTIVATE: 'mdc-tab-indicator--fading-deactivate', - SLIDING_ACTIVATE: 'mdc-tab-indicator--sliding-activate', + NO_TRANSITION: 'mdc-tab-indicator--no-transition', }; /** @enum {string} */ diff --git a/packages/mdc-tab-indicator/fading-foundation.js b/packages/mdc-tab-indicator/fading-foundation.js index b86a96306ad..392d1f0f939 100644 --- a/packages/mdc-tab-indicator/fading-foundation.js +++ b/packages/mdc-tab-indicator/fading-foundation.js @@ -22,30 +22,11 @@ import MDCTabIndicatorFoundation from './foundation'; * @final */ class MDCFadingTabIndicatorFoundation extends MDCTabIndicatorFoundation { - /** @param {...?} args */ - constructor(...args) { - super(...args); - - /** @private {function(?Event): undefined} */ - this.handleTransitionEnd_ = () => this.handleTransitionEnd(); - } - - /** Handles the transitionend event */ - handleTransitionEnd() { - this.adapter_.deregisterEventHandler('transitionend', this.handleTransitionEnd_); - this.adapter_.removeClass(MDCTabIndicatorFoundation.cssClasses.FADING_ACTIVATE); - this.adapter_.removeClass(MDCTabIndicatorFoundation.cssClasses.FADING_DEACTIVATE); - } - activate() { - this.adapter_.registerEventHandler('transitionend', this.handleTransitionEnd_); - this.adapter_.addClass(MDCTabIndicatorFoundation.cssClasses.FADING_ACTIVATE); this.adapter_.addClass(MDCTabIndicatorFoundation.cssClasses.ACTIVE); } deactivate() { - this.adapter_.registerEventHandler('transitionend', this.handleTransitionEnd_); - this.adapter_.addClass(MDCTabIndicatorFoundation.cssClasses.FADING_DEACTIVATE); this.adapter_.removeClass(MDCTabIndicatorFoundation.cssClasses.ACTIVE); } } diff --git a/packages/mdc-tab-indicator/foundation.js b/packages/mdc-tab-indicator/foundation.js index fd160f38386..79ccbbbbb81 100644 --- a/packages/mdc-tab-indicator/foundation.js +++ b/packages/mdc-tab-indicator/foundation.js @@ -43,8 +43,6 @@ class MDCTabIndicatorFoundation extends MDCFoundation { */ static get defaultAdapter() { return /** @type {!MDCTabIndicatorAdapter} */ ({ - registerEventHandler: () => {}, - deregisterEventHandler: () => {}, addClass: () => {}, removeClass: () => {}, computeContentClientRect: () => {}, diff --git a/packages/mdc-tab-indicator/index.js b/packages/mdc-tab-indicator/index.js index 47f120abeaa..80303e35166 100644 --- a/packages/mdc-tab-indicator/index.js +++ b/packages/mdc-tab-indicator/index.js @@ -61,8 +61,6 @@ class MDCTabIndicator extends MDCComponent { */ getDefaultFoundation() { const adapter = /** @type {!MDCTabIndicatorAdapter} */ (Object.assign({ - registerEventHandler: (evtType, handler) => this.root_.addEventListener(evtType, handler), - deregisterEventHandler: (evtType, handler) => this.root_.removeEventListener(evtType, handler), addClass: (className) => this.root_.classList.add(className), removeClass: (className) => this.root_.classList.remove(className), computeContentClientRect: () => this.content_.getBoundingClientRect(), diff --git a/packages/mdc-tab-indicator/mdc-tab-indicator.scss b/packages/mdc-tab-indicator/mdc-tab-indicator.scss index 445403901d5..4a03b5b2e58 100644 --- a/packages/mdc-tab-indicator/mdc-tab-indicator.scss +++ b/packages/mdc-tab-indicator/mdc-tab-indicator.scss @@ -53,16 +53,20 @@ opacity: 1; } -.mdc-tab-indicator--sliding-activate > .mdc-tab-indicator__content { +// Slide by default +.mdc-tab-indicator > .mdc-tab-indicator__content { transition: 250ms transform $mdc-animation-standard-curve-timing-function; } -.mdc-tab-indicator--fading-activate > .mdc-tab-indicator__content, -.mdc-tab-indicator--fading-deactivate > .mdc-tab-indicator__content { +// --no-transition is applied in cases where styles need to be applied immediately to set up a transition +.mdc-tab-indicator--no-transition > .mdc-tab-indicator__content { + transition: none; +} + +.mdc-tab-indicator--fade > .mdc-tab-indicator__content { transition: 150ms opacity linear; } -.mdc-tab-indicator--fading-activate > .mdc-tab-indicator__content { +.mdc-tab-indicator--active.mdc-tab-indicator--fade > .mdc-tab-indicator__content { transition-delay: 100ms; } - diff --git a/packages/mdc-tab-indicator/sliding-foundation.js b/packages/mdc-tab-indicator/sliding-foundation.js index b601f7e8879..87f30cc78e5 100644 --- a/packages/mdc-tab-indicator/sliding-foundation.js +++ b/packages/mdc-tab-indicator/sliding-foundation.js @@ -22,27 +22,12 @@ import MDCTabIndicatorFoundation from './foundation'; * @final */ class MDCSlidingTabIndicatorFoundation extends MDCTabIndicatorFoundation { - /** @param {...?} args */ - constructor(...args) { - super(...args); - - /** @private {function(?Event): undefined} */ - this.handleTransitionEnd_ = () => this.handleTransitionEnd(); - } - - /** Handles the transitionend event */ - handleTransitionEnd() { - this.adapter_.deregisterEventHandler('transitionend', this.handleTransitionEnd_); - this.adapter_.removeClass(MDCTabIndicatorFoundation.cssClasses.SLIDING_ACTIVATE); - } - /** @param {!ClientRect=} previousIndicatorClientRect */ activate(previousIndicatorClientRect) { - this.adapter_.addClass(MDCTabIndicatorFoundation.cssClasses.ACTIVE); - // Early exit if no indicator is present to handle cases where an indicator // may be activated without a prior indicator state if (!previousIndicatorClientRect) { + this.adapter_.addClass(MDCTabIndicatorFoundation.cssClasses.ACTIVE); return; } @@ -53,26 +38,19 @@ class MDCSlidingTabIndicatorFoundation extends MDCTabIndicatorFoundation { const currentClientRect = this.computeContentClientRect(); const widthDelta = previousIndicatorClientRect.width / currentClientRect.width; const xPosition = previousIndicatorClientRect.left - currentClientRect.left; + this.adapter_.addClass(MDCTabIndicatorFoundation.cssClasses.NO_TRANSITION); this.adapter_.setContentStyleProperty('transform', `translateX(${xPosition}px) scaleX(${widthDelta})`); - // Force repaint + // Force repaint before updating classes and transform to ensure the transform properly takes effect this.computeContentClientRect(); - // Add animating class and remove transformation in a new frame - requestAnimationFrame(() => { - this.adapter_.addClass(MDCTabIndicatorFoundation.cssClasses.SLIDING_ACTIVATE); - this.adapter_.setContentStyleProperty('transform', ''); - }); - - this.adapter_.registerEventHandler('transitionend', this.handleTransitionEnd_); + this.adapter_.removeClass(MDCTabIndicatorFoundation.cssClasses.NO_TRANSITION); + this.adapter_.addClass(MDCTabIndicatorFoundation.cssClasses.ACTIVE); + this.adapter_.setContentStyleProperty('transform', ''); } deactivate() { this.adapter_.removeClass(MDCTabIndicatorFoundation.cssClasses.ACTIVE); - // We remove the animating class in deactivate in case the Tab is deactivated before the animation completes and - // the "transitionend" handler isn't called. - this.adapter_.removeClass(MDCTabIndicatorFoundation.cssClasses.SLIDING_ACTIVATE); - this.adapter_.deregisterEventHandler('transitionend', this.handleTransitionEnd_); } } diff --git a/test/unit/mdc-tab-indicator/fading-foundation.test.js b/test/unit/mdc-tab-indicator/fading-foundation.test.js index 6195d18c978..fbc3e2914fd 100644 --- a/test/unit/mdc-tab-indicator/fading-foundation.test.js +++ b/test/unit/mdc-tab-indicator/fading-foundation.test.js @@ -17,7 +17,6 @@ import td from 'testdouble'; -import {captureHandlers} from '../helpers/foundation'; import {setupFoundationTest} from '../helpers/setup'; import MDCFadingTabIndicatorFoundation from '../../../packages/mdc-tab-indicator/fading-foundation'; @@ -25,62 +24,14 @@ suite('MDCFadingTabIndicatorFoundation'); const setupTest = () => setupFoundationTest(MDCFadingTabIndicatorFoundation); -test('#activate registers a transitionend handler', () => { - const {foundation, mockAdapter} = setupTest(); - foundation.activate(); - td.verify(mockAdapter.registerEventHandler('transitionend', td.matchers.isA(Function))); -}); - test(`#activate adds the ${MDCFadingTabIndicatorFoundation.cssClasses.ACTIVE} class`, () => { const {foundation, mockAdapter} = setupTest(); foundation.activate(); td.verify(mockAdapter.addClass(MDCFadingTabIndicatorFoundation.cssClasses.ACTIVE)); }); -test(`#activate adds the ${MDCFadingTabIndicatorFoundation.cssClasses.FADING_ACTIVATE} class`, () => { - const {foundation, mockAdapter} = setupTest(); - foundation.activate(); - td.verify(mockAdapter.addClass(MDCFadingTabIndicatorFoundation.cssClasses.FADING_ACTIVATE)); -}); - -test('#deactivate registers a transitionend handler', () => { - const {foundation, mockAdapter} = setupTest(); - foundation.deactivate(); - td.verify(mockAdapter.registerEventHandler('transitionend', td.matchers.isA(Function))); -}); - test(`#deactivate removes the ${MDCFadingTabIndicatorFoundation.cssClasses.ACTIVE} class`, () => { const {foundation, mockAdapter} = setupTest(); foundation.deactivate(); td.verify(mockAdapter.removeClass(MDCFadingTabIndicatorFoundation.cssClasses.ACTIVE)); }); - -test(`#deactivate adds the ${MDCFadingTabIndicatorFoundation.cssClasses.FADING_DEACTIVATE} class`, () => { - const {foundation, mockAdapter} = setupTest(); - foundation.deactivate(); - td.verify(mockAdapter.addClass(MDCFadingTabIndicatorFoundation.cssClasses.FADING_DEACTIVATE)); -}); - -test('on transitionend, deregister the transitionend handler', () => { - const {foundation, mockAdapter} = setupTest(); - const handlers = captureHandlers(mockAdapter, 'registerEventHandler'); - foundation.activate(); - handlers.transitionend(); - td.verify(mockAdapter.deregisterEventHandler('transitionend', td.matchers.isA(Function))); -}); - -test(`on transitionend, remove the ${MDCFadingTabIndicatorFoundation.cssClasses.FADING_ACTIVATE} class`, () => { - const {foundation, mockAdapter} = setupTest(); - const handlers = captureHandlers(mockAdapter, 'registerEventHandler'); - foundation.activate(); - handlers.transitionend(); - td.verify(mockAdapter.removeClass(MDCFadingTabIndicatorFoundation.cssClasses.FADING_ACTIVATE)); -}); - -test(`on transitionend, remove the ${MDCFadingTabIndicatorFoundation.cssClasses.FADING_DEACTIVATE} class`, () => { - const {foundation, mockAdapter} = setupTest(); - const handlers = captureHandlers(mockAdapter, 'registerEventHandler'); - foundation.activate(); - handlers.transitionend(); - td.verify(mockAdapter.removeClass(MDCFadingTabIndicatorFoundation.cssClasses.FADING_DEACTIVATE)); -}); diff --git a/test/unit/mdc-tab-indicator/foundation.test.js b/test/unit/mdc-tab-indicator/foundation.test.js index 0cf5d62ae99..33c62b8d838 100644 --- a/test/unit/mdc-tab-indicator/foundation.test.js +++ b/test/unit/mdc-tab-indicator/foundation.test.js @@ -34,7 +34,6 @@ test('exports strings', () => { test('defaultAdapter returns a complete adapter implementation', () => { verifyDefaultAdapter(MDCTabIndicatorFoundation, [ - 'registerEventHandler', 'deregisterEventHandler', 'addClass', 'removeClass', 'setContentStyleProperty', 'computeContentClientRect', @@ -52,13 +51,13 @@ test('#computeContentClientRect returns the client rect', () => { test('#activate is abstract and does nothing', () => { const {foundation, mockAdapter} = setupTest(); foundation.activate(); - td.verify(mockAdapter.addClass, {times: 0}); - td.verify(mockAdapter.removeClass, {times: 0}); + td.verify(mockAdapter.addClass(td.matchers.isA(String)), {times: 0}); + td.verify(mockAdapter.removeClass(td.matchers.isA(String)), {times: 0}); }); test('#deactivate is abstract and does nothing', () => { const {foundation, mockAdapter} = setupTest(); foundation.deactivate(); - td.verify(mockAdapter.addClass, {times: 0}); - td.verify(mockAdapter.removeClass, {times: 0}); + td.verify(mockAdapter.addClass(td.matchers.isA(String)), {times: 0}); + td.verify(mockAdapter.removeClass(td.matchers.isA(String)), {times: 0}); }); diff --git a/test/unit/mdc-tab-indicator/mdc-tab-indicator.test.js b/test/unit/mdc-tab-indicator/mdc-tab-indicator.test.js index 94d437fb484..c33c152db0d 100644 --- a/test/unit/mdc-tab-indicator/mdc-tab-indicator.test.js +++ b/test/unit/mdc-tab-indicator/mdc-tab-indicator.test.js @@ -18,7 +18,6 @@ import bel from 'bel'; import {assert} from 'chai'; import td from 'testdouble'; -import domEvents from 'dom-events'; import { MDCTabIndicator, @@ -69,23 +68,6 @@ test('#adapter.removeClass removes a class to the root element', () => { assert.isFalse(root.classList.contains('foo')); }); -test('#adapter.registerEventHandler adds an event listener on the root element', () => { - const {root, component} = setupTest(); - const handler = td.func('transitionend handler'); - component.getDefaultFoundation().adapter_.registerEventHandler('transitionend', handler); - domEvents.emit(root, 'transitionend'); - td.verify(handler(td.matchers.anything())); -}); - -test('#adapter.deregisterEventHandler remoes an event listener from the root element', () => { - const {root, component} = setupTest(); - const handler = td.func('transitionend handler'); - root.addEventListener('transitionend', handler); - component.getDefaultFoundation().adapter_.deregisterEventHandler('transitionend', handler); - domEvents.emit(root, 'transitionend'); - td.verify(handler(td.matchers.anything()), {times: 0}); -}); - test('#adapter.computeContentClientRect returns the root element client rect', () => { const {component, root, content} = setupTest(); document.body.appendChild(root); diff --git a/test/unit/mdc-tab-indicator/sliding-foundation.test.js b/test/unit/mdc-tab-indicator/sliding-foundation.test.js index 2696f751af8..f7d5dd6e4d5 100644 --- a/test/unit/mdc-tab-indicator/sliding-foundation.test.js +++ b/test/unit/mdc-tab-indicator/sliding-foundation.test.js @@ -17,9 +17,7 @@ import td from 'testdouble'; -import {captureHandlers} from '../helpers/foundation'; import {setupFoundationTest} from '../helpers/setup'; -import {createMockRaf} from '../helpers/raf'; import MDCSlidingTabIndicatorFoundation from '../../../packages/mdc-tab-indicator/sliding-foundation'; suite('MDCSlidingTabIndicatorFoundation'); @@ -29,45 +27,23 @@ const setupTest = () => setupFoundationTest(MDCSlidingTabIndicatorFoundation); test(`#activate adds the ${MDCSlidingTabIndicatorFoundation.cssClasses.ACTIVE} class`, () => { const {foundation, mockAdapter} = setupTest(); td.when(mockAdapter.computeContentClientRect()).thenReturn({width: 100, left: 10}); - foundation.activate({width: 90, left: 25}); - td.verify(mockAdapter.addClass(MDCSlidingTabIndicatorFoundation.cssClasses.ACTIVE)); -}); - -test('#activate sets the transform property', () => { - const {foundation, mockAdapter} = setupTest(); - td.when(mockAdapter.computeContentClientRect()).thenReturn({width: 100, left: 10}); - foundation.activate({width: 90, left: 25}); - td.verify(mockAdapter.setContentStyleProperty('transform', 'translateX(15px) scaleX(0.9)')); -}); -test('#activate registers a transitionend handler', () => { - const {foundation, mockAdapter} = setupTest(); - td.when(mockAdapter.computeContentClientRect()).thenReturn({width: 100, left: 10}); foundation.activate({width: 90, left: 25}); - td.verify(mockAdapter.registerEventHandler('transitionend', td.matchers.isA(Function))); + td.verify(mockAdapter.addClass(MDCSlidingTabIndicatorFoundation.cssClasses.ACTIVE)); }); -test(`#activate adds the ${MDCSlidingTabIndicatorFoundation.cssClasses.SLIDING_ACTIVATE} class`, () => { +test('#activate sets the transform property with no transition, then transitions it back', () => { const {foundation, mockAdapter} = setupTest(); - const raf = createMockRaf(); td.when(mockAdapter.computeContentClientRect()).thenReturn({width: 100, left: 10}); - foundation.activate({width: 90, left: 25}); - raf.flush(); - raf.restore(); - td.verify(mockAdapter.addClass(MDCSlidingTabIndicatorFoundation.cssClasses.SLIDING_ACTIVATE)); -}); -test('#activate resets the transform property', () => { - const {foundation, mockAdapter} = setupTest(); - const raf = createMockRaf(); - td.when(mockAdapter.computeContentClientRect()).thenReturn({width: 100, left: 10}); foundation.activate({width: 90, left: 25}); - raf.flush(); - raf.restore(); + td.verify(mockAdapter.addClass(MDCSlidingTabIndicatorFoundation.cssClasses.NO_TRANSITION)); + td.verify(mockAdapter.setContentStyleProperty('transform', 'translateX(15px) scaleX(0.9)')); + td.verify(mockAdapter.removeClass(MDCSlidingTabIndicatorFoundation.cssClasses.NO_TRANSITION)); td.verify(mockAdapter.setContentStyleProperty('transform', '')); }); -test('#activate does not modify transform if no client rect is passed', () => { +test('#activate does not modify transform and does not transition if no client rect is passed', () => { const {foundation, mockAdapter} = setupTest(); foundation.activate(); td.verify(mockAdapter.setContentStyleProperty('transform', td.matchers.isA(String)), {times: 0}); @@ -78,33 +54,3 @@ test(`#deactivate removes the ${MDCSlidingTabIndicatorFoundation.cssClasses.ACTI foundation.deactivate(); td.verify(mockAdapter.removeClass(MDCSlidingTabIndicatorFoundation.cssClasses.ACTIVE)); }); - -test(`#deactivate removes the ${MDCSlidingTabIndicatorFoundation.cssClasses.SLIDING_ACTIVATE} class`, () => { - const {foundation, mockAdapter} = setupTest(); - foundation.deactivate(); - td.verify(mockAdapter.removeClass(MDCSlidingTabIndicatorFoundation.cssClasses.SLIDING_ACTIVATE)); -}); - -test('#deactivate deregisters the transitionend handler', () => { - const {foundation, mockAdapter} = setupTest(); - foundation.deactivate(); - td.verify(mockAdapter.deregisterEventHandler('transitionend', td.matchers.isA(Function))); -}); - -test('on transitionend, deregister the transitionend handler', () => { - const {foundation, mockAdapter} = setupTest(); - const handlers = captureHandlers(mockAdapter, 'registerEventHandler'); - td.when(mockAdapter.computeContentClientRect()).thenReturn({width: 100, left: 10}); - foundation.activate({width: 90, left: 25}); - handlers.transitionend(); - td.verify(mockAdapter.deregisterEventHandler('transitionend', td.matchers.isA(Function))); -}); - -test(`on transitionend, remove the ${MDCSlidingTabIndicatorFoundation.cssClasses.SLIDING_ACTIVATE} class`, () => { - const {foundation, mockAdapter} = setupTest(); - const handlers = captureHandlers(mockAdapter, 'registerEventHandler'); - td.when(mockAdapter.computeContentClientRect()).thenReturn({width: 100, left: 10}); - foundation.activate({width: 90, left: 25}); - handlers.transitionend(); - td.verify(mockAdapter.removeClass(MDCSlidingTabIndicatorFoundation.cssClasses.SLIDING_ACTIVATE)); -});