empty viewport, prevent duplicate series meta from add, protect broadcast events when no data has changed

extensions/default/src/Panels/PanelStudyBrowser.jsx

@@ -51,7 +51,7 @@ function PanelStudyBrowser({
 
   // ~~ Initial Thumbnails
   useEffect(() => {
-    const currentDisplaySets = DisplaySetService.activeDisplaySets || [];
+    const currentDisplaySets = DisplaySetService.activeDisplaySets;
     currentDisplaySets.forEach(async dSet => {
       const newImageSrcEntry = {};
       const displaySet = DisplaySetService.getDisplaySetByUID(
@@ -71,7 +71,7 @@ function PanelStudyBrowser({
   // ~~ displaySets
   useEffect(() => {
     // TODO: Are we sure `activeDisplaySets` will always be accurate?
-    const currentDisplaySets = DisplaySetService.activeDisplaySets || [];
+    const currentDisplaySets = DisplaySetService.activeDisplaySets;
     const mappedDisplaySets = _mapDisplaySets(
       currentDisplaySets,
       thumbnailImageSrcMap
@@ -164,6 +164,7 @@ function PanelStudyBrowser({
 PanelStudyBrowser.propTypes = {
   DisplaySetService: PropTypes.shape({
     EVENTS: PropTypes.object.isRequired,
+    activeDisplaySets: PropTypes.arrayOf(PropTypes.object).isRequired,
     getDisplaySetByUID: PropTypes.func.isRequired,
     hasDisplaySetsForStudy: PropTypes.func.isRequired,
     subscribe: PropTypes.func.isRequired,

platform/core/src/services/DicomMetadataStore/DicomMetadataStore.js

@@ -72,11 +72,16 @@ const BaseImplementation = {
       study = _model.studies[_model.studies.length - 1];
     }
 
-    study.addSeries(instances);
+    // TODO: Worth identifying why this is being called many times with series
-    this._broadcastEvent(EVENTS.INSTANCES_ADDED, {
+    // that are already "added"?
-      StudyInstanceUID,
+    const didAddSeries = study.addSeries(instances);
-      SeriesInstanceUID,
+
-    });
+    if (didAddSeries) {
+      this._broadcastEvent(EVENTS.INSTANCES_ADDED, {
+        StudyInstanceUID,
+        SeriesInstanceUID,
+      });
+    }
   },
   addStudy(study) {
     const { StudyInstanceUID } = study;

platform/core/src/services/DicomMetadataStore/createStudyMetadata.js

@@ -4,9 +4,24 @@ function createStudyMetadata(StudyInstanceUID) {
   return {
     StudyInstanceUID,
     series: [],
+    /**
+     *
+     * @param {object[]} instances
+     * @param {string} instances[].SeriesInstanceUID
+     * @returns {bool} true if series were added; false if series already exist
+     */
     addSeries: function(instances) {
-      const series = createSeriesMetadata(instances);
+      const { SeriesInstanceUID } = instances[0];
-      this.series.push(series);
+      const seriesExists = this.series.some(
+        s => s.SeriesInstanceUID === SeriesInstanceUID
+      );
+
+      if (!seriesExists) {
+        const series = createSeriesMetadata(instances);
+        this.series.push(series);
+      }
+
+      return !seriesExists;
     },
   };
 }

platform/core/src/services/DisplaySetService/DisplaySetService.js

@@ -6,6 +6,7 @@ const displaySetCache = [];
 export default class DisplaySetService {
   constructor() {
     this.displaySets = {};
+    this.activeDisplaySets = [];
     this.listeners = {};
     this.EVENTS = EVENTS;
 
@@ -77,9 +78,10 @@ export default class DisplaySetService {
       displaySetsAdded = displaySets;
     }
 
+    // TODO: This is tricky. How do we know we're not resetting to the same/existing DSs?
+    // TODO: This is likely run anytime we touch DicomMetadataStore. How do we prevent uneccessary broadcasts?
     if (displaySetsAdded && displaySetsAdded.length) {
       this._broadcastEvent(EVENTS.DISPLAY_SETS_ADDED, displaySetsAdded);
-
       this._broadcastEvent(EVENTS.DISPLAY_SETS_CHANGED, this.activeDisplaySets);
     }
   };

platform/core/src/services/_shared/pubSubServiceInterface.js

@@ -13,7 +13,7 @@ export default {
 };
 
 /**
- * Subscribe to measurement updates.
+ * Subscribe to updates.
  *
  * @param {string} eventName The name of the event
  * @param {Function} callback Events callback
@@ -24,7 +24,7 @@ function subscribe(eventName, callback) {
     const listenerId = guid();
     const subscription = { id: listenerId, callback };
 
-    console.info(`displaySetService: Subscribing to '${eventName}'.`);
+    console.info(`Subscribing to '${eventName}'.`);
     if (Array.isArray(this.listeners[eventName])) {
       this.listeners[eventName].push(subscription);
     } else {
@@ -60,7 +60,7 @@ function _unsubscribe(eventName, listenerId) {
 }
 
 /**
- * Check if a given displaySetService event is valid.
+ * Check if a given event is valid.
  *
  * @param {string} eventName The name of the event
  * @return {boolean} Event name validation
@@ -70,7 +70,7 @@ function _isValidEvent(eventName) {
 }
 
 /**
- * Broadcasts displaySetService changes.
+ * Broadcasts changes.
  *
  * @param {string} eventName - The event name
  * @param {func} callbackProps - Properties to pass callback

platform/ui/index.js

@@ -14,6 +14,9 @@ export {
   ImageViewerContext,
   ImageViewerProvider,
   useImageViewer,
+  ViewportGridContext,
+  ViewportGridProvider,
+  useViewportGrid,
 } from './src/contextProviders';
 
 /** COMPONENTS */

platform/ui/src/contextProviders/ViewportGridProvider.jsx

@@ -0,0 +1,20 @@
+import React, { createContext, useContext, useReducer } from 'react';
+
+// export const VIEWPORT_GRID_DEFAULT_VALUE = {
+//   numCols: 1,
+//   numRows: 1,
+//   activeViewportIndex: 0,
+//   viewports: [],
+// };
+
+export const ViewportGridContext = createContext();
+
+export function ViewportGridProvider({ reducer, initialState, children }) {
+  return (
+    <ViewportGridContext.Provider value={useReducer(reducer, initialState)}>
+      {children}
+    </ViewportGridContext.Provider>
+  );
+}
+
+export const useViewportGrid = () => useContext(ViewportGridContext);

platform/ui/src/contextProviders/index.js

@@ -16,4 +16,10 @@ export {
   useImageViewer,
 } from './ImageViewerProvider';
 
+export {
+  ViewportGridContext,
+  ViewportGridProvider,
+  useViewportGrid,
+} from './ViewportGridProvider';
+
 export { default as DragAndDropProvider } from './DragAndDropProvider';

platform/viewer/src/App.jsx

@@ -6,7 +6,7 @@ import { ThemeWrapper } from '@ohif/ui';
 // Viewer Project
 // TODO: Should this influence study list?
 import { appConfigContext } from '@state/appConfig.context';
-import { useAppConfig } from './hooks/useAppConfig';
+import { useAppConfig } from '@hooks/useAppConfig';
 import createRoutes from './routes';
 import appInit from './appInit.js';
 

platform/viewer/src/components/EmptyViewport.jsx

@@ -0,0 +1,11 @@
+import React from 'react';
+
+function EmptyViewport() {
+  return (
+    <div>
+      <p>Please drag a stack here to view images.</p>
+    </div>
+  );
+}
+
+export default EmptyViewport;

platform/viewer/src/components/ViewportGrid.jsx

@@ -1,19 +1,17 @@
 /**
  * CSS Grid Reference: http://grid.malven.co/
  */
-import React, { useState, useCallback, useEffect } from 'react';
+import React from 'react';
 import PropTypes from 'prop-types';
-import { ViewportGrid, ViewportPane } from '@ohif/ui';
+import { ViewportGrid, ViewportPane, useViewportGrid } from '@ohif/ui';
-// import DefaultViewport from './DefaultViewport.js';
+import EmptyViewport from './EmptyViewport';
-// import EmptyViewport from './EmptyViewport.js';
 
 function ViewerViewportGrid(props) {
-  const {
+  const { servicesManager, viewportComponents, dataSource } = props;
-    activeViewportIndex,
+  const [
-    servicesManager,
+    { numCols, numRows, activeViewportIndex, viewports },
-    viewportComponents,
+    dispatch,
-    dataSource,
+  ] = useViewportGrid();
-  } = props;
 
   // TODO -> Need some way of selecting which displaySets hit the viewports.
   const { DisplaySetService } = servicesManager.services;
@@ -23,8 +21,8 @@ function ViewerViewportGrid(props) {
     let displaySetInstanceUID;
 
     // Fallback
-    if(!displaySets || !displaySets.length) {
+    if (!displaySets || !displaySets.length) {
-      const displaySet = DisplaySetService.activeDisplaySets[0]
+      const displaySet = DisplaySetService.activeDisplaySets[0];
       displaySetInstanceUID = displaySet.displaySetInstanceUID;
     } else {
       const displaySet = displaySets[0];
@@ -43,86 +41,97 @@ function ViewerViewportGrid(props) {
     };
   };
 
-  // From ViewportGridService and/or ContextProvider
+  // TODO:
-  const [viewportGrid, setViewportGrid] = useState({
+  // Hmm... Should a "displaySet" being added update the viewport based on HP?
-    numCols: 1,
+  // I guess it might.
-    numRows: 1,
+  // This is where you would likely "fill" emptyViewports if none had content
-    viewports: []
+  // Or to recheck best placement/priority based on all activeDisplaySets
-  });
+  // useEffect(() => {
+  //   const { unsubscribe } = DisplaySetService.subscribe(
+  //     DisplaySetService.EVENTS.DISPLAY_SETS_ADDED,
+  //     displaySets => {
+  //       displaySets.forEach(ds => console.log(`VPG:ADD::${ds.StudyInstanceUID}`));
+  //       const hp = HangingProtocolService(displaySets);
+  //       viewportGrid.setViewportGrid(hp);
+  //     },
+  //   );
 
-
+  //   return unsubscribe;
-  useEffect(() => {
+  // }, []);
-    const { unsubscribe } = DisplaySetService.subscribe(
-      DisplaySetService.EVENTS.DISPLAY_SETS_ADDED,
-      displaySets => {
-        setViewportGrid(HangingProtocolService(displaySets));
-      },
-    );
-
-    return unsubscribe;
-  }, []);
 
   // TODO: either need hover to change "active viewport"
   // so we can use it as our target for setting the displaySet,
   // or the dropHandler needs to know which viewport was dropped on
   // in event data
-  const onDropHandler = ({displaySetInstanceUID}) => {
+  const onDropHandler = ({ displaySetInstanceUID }) => {
-    const droppedDisplaySet = DisplaySetService.getDisplaySetByUID(displaySetInstanceUID);
+    const droppedDisplaySet = DisplaySetService.getDisplaySetByUID(
-    const updatedViewportGridState = HangingProtocolService([droppedDisplaySet]);
+      displaySetInstanceUID
+    );
+    const updatedViewportGridState = HangingProtocolService([
+      droppedDisplaySet,
+    ]);
 
-    console.warn('DROPPED: ', displaySetInstanceUID, droppedDisplaySet, updatedViewportGridState);
+    console.warn(
+      'DROPPED: ',
+      displaySetInstanceUID,
+      droppedDisplaySet,
+      updatedViewportGridState
+    );
 
-    // This is not updating the displayed DisplaySet
+    dispatch({ type: 'action-name', payload: updatedViewportGridState });
-    setViewportGrid(updatedViewportGridState);
+  };
-  }
 
-  // viewportData --> displaySets
+  const getViewportPanes = () => {
-  const getViewportPanes = () =>
+    const viewportPanes = [];
-    viewportGrid.viewports.map((viewport, viewportIndex) => {
+    const numViewportPanes = numCols * numRows;
-      const displaySetInstanceUID = viewport.displaySetInstanceUID;
-      if (!displaySetInstanceUID) {
-        return null;
-      }
 
-      const displaySet = DisplaySetService.getDisplaySetByUID(displaySetInstanceUID);
+    for (let i = 0; i < numViewportPanes; i++) {
+      const viewportIndex = i;
+      const paneMeta = viewports[i];
+      const isEmpty = !paneMeta || !paneMeta.displaySetInstanceUID;
 
-      // TODO: Better Empty Viewport
+      if (isEmpty) {
-      if (!displaySet) {
+        viewportPanes[i] = (
-        return (
           <ViewportPane
             key={viewportIndex}
             className="m-1"
-            // Pass in as prop?
             acceptDropsFor="displayset"
             onDrop={onDropHandler}
             isActive={activeViewportIndex === viewportIndex}
-          />
+          >
+            <EmptyViewport />
+          </ViewportPane>
+        );
+      } else {
+        const displaySet = DisplaySetService.getDisplaySetByUID(
+          paneMeta.displaySetInstanceUID
+        );
+
+        const ViewportComponent = _getViewportComponent(
+          displaySet,
+          viewportComponents
+        );
+
+        viewportPanes[i] = (
+          <ViewportPane
+            key={viewportIndex}
+            className="m-1"
+            acceptDropsFor="displayset"
+            onDrop={onDropHandler}
+            isActive={activeViewportIndex === viewportIndex}
+          >
+            <ViewportComponent
+              displaySet={displaySet}
+              viewportIndex={viewportIndex}
+              dataSource={dataSource}
+            />
+          </ViewportPane>
         );
       }
+    }
 
-      // TODO -> Need way for other viewport e.g. vtk to be used.
+    return viewportPanes;
-
+  };
-      const ViewportComponent = _getViewportComponent(
-        displaySet,
-        viewportComponents
-      );
-
-      return (
-        <ViewportPane
-          key={viewportIndex}
-          className="m-1"
-          acceptDropsFor="displayset"
-          onDrop={onDropHandler}
-          isActive={activeViewportIndex === viewportIndex}
-        >
-          <ViewportComponent
-            displaySet={displaySet}
-            viewportIndex={viewportIndex}
-            dataSource={dataSource}
-          />
-        </ViewportPane>
-      );
-    });
 
   // const ViewportPanes = React.useMemo(getViewportPanes, [
   //   viewportComp'onents,
@@ -131,7 +140,7 @@ function ViewerViewportGrid(props) {
   // ]);
 
   return (
-    <ViewportGrid numRows={viewportGrid.numRows} numCols={viewportGrid.numCols}>
+    <ViewportGrid numRows={numRows} numCols={numCols}>
       {/* {ViewportPanes} */}
       {getViewportPanes()}
     </ViewportGrid>
@@ -139,19 +148,11 @@ function ViewerViewportGrid(props) {
 }
 
 ViewerViewportGrid.propTypes = {
-  // viewports: PropTypes.array.isRequired,
-  activeViewportIndex: PropTypes.number.isRequired,
   viewportComponents: PropTypes.array.isRequired,
-  // numRows: PropTypes.number.isRequired,
-  // numColumns: PropTypes.number.isRequired,
 };
 
 ViewerViewportGrid.defaultProps = {
-  // numRows: 1,
-  // numColumns: 1,
-  viewportData: [],
   viewportComponents: [],
-  activeViewportIndex: 0,
 };
 
 function _getViewportComponent(displaySet, viewportComponents) {

platform/viewer/src/routes/Mode/Mode.jsx

@@ -3,7 +3,11 @@ import { useParams } from 'react-router';
 import PropTypes from 'prop-types';
 // TODO: DicomMetadataStore should be injected?
 import { DicomMetadataStore, ToolBarManager } from '@ohif/core';
-import { DragAndDropProvider, ImageViewerProvider } from '@ohif/ui';
+import {
+  DragAndDropProvider,
+  ImageViewerProvider,
+  ViewportGridProvider,
+} from '@ohif/ui';
 //
 import { useQuery } from '@hooks';
 import ViewportGrid from '@components/ViewportGrid';
@@ -119,23 +123,44 @@ export default function ModeRoute({
     console.log(state, action);
   };
 
+  const viewportGridReducer = (state, action) => {
+    console.log(state, action);
+
+    switch (action.type) {
+      case 'DO_TODO':
+        return state;
+      default:
+        return action.payload;
+    }
+  };
+
   return (
     <ImageViewerProvider
       initialState={{ StudyInstanceUIDs: StudyInstanceUIDsAsArray }}
       reducer={reducer}
     >
-      <CombinedContextProvider>
+      <ViewportGridProvider
-        {/* TODO: extensionManager is already provided to the extension module.
+        initialState={{
-         *  Use it from there instead of passing as a prop here.
+          numRows: 1,
-         */}
+          numCols: 1,
-        <DragAndDropProvider>
+          viewports: [],
-          <LayoutComponent
+          activeViewportIndex: 0,
-            {...layoutTemplateData.props}
+        }}
-            StudyInstanceUIDs={StudyInstanceUIDs}
+        reducer={viewportGridReducer}
-            ViewportGridComp={ViewportGridWithDataSource}
+      >
-          />
+        <CombinedContextProvider>
-        </DragAndDropProvider>
+          {/* TODO: extensionManager is already provided to the extension module.
-      </CombinedContextProvider>
+           *  Use it from there instead of passing as a prop here.
+           */}
+          <DragAndDropProvider>
+            <LayoutComponent
+              {...layoutTemplateData.props}
+              StudyInstanceUIDs={StudyInstanceUIDs}
+              ViewportGridComp={ViewportGridWithDataSource}
+            />
+          </DragAndDropProvider>
+        </CombinedContextProvider>
+      </ViewportGridProvider>
     </ImageViewerProvider>
   );
 }

platform/viewer/src/state/ViewportGridProvider.js

@@ -1,37 +0,0 @@
-import React, { useState, createContext, useContext } from 'react';
-import PropTypes from 'prop-types';
-
-const ViewportGridContext = createContext(null);
-
-export const useViewportGrid = () => useContext(ViewportGridContext);
-
-export default function ViewportGridProvider({ children }) {
-  const [viewportGrid, setViewportGrid] = useState({
-    rows: 1,
-    columns: 1,
-    viewports: [],
-  });
-
-  const get = () => {
-    return viewportGrid;
-  };
-
-  const set = ({ rows, columns, viewports }) => {
-    setViewportGrid({ rows, columns, viewports });
-  };
-
-  return (
-    <ViewportGridProvider.Provider value={{ get, set }}>
-      {/* {children} */}
-    </ViewportGridProvider.Provider>
-  );
-}
-
-ViewportGridProvider.propTypes = {
-  children: PropTypes.oneOfType([
-    PropTypes.arrayOf(PropTypes.node),
-    PropTypes.node,
-  ]).isRequired,
-};
-
-export default ViewportGridProvider;