From 90d9943d5ac7a9f004bf2dfe2350e6e0048b9cef Mon Sep 17 00:00:00 2001 From: mgr34 Date: Thu, 13 Dec 2018 14:28:48 -0500 Subject: [PATCH 1/6] fix(top-app-bar): foundation should be destoryed and reinitialized on variant changes --- packages/top-app-bar/index.js | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/top-app-bar/index.js b/packages/top-app-bar/index.js index befe0ad13..b09c8aa8a 100644 --- a/packages/top-app-bar/index.js +++ b/packages/top-app-bar/index.js @@ -69,10 +69,26 @@ export default class TopAppBar extends React.Component { this.foundation_.destroy(); } + componentDidUpdate(prevProps) { + const foundationChanged = ['short', 'shortCollapsed', 'fixed'] + .filter((variant) => this.props[variant] !== prevProps[variant] ) + .length; + + if (foundationChanged) { + this.initializeFoundation(); + } + } + + initializeFoundation = () => { - if (this.props.short) { + const {short, shortCollapsed, fixed} = this.props; + if (this.foundation_) { + this.foundation_.destroy(); + } + + if (short || shortCollapsed) { this.foundation_ = new MDCShortTopAppBarFoundation(this.adapter); - } else if (this.props.fixed) { + } else if (fixed) { this.foundation_ = new MDCFixedTopAppBarFoundation(this.adapter); } else { this.foundation_ = new MDCTopAppBarFoundation(this.adapter); @@ -81,6 +97,7 @@ export default class TopAppBar extends React.Component { this.foundation_.init(); } + addClassesToElement(classes, element) { const updatedProps = { className: classnames(classes, element.props.className), From ea64e2c9b0e3cdd6df69639b6da19233105b1fa1 Mon Sep 17 00:00:00 2001 From: mgr34 Date: Thu, 13 Dec 2018 14:58:20 -0500 Subject: [PATCH 2/6] fix(top-app-bar): testing that foundation should be destoryed and reinitialized on variant changes --- test/screenshot/top-app-bar/variants.js | 1 + test/unit/top-app-bar/index.test.js | 105 ++++++++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/test/screenshot/top-app-bar/variants.js b/test/screenshot/top-app-bar/variants.js index f1c12eded..0943f1d38 100644 --- a/test/screenshot/top-app-bar/variants.js +++ b/test/screenshot/top-app-bar/variants.js @@ -6,4 +6,5 @@ export default [ 'standard', 'standardNoActionItems', 'standardWithNavigationIconElement', + 'prominentToShortCollapsed', ]; diff --git a/test/unit/top-app-bar/index.test.js b/test/unit/top-app-bar/index.test.js index f840f1582..c03c77491 100644 --- a/test/unit/top-app-bar/index.test.js +++ b/test/unit/top-app-bar/index.test.js @@ -204,6 +204,111 @@ test('#enableRippleOnElement throws error if a native element', () => { ); }); +test('when changes from short to fixed the foundation changes', () => { + const wrapper = shallow(); + const originalFoundation = wrapper.instance().foundation_; + wrapper.setProps({fixed: true, short: false}); + assert.notEqual(wrapper.instance().foundation_, originalFoundation); + assert.exists(wrapper.instance().foundation_); +}); + +test('when changes from short to fixed the foundation changes', () => { + const wrapper = shallow(); + const originalFoundation = wrapper.instance().foundation_; + wrapper.setProps({fixed: true, short: false}); + assert.notEqual(wrapper.instance().foundation_, originalFoundation); + assert.exists(wrapper.instance().foundation_); +}); + +test('when changes from short to standard the foundation changes', () => { + const wrapper = shallow(); + const originalFoundation = wrapper.instance().foundation_; + wrapper.setProps({short: false}); + assert.notEqual(wrapper.instance().foundation_, originalFoundation); + assert.exists(wrapper.instance().foundation_); +}); + +test('when changes from short to prominent the foundation changes', () => { + const wrapper = shallow(); + const originalFoundation = wrapper.instance().foundation_; + wrapper.setProps({short: false, prominent: true}); + assert.notEqual(wrapper.instance().foundation_, originalFoundation); + assert.exists(wrapper.instance().foundation_); +}); + +test('when changes from short to shortCollpased the foundation changes', () => { + const wrapper = shallow(); + const originalFoundation = wrapper.instance().foundation_; + wrapper.setProps({shortCollapsed: true}); + assert.notEqual(wrapper.instance().foundation_, originalFoundation); + assert.exists(wrapper.instance().foundation_); +}); + +test('when changes from fixed to prominent the foundation changes', () => { + const wrapper = shallow(); + const originalFoundation = wrapper.instance().foundation_; + wrapper.setProps({fixed: false, prominent: true}); + assert.notEqual(wrapper.instance().foundation_, originalFoundation); + assert.exists(wrapper.instance().foundation_); +}); + +test('when changes from fixed to short the foundation changes', () => { + const wrapper = shallow(); + const originalFoundation = wrapper.instance().foundation_; + wrapper.setProps({fixed: false, short: true}); + assert.notEqual(wrapper.instance().foundation_, originalFoundation); + assert.exists(wrapper.instance().foundation_); +}); + +test('when changes from fixed to shortCollpased the foundation changes', () => { + const wrapper = shallow(); + const originalFoundation = wrapper.instance().foundation_; + wrapper.setProps({fixed: false, shortCollapsed: true}); + assert.notEqual(wrapper.instance().foundation_, originalFoundation); + assert.exists(wrapper.instance().foundation_); +}); + +test('when changes from fixed to standard the foundation changes', () => { + const wrapper = shallow(); + const originalFoundation = wrapper.instance().foundation_; + wrapper.setProps({fixed: false}); + assert.notEqual(wrapper.instance().foundation_, originalFoundation); + assert.exists(wrapper.instance().foundation_); +}); + +test('when changes from standard to fixed the foundation changes', () => { + const wrapper = shallow(); + const originalFoundation = wrapper.instance().foundation_; + wrapper.setProps({fixed: true}); + assert.notEqual(wrapper.instance().foundation_, originalFoundation); + assert.exists(wrapper.instance().foundation_); +}); + +test('when changes from standard to short the foundation changes', () => { + const wrapper = shallow(); + const originalFoundation = wrapper.instance().foundation_; + wrapper.setProps({short: true}); + assert.notEqual(wrapper.instance().foundation_, originalFoundation); + assert.exists(wrapper.instance().foundation_); +}); + +test('when changes from standard to shortCollapsed the foundation changes', () => { + const wrapper = shallow(); + const originalFoundation = wrapper.instance().foundation_; + wrapper.setProps({shortCollapsed: true}); + assert.notEqual(wrapper.instance().foundation_, originalFoundation); + assert.exists(wrapper.instance().foundation_); +}); + +test('when changes from standard to prominent the foundation does not ' + + 'change', () => { + const wrapper = shallow(); + const originalFoundation = wrapper.instance().foundation_; + wrapper.setProps({prominent: true}); + assert.equal(wrapper.instance().foundation_, originalFoundation); + assert.exists(wrapper.instance().foundation_); +}); + test('#componentWillUnmount destroys foundation', () => { const wrapper = shallow(); const foundation = wrapper.instance().foundation_; From 6fe6c52552c1036ba8d937d563b564ed1194a8c9 Mon Sep 17 00:00:00 2001 From: mgr34 Date: Thu, 13 Dec 2018 14:59:56 -0500 Subject: [PATCH 3/6] fix(top-app-bar): testing that foundation should be destoryed and reinitialized on variant changes (adding missing screenshot test) --- .../top-app-bar/prominentToShortCollapsed.js | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 test/screenshot/top-app-bar/prominentToShortCollapsed.js diff --git a/test/screenshot/top-app-bar/prominentToShortCollapsed.js b/test/screenshot/top-app-bar/prominentToShortCollapsed.js new file mode 100644 index 000000000..041299759 --- /dev/null +++ b/test/screenshot/top-app-bar/prominentToShortCollapsed.js @@ -0,0 +1,63 @@ +import React from 'react'; +import TopAppBar from '../../../packages/top-app-bar'; +import MaterialIcon from '../../../packages/material-icon'; +import MainTopAppBarContent from './mainContent'; + +class TopAppBarProminentToShortCollapsedScreenshotTest extends React.Component { + state = { + isPhone: window.innerWidth < 599, + }; + + componentDidMount() { + window.addEventListener('resize', this.updateTopAppBarVariant); + } + + shouldComponentUpdate(_, nextState) { + if (nextState.isPhone === this.state.isPhone) { + return false; + } + return true; + } + + componentWillUnmount() { + window.removeEventListener('resize', this.updateTopAppBarVariant); + } + + updateTopAppBarVariant = () => { + const isPhone = window.innerWidth < 599; + this.setState({isPhone}); + } + + render() { + if (this.state.isPhone) { + return ( +
+ console.log('click')} + />} + /> + +
+ ); + } + + return ( +
+ console.log('click')} + />} + /> + +
+ ); + } +} + +export default TopAppBarProminentToShortCollapsedScreenshotTest; From 405a109ba59c7a12899b282c81399fc0d1b2dec6 Mon Sep 17 00:00:00 2001 From: mgr34 Date: Fri, 14 Dec 2018 14:56:46 -0500 Subject: [PATCH 4/6] fix(top-app-bar): dropped filter in favor of some --- packages/top-app-bar/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/top-app-bar/index.js b/packages/top-app-bar/index.js index b09c8aa8a..fb3d64b0a 100644 --- a/packages/top-app-bar/index.js +++ b/packages/top-app-bar/index.js @@ -71,8 +71,7 @@ export default class TopAppBar extends React.Component { componentDidUpdate(prevProps) { const foundationChanged = ['short', 'shortCollapsed', 'fixed'] - .filter((variant) => this.props[variant] !== prevProps[variant] ) - .length; + .some((variant) => this.props[variant] !== prevProps[variant] ); if (foundationChanged) { this.initializeFoundation(); From 1cebcef77a9c61a339b56e7be24765e45d42b324 Mon Sep 17 00:00:00 2001 From: mgr34 Date: Sun, 16 Dec 2018 15:04:51 -0500 Subject: [PATCH 5/6] fix(top-app-bar): simplifying shouldComponentUpdate function --- test/screenshot/top-app-bar/prominentToShortCollapsed.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/screenshot/top-app-bar/prominentToShortCollapsed.js b/test/screenshot/top-app-bar/prominentToShortCollapsed.js index 041299759..a59f25bb2 100644 --- a/test/screenshot/top-app-bar/prominentToShortCollapsed.js +++ b/test/screenshot/top-app-bar/prominentToShortCollapsed.js @@ -13,10 +13,7 @@ class TopAppBarProminentToShortCollapsedScreenshotTest extends React.Component { } shouldComponentUpdate(_, nextState) { - if (nextState.isPhone === this.state.isPhone) { - return false; - } - return true; + return nextState.isPhone !== this.state.isPhone; } componentWillUnmount() { From bb7765a6035740abd28e686cb71e09398a83c282 Mon Sep 17 00:00:00 2001 From: Matt Goo Date: Sun, 16 Dec 2018 22:22:42 -0800 Subject: [PATCH 6/6] WIP: add golden --- test/screenshot/golden.json | 1 + 1 file changed, 1 insertion(+) diff --git a/test/screenshot/golden.json b/test/screenshot/golden.json index c18cb11b8..e2104bcaf 100644 --- a/test/screenshot/golden.json +++ b/test/screenshot/golden.json @@ -32,6 +32,7 @@ "top-app-bar/standard": "7a2dd6318d62ac2eabd66f1b28100db7c15840ccb753660065fa9524db6435d6", "top-app-bar/standardNoActionItems": "6d361edb994cafcc6ac720336d12ee7d7114745993e16abd6e6b00f078424ff2", "top-app-bar/standardWithNavigationIconElement": "95afd559c35dede805e4d4b51ad1fabd82b4117c358a8679e3166b88e059bf68", + "top-app-bar/prominentToShortCollapsed": "ce23a6060af19f93ee0255e45ebe51c7243ff12c14b2c09d070501ea7806e316", "drawer/permanentBelowTopAppBar": "587ee2605c4b3e3f0408c6130b824b58587e05cedf9b964e14fc481b9de1e4c2", "drawer/dismissibleBelowTopAppBar": "a9bf24c0edd3dcc9167516ce3bdd146fd1a352c4f5b9ea76a1f9dad1ba61e0f8", "drawer/permanent": "6355905c2241b5e6fdddc2e25119a1cc3b062577375a88b59e6750c4b76e4561",