From 3e173e1ddc3bff5c7c9c2d794eeb36a9354ec98c Mon Sep 17 00:00:00 2001 From: j-o-d-o Date: Thu, 17 Aug 2017 07:06:29 +0200 Subject: [PATCH] fix(drawer): Change how click events are handled Remove stopPropagation() for the drawer and add conditonal closing (simliar to the Dialog). BREAKING CHANGE: Adds eventTargetHasClass method to the temporary drawer adapter API. --- packages/mdc-drawer/README.md | 1 + packages/mdc-drawer/slidable/foundation.js | 3 --- packages/mdc-drawer/temporary/foundation.js | 7 ++++++- packages/mdc-drawer/temporary/index.js | 1 + test/unit/mdc-drawer/mdc-temporary-drawer.test.js | 9 +++++++++ test/unit/mdc-drawer/persistent.foundation.test.js | 8 -------- test/unit/mdc-drawer/slidable.foundation.test.js | 2 -- test/unit/mdc-drawer/temporary.foundation.test.js | 2 +- 8 files changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/mdc-drawer/README.md b/packages/mdc-drawer/README.md index a2746615cab..65831d7fffa 100644 --- a/packages/mdc-drawer/README.md +++ b/packages/mdc-drawer/README.md @@ -402,6 +402,7 @@ The adapter for temporary drawers must provide the following functions, with cor | `addClass(className: string) => void` | Adds a class to the root element. | | `removeClass(className: string) => void` | Removes a class from the root element. | | `hasClass(className: string) => boolean` | Returns boolean indicating whether element has a given class. | +| `eventTargetHasClass(target: EventTarget, className: string) => boolean` | Returns true if target has className, false otherwise. | | `addBodyClass(className: string) => void` | Adds a class to the body. | | `removeBodyClass(className: string) => void` | Removes a class from the body. | | `hasNecessaryDom() => boolean` | Returns boolean indicating whether the necessary DOM is present (namely, the `mdc-temporary-drawer__drawer` drawer container). | diff --git a/packages/mdc-drawer/slidable/foundation.js b/packages/mdc-drawer/slidable/foundation.js index 200b3a47f16..f2c451707ec 100644 --- a/packages/mdc-drawer/slidable/foundation.js +++ b/packages/mdc-drawer/slidable/foundation.js @@ -54,7 +54,6 @@ export class MDCSlidableDrawerFoundation extends MDCFoundation { this.inert_ = false; - this.drawerClickHandler_ = (evt) => evt.stopPropagation(); this.componentTouchStartHandler_ = (evt) => this.handleTouchStart_(evt); this.componentTouchMoveHandler_ = (evt) => this.handleTouchMove_(evt); this.componentTouchEndHandler_ = (evt) => this.handleTouchEnd_(evt); @@ -84,14 +83,12 @@ export class MDCSlidableDrawerFoundation extends MDCFoundation { this.isOpen_ = false; } - this.adapter_.registerDrawerInteractionHandler('click', this.drawerClickHandler_); this.adapter_.registerDrawerInteractionHandler('touchstart', this.componentTouchStartHandler_); this.adapter_.registerInteractionHandler('touchmove', this.componentTouchMoveHandler_); this.adapter_.registerInteractionHandler('touchend', this.componentTouchEndHandler_); } destroy() { - this.adapter_.deregisterDrawerInteractionHandler('click', this.drawerClickHandler_); this.adapter_.deregisterDrawerInteractionHandler('touchstart', this.componentTouchStartHandler_); this.adapter_.deregisterInteractionHandler('touchmove', this.componentTouchMoveHandler_); this.adapter_.deregisterInteractionHandler('touchend', this.componentTouchEndHandler_); diff --git a/packages/mdc-drawer/temporary/foundation.js b/packages/mdc-drawer/temporary/foundation.js index c68535c4ab6..3880ae08c9e 100644 --- a/packages/mdc-drawer/temporary/foundation.js +++ b/packages/mdc-drawer/temporary/foundation.js @@ -32,6 +32,7 @@ export default class MDCTemporaryDrawerFoundation extends MDCSlidableDrawerFound removeBodyClass: (/* className: string */) => {}, isDrawer: () => false, updateCssVariable: (/* value: string */) => {}, + eventTargetHasClass: (/* target: EventTarget, className: string */) => /* boolean */ false, }); } @@ -42,7 +43,11 @@ export default class MDCTemporaryDrawerFoundation extends MDCSlidableDrawerFound MDCTemporaryDrawerFoundation.cssClasses.ANIMATING, MDCTemporaryDrawerFoundation.cssClasses.OPEN); - this.componentClickHandler_ = () => this.close(); + this.componentClickHandler_ = (evt) => { + if (this.adapter_.eventTargetHasClass(evt.target, cssClasses.ROOT)) { + this.close(true); + } + }; } init() { diff --git a/packages/mdc-drawer/temporary/index.js b/packages/mdc-drawer/temporary/index.js index 8e2d1e61736..5e56155085c 100644 --- a/packages/mdc-drawer/temporary/index.js +++ b/packages/mdc-drawer/temporary/index.js @@ -52,6 +52,7 @@ export class MDCTemporaryDrawer extends MDCComponent { hasClass: (className) => this.root_.classList.contains(className), addBodyClass: (className) => document.body.classList.add(className), removeBodyClass: (className) => document.body.classList.remove(className), + eventTargetHasClass: (target, className) => target.classList.contains(className), hasNecessaryDom: () => Boolean(this.drawer), registerInteractionHandler: (evt, handler) => this.root_.addEventListener(util.remapEvent(evt), handler, util.applyPassive()), diff --git a/test/unit/mdc-drawer/mdc-temporary-drawer.test.js b/test/unit/mdc-drawer/mdc-temporary-drawer.test.js index aad35ddbfa4..fb088dbe160 100644 --- a/test/unit/mdc-drawer/mdc-temporary-drawer.test.js +++ b/test/unit/mdc-drawer/mdc-temporary-drawer.test.js @@ -87,6 +87,15 @@ test('adapter#hasClass returns false if the root element does not have specified assert.isNotOk(component.getDefaultFoundation().adapter_.hasClass('foo')); }); +test('adapter#eventTargetHasClass returns whether or not the className is in the target\'s classList', () => { + const {component} = setupTest(); + const target = bel`
`; + const {adapter_: adapter} = component.getDefaultFoundation(); + + assert.isTrue(adapter.eventTargetHasClass(target, 'existent-class')); + assert.isFalse(adapter.eventTargetHasClass(target, 'non-existent-class')); +}); + test('adapter#hasNecessaryDom returns true if the DOM includes a drawer', () => { const {component} = setupTest(); assert.isOk(component.getDefaultFoundation().adapter_.hasNecessaryDom()); diff --git a/test/unit/mdc-drawer/persistent.foundation.test.js b/test/unit/mdc-drawer/persistent.foundation.test.js index b85e94fbb8f..b3dc764d477 100644 --- a/test/unit/mdc-drawer/persistent.foundation.test.js +++ b/test/unit/mdc-drawer/persistent.foundation.test.js @@ -50,14 +50,6 @@ test('defaultAdapter returns a complete adapter implementation', () => { ]); }); -test('#init is super.init', () => { - const {foundation, mockAdapter} = setupTest(); - const {isA} = td.matchers; - - foundation.init(); - td.verify(mockAdapter.registerDrawerInteractionHandler('click', isA(Function))); -}); - test('#isRootTransitioningEventTarget_ returns true if the element is the drawer element', () => { const {foundation, mockAdapter} = setupTest(); diff --git a/test/unit/mdc-drawer/slidable.foundation.test.js b/test/unit/mdc-drawer/slidable.foundation.test.js index 0abf4e0a4a9..6ad0088745b 100644 --- a/test/unit/mdc-drawer/slidable.foundation.test.js +++ b/test/unit/mdc-drawer/slidable.foundation.test.js @@ -78,7 +78,6 @@ test('#init calls component and drawer event registrations', () => { const {isA} = td.matchers; foundation.init(); - td.verify(mockAdapter.registerDrawerInteractionHandler('click', isA(Function))); td.verify(mockAdapter.registerDrawerInteractionHandler('touchstart', isA(Function))); td.verify(mockAdapter.registerInteractionHandler('touchmove', isA(Function))); td.verify(mockAdapter.registerInteractionHandler('touchend', isA(Function))); @@ -90,7 +89,6 @@ test('#destroy calls component and drawer event deregistrations', () => { foundation.init(); foundation.destroy(); - td.verify(mockAdapter.deregisterDrawerInteractionHandler('click', isA(Function))); td.verify( mockAdapter.deregisterDrawerInteractionHandler('touchstart', isA(Function)) ); diff --git a/test/unit/mdc-drawer/temporary.foundation.test.js b/test/unit/mdc-drawer/temporary.foundation.test.js index e91b838e6b4..93f8b56d00f 100644 --- a/test/unit/mdc-drawer/temporary.foundation.test.js +++ b/test/unit/mdc-drawer/temporary.foundation.test.js @@ -42,7 +42,7 @@ test('exports cssClasses', () => { test('defaultAdapter returns a complete adapter implementation', () => { verifyDefaultAdapter(MDCTemporaryDrawerFoundation, [ - 'addClass', 'removeClass', 'hasClass', 'addBodyClass', 'removeBodyClass', + 'addClass', 'removeClass', 'hasClass', 'addBodyClass', 'removeBodyClass', 'eventTargetHasClass', 'hasNecessaryDom', 'registerInteractionHandler', 'deregisterInteractionHandler', 'registerDrawerInteractionHandler', 'deregisterDrawerInteractionHandler', 'registerTransitionEndHandler', 'deregisterTransitionEndHandler', 'registerDocumentKeydownHandler',