fix(top-app-bar): foundation should be destroyed and reinitialized on variant change (#519)

Author: mgr34

packages/top-app-bar/index.js

@@ -71,10 +71,25 @@ export default class TopAppBar extends React.Component {
     this.foundation_.destroy();
   }
 
+  componentDidUpdate(prevProps) {
+    const foundationChanged = ['short', 'shortCollapsed', 'fixed']
+      .some((variant) => this.props[variant] !== prevProps[variant] );
+
+    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);
@@ -83,6 +98,7 @@ export default class TopAppBar extends React.Component {
     this.foundation_.init();
   }
 
+
   addClassesToElement(classes, element) {
     const updatedProps = {
       className: classnames(classes, element.props.className),

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",
   "top-app-bar/dense": "e273e6c777f168248f5757c1f00a74206f4cce51c484d91cc7d36dc16de7d0de",
   "top-app-bar/prominentDense": "cc8af934f9187ffd8f250834ef7c73e5c53c5ace10126bb855f74878ba125149",
   "drawer/permanentBelowTopAppBar": "587ee2605c4b3e3f0408c6130b824b58587e05cedf9b964e14fc481b9de1e4c2",
@@ -41,4 +42,4 @@
   "drawer/modal": "da83487c9349b253f7d4de01f92d442de55aab92a8028b77ff1a48cfaa265b72",
   "drawer/permanentToModal": "6355905c2241b5e6fdddc2e25119a1cc3b062577375a88b59e6750c4b76e4561",
   "typography": "c5e87d672d8c05ca3b61c0df4971eabe3c6a6a1f24a9b98f71f55a23360c498a"
-}
+}

test/screenshot/top-app-bar/prominentToShortCollapsed.js

@@ -0,0 +1,60 @@
+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) {
+    return nextState.isPhone !== this.state.isPhone;
+  }
+
+  componentWillUnmount() {
+    window.removeEventListener('resize', this.updateTopAppBarVariant);
+  }
+
+  updateTopAppBarVariant = () => {
+    const isPhone = window.innerWidth < 599;
+    this.setState({isPhone});
+  }
+
+  render() {
+    if (this.state.isPhone) {
+      return (
+        <div>
+          <TopAppBar
+            shortCollapsed
+            navigationIcon={<MaterialIcon
+              icon='menu'
+              onClick={() => console.log('click')}
+            />}
+          />
+          <MainTopAppBarContent />
+        </div>
+      );
+    }
+
+    return (
+      <div>
+        <TopAppBar
+          prominent
+          title={'Annie, I\'m a Hawk'}
+          navigationIcon={<MaterialIcon
+            icon='menu'
+            onClick={() => console.log('click')}
+          />}
+        />
+        <MainTopAppBarContent />
+      </div>
+    );
+  }
+}
+
+export default TopAppBarProminentToShortCollapsedScreenshotTest;

test/screenshot/top-app-bar/variants.js

@@ -8,4 +8,5 @@ export default [
   'standard',
   'standardNoActionItems',
   'standardWithNavigationIconElement',
+  'prominentToShortCollapsed',
 ];

test/unit/top-app-bar/index.test.js

@@ -215,6 +215,111 @@ test('#enableRippleOnElement throws error if a native element', () => {
   );
 });
 
+test('when changes from short to fixed the foundation changes', () => {
+  const wrapper = shallow(<TopAppBar short />);
+  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(<TopAppBar short />);
+  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(<TopAppBar short />);
+  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(<TopAppBar short />);
+  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(<TopAppBar short />);
+  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(<TopAppBar fixed/>);
+  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(<TopAppBar fixed/>);
+  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(<TopAppBar fixed/>);
+  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(<TopAppBar fixed/>);
+  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(<TopAppBar />);
+  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(<TopAppBar />);
+  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(<TopAppBar />);
+  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(<TopAppBar />);
+  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(<TopAppBar />);
   const foundation = wrapper.instance().foundation_;