Merge pull request #1920 from ProjectMirador/1914-mosaic-lifecycle

Move determining the workspace layout to component lifecycle methods instead of render()

__tests__/src/components/Window.test.js

@@ -7,6 +7,10 @@ import WindowMiddleContent from '../../../src/containers/WindowMiddleContent';
 describe('Window', () => {
   let wrapper;
   const window = { id: 123, xywh: [0, 0, 400, 500] };
+  it('should render nothing, if provided with no window data', () => {
+    wrapper = shallow(<Window />);
+    expect(wrapper.find('.mirador-window')).toHaveLength(0);
+  });
   it('should render outer element', () => {
     wrapper = shallow(<Window window={window} />);
     expect(wrapper.find('.mirador-window')).toHaveLength(1);

__tests__/src/components/WorkspaceMosaic.test.js

@@ -3,44 +3,48 @@ import { shallow } from 'enzyme';
 import { Mosaic } from 'react-mosaic-component';
 import WorkspaceMosaic from '../../../src/components/WorkspaceMosaic';
 
-describe('WorkspaceMosaic', () => {
-  const windows = { 1: { id: 1 }, 2: { id: 2 } };
-  let wrapper;
-  beforeEach(() => {
-    wrapper = shallow(
+/** create wrapper */
+function createWrapper(props) {
+  return shallow(
     <WorkspaceMosaic
-        windows={windows}
+      windows={{}}
       workspace={{}}
       updateWorkspaceMosaicLayout={() => {}}
+      {...props}
     />,
   );
+}
+
+describe('WorkspaceMosaic', () => {
+  const windows = { 1: { id: 1 }, 2: { id: 2 } };
+  let wrapper;
+  beforeEach(() => {
+    wrapper = createWrapper({ windows });
   });
   it('should render properly with an initialValue', () => {
     expect(wrapper.matchesElement(
       <Mosaic initialValue={{ direction: 'row', first: '1', second: '2' }} />,
     )).toBe(true);
   });
+  describe('componentDidUpdate', () => {
+    it('updates the workspace layout when windows change', () => {
+      const updateWorkspaceMosaicLayout = jest.fn();
+      wrapper = createWrapper({ windows, updateWorkspaceMosaicLayout });
+
+      wrapper.setProps({ windows: { ...windows, 3: { id: 3 } } });
+
+      expect(updateWorkspaceMosaicLayout).toHaveBeenCalled();
+    });
+  });
   describe('determineWorkspaceLayout', () => {
     it('when window ids do not match workspace layout', () => {
-      wrapper = shallow(
-        <WorkspaceMosaic
-          windows={windows}
-          workspace={{ layout: 'foo' }}
-          updateWorkspaceMosaicLayout={() => {}}
-        />,
-      );
+      wrapper = createWrapper({ windows, workspace: { layout: 'foo' } });
       expect(wrapper.instance().determineWorkspaceLayout()).toMatchObject({
         direction: 'row', first: '1', second: '2',
       });
     });
     it('when window ids match workspace layout', () => {
-      wrapper = shallow(
-        <WorkspaceMosaic
-          windows={{ foo: { id: 'foo' } }}
-          workspace={{ layout: 'foo' }}
-          updateWorkspaceMosaicLayout={() => {}}
-        />,
-      );
+      wrapper = createWrapper({ windows: { foo: { id: 'foo' } }, workspace: { layout: 'foo' } });
       expect(wrapper.instance().determineWorkspaceLayout()).toBeNull();
     });
   });
@@ -54,16 +58,11 @@ describe('WorkspaceMosaic', () => {
   });
   describe('mosaicChange', () => {
     it('calls the provided prop to update layout', () => {
-      const mock = jest.fn();
-      wrapper = shallow(
-        <WorkspaceMosaic
-          windows={{ foo: { id: 'foo' } }}
-          workspace={{ layout: 'foo' }}
-          updateWorkspaceMosaicLayout={mock}
-        />,
-      );
+      const updateWorkspaceMosaicLayout = jest.fn();
+      wrapper = createWrapper({ windows, updateWorkspaceMosaicLayout });
+
       wrapper.instance().mosaicChange();
-      expect(mock).toBeCalled();
+      expect(updateWorkspaceMosaicLayout).toBeCalled();
     });
   });
 });

src/components/Window.js

@@ -15,6 +15,8 @@ class Window extends Component {
    */
   render() {
     const { manifest, window } = this.props;
+    if (!window) return <></>;
+
     return (
       <div id={window.id} className={ns('window')}>
         <WindowTopBar
@@ -38,11 +40,12 @@ class Window extends Component {
 }
 
 Window.propTypes = {
-  window: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types
+  window: PropTypes.object, // eslint-disable-line react/forbid-prop-types
   manifest: PropTypes.object, // eslint-disable-line react/forbid-prop-types
 };
 
 Window.defaultProps = {
+  window: null,
   manifest: null,
 };
 

src/components/WorkspaceMosaic.js

@@ -23,6 +23,44 @@ class WorkspaceMosaic extends React.Component {
     this.zeroStateView = <div />;
   }
 
+  /** */
+  componentDidMount() {
+    const { updateWorkspaceMosaicLayout } = this.props;
+
+    const newLayout = this.determineWorkspaceLayout();
+    if (newLayout) updateWorkspaceMosaicLayout(newLayout);
+  }
+
+  /** */
+  componentDidUpdate(prevProps) {
+    const { windows, workspace, updateWorkspaceMosaicLayout } = this.props;
+    if (prevProps.windows !== windows || prevProps.workspace !== workspace) {
+      const newLayout = this.determineWorkspaceLayout();
+      if (newLayout) updateWorkspaceMosaicLayout(newLayout);
+    }
+  }
+
+  /**
+   * Used to determine whether or not a "new" layout should be autogenerated.
+   * If a Window is added or removed, generate that new layout and use that for
+   * this render. When the Mosaic changes, that will trigger a new store update.
+   */
+  determineWorkspaceLayout() {
+    const { windows, workspace } = this.props;
+    const windowKeys = Object.keys(windows).sort();
+    const leaveKeys = getLeaves(workspace.layout);
+    // Check every window is in the layout, and all layout windows are present
+    // in store
+    if (!windowKeys.every(e => leaveKeys.includes(e))
+    || !leaveKeys.every(e => windowKeys.includes(e))) {
+      const newLayout = createBalancedTreeFromLeaves(windowKeys);
+
+      return newLayout;
+    }
+
+    return null;
+  }
+
   /**
    * Render a tile (Window) in the Mosaic.
    */
@@ -46,35 +84,14 @@ class WorkspaceMosaic extends React.Component {
     updateWorkspaceMosaicLayout(newLayout);
   }
 
-  /**
-   * Used to determine whether or not a "new" layout should be autogenerated.
-   * If a Window is added or removed, generate that new layout and use that for
-   * this render. When the Mosaic changes, that will trigger a new store update.
-   */
-  determineWorkspaceLayout() {
-    const { windows, workspace, updateWorkspaceMosaicLayout } = this.props;
-    const windowKeys = Object.keys(windows).sort();
-    const leaveKeys = getLeaves(workspace.layout);
-    // Check every window is in the layout, and all layout windows are present
-    // in store
-    if (!windowKeys.every(e => leaveKeys.includes(e))
-    || !leaveKeys.every(e => windowKeys.includes(e))) {
-      const newLayout = createBalancedTreeFromLeaves(windowKeys);
-      updateWorkspaceMosaicLayout(newLayout);
-      return newLayout;
-    }
-    return null;
-  }
-
   /**
    */
   render() {
     const { workspace } = this.props;
-    const newLayout = this.determineWorkspaceLayout();
     return (
       <Mosaic
         renderTile={this.tileRenderer}
-        initialValue={newLayout || workspace.layout}
+        initialValue={workspace.layout || this.determineWorkspaceLayout()}
         onChange={this.mosaicChange}
         className="mirador-mosaic"
         zeroStateView={this.zeroStateView}