From 7643f3bf18eb71710bb234e141dcbad7858df657 Mon Sep 17 00:00:00 2001 From: Abhinay Omkar Date: Fri, 18 May 2018 16:02:43 -0400 Subject: [PATCH] fix(top-app-bar): Fix JS error when navigation icon is not present. (#2751) --- packages/mdc-top-app-bar/index.js | 4 ++- .../mdc-top-app-bar/mdc-top-app-bar.test.js | 29 ++++++++++++++----- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/packages/mdc-top-app-bar/index.js b/packages/mdc-top-app-bar/index.js index 395ab929e0b..4ba13c15419 100644 --- a/packages/mdc-top-app-bar/index.js +++ b/packages/mdc-top-app-bar/index.js @@ -46,7 +46,9 @@ class MDCTopAppBar extends MDCComponent { // Get all icons in the toolbar and instantiate the ripples const icons = [].slice.call(this.root_.querySelectorAll(strings.ACTION_ITEM_SELECTOR)); - icons.push(this.navIcon_); + if (this.navIcon_) { + icons.push(this.navIcon_); + } this.iconRipples_ = icons.map((icon) => { const ripple = rippleFactory(icon); diff --git a/test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js b/test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js index b188ca75e70..3bbf09f6253 100644 --- a/test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js +++ b/test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js @@ -66,6 +66,11 @@ function getFixture(removeIcon) { return html; } +function getIconsCount(root) { + const selector = strings.ACTION_ITEM_SELECTOR + ',' + strings.NAVIGATION_ICON_SELECTOR; + return root.querySelectorAll(selector).length; +} + class FakeRipple { constructor(root) { this.root = root; @@ -74,11 +79,11 @@ class FakeRipple { } } -function setupTest(removeIcon = false) { +function setupTest(removeIcon = false, rippleFactory = (el) => new FakeRipple(el)) { const fixture = getFixture(removeIcon); const root = fixture.querySelector(strings.ROOT_SELECTOR); const icon = root.querySelector(strings.NAVIGATION_ICON_SELECTOR); - const component = new MDCTopAppBar(root, undefined, (el) => new FakeRipple(el)); + const component = new MDCTopAppBar(root, undefined, rippleFactory); return {root, component, icon}; } @@ -89,12 +94,22 @@ test('attachTo initializes and returns an MDCTopAppBar instance', () => { assert.isTrue(MDCTopAppBar.attachTo(getFixture()) instanceof MDCTopAppBar); }); -test('constructor instantiates icon ripples', () => { - const {root, component} = setupTest(); - const selector = strings.ACTION_ITEM_SELECTOR + ',' + strings.NAVIGATION_ICON_SELECTOR; - const totalIcons = root.querySelectorAll(selector).length; +test('constructor instantiates icon ripples for all icons', () => { + const rippleFactory = td.function(); + td.when(rippleFactory(td.matchers.anything())).thenReturn((el) => new FakeRipple(el)); + const {root} = setupTest(/** removeIcon */ false, rippleFactory); + + const totalIcons = getIconsCount(root); + td.verify(rippleFactory(td.matchers.anything()), {times: totalIcons}); +}); + +test('constructor does not instantiate ripple for nav icon when not present', () => { + const rippleFactory = td.function(); + td.when(rippleFactory(td.matchers.anything())).thenReturn((el) => new FakeRipple(el)); + const {root} = setupTest(/** removeIcon */ true, rippleFactory); - assert.isTrue(component.iconRipples_.length === totalIcons); + const totalIcons = getIconsCount(root); + td.verify(rippleFactory(td.matchers.anything()), {times: totalIcons}); }); test('destroy destroys icon ripples', () => {