From 10930cda68dd9f070cab733c7e622d54de04d67e Mon Sep 17 00:00:00 2001 From: Erik Ziegler Date: Sat, 16 May 2020 11:05:50 +0200 Subject: [PATCH] Avoid re-renders in study browser and entire page --- extensions/default/src/MeasurementTable.js | 4 +- extensions/default/src/ViewerLayout/index.jsx | 34 ++++---- extensions/default/src/getPanelModule.js | 53 ++++++++++-- platform/core/src/ToolbarLayoutContext.js | 39 +++++++++ platform/core/src/ViewModelContext.js | 14 +--- platform/core/src/displaySetManager.js | 4 +- platform/core/src/index.js | 6 ++ .../components/StudyBrowser/StudyBrowser.jsx | 11 ++- platform/viewer/src/routes/ModeRoute.js | 82 ++++++++++++------- 9 files changed, 180 insertions(+), 67 deletions(-) create mode 100644 platform/core/src/ToolbarLayoutContext.js diff --git a/extensions/default/src/MeasurementTable.js b/extensions/default/src/MeasurementTable.js index 9915645ad..56f306d69 100644 --- a/extensions/default/src/MeasurementTable.js +++ b/extensions/default/src/MeasurementTable.js @@ -8,10 +8,10 @@ import { } from '@ohif/ui'; export default function MeasurementTable({ servicesManager, commandsManager }) { - console.warn(servicesManager); - const { MeasurementService } = servicesManager.services; + console.error('MeasurementTable rendering!!!!!!!!!!!!!'); + const actionButtons = ( alert('Export')}> diff --git a/extensions/default/src/ViewerLayout/index.jsx b/extensions/default/src/ViewerLayout/index.jsx index cc2ee7409..e27ca087b 100644 --- a/extensions/default/src/ViewerLayout/index.jsx +++ b/extensions/default/src/ViewerLayout/index.jsx @@ -1,19 +1,31 @@ import React, { useEffect, useState } from 'react'; import PropTypes from 'prop-types'; import { SidePanel, Toolbar } from '@ohif/ui'; +import { useToolbarLayout, useViewModel } from '@ohif/core'; // import Header from './Header.jsx'; import { displaySetManager } from '@ohif/core'; +function ViewportDataCreator({setViewportData}) { + const { displaySetInstanceUIDs } = useViewModel() + console.log(displaySetInstanceUIDs); + + useEffect(() => { + setViewportData([ + displaySetManager.getDisplaySetByUID(displaySetInstanceUIDs[0]), + ]); + }, [displaySetInstanceUIDs, setViewportData]); + + return null; +} + function ViewerLayout({ // From Extension Module Params extensionManager, // From Modes leftPanels, rightPanels, - toolBarLayout, viewports, - displaySetInstanceUIDs, ViewportGrid, }) { /** @@ -33,14 +45,6 @@ function ViewerLayout({ // TODO -> Need some way of selecting which displaySets hit the viewports. const [viewportData, setViewportData] = useState([]); - console.log(displaySetInstanceUIDs); - - useEffect(() => { - setViewportData([ - displaySetManager.getDisplaySetByUID(displaySetInstanceUIDs[0]), - ]); - }, [displaySetInstanceUIDs]); - const getPanelData = id => { const entry = extensionManager.getModuleEntry(id); // TODO, not sure why sidepanel content has to be JSX, and not a children prop? @@ -66,16 +70,18 @@ function ViewerLayout({ const leftPanelComponents = leftPanels.map(getPanelData); const rightPanelComponents = rightPanels.map(getPanelData); - const viewportComponents = viewports.map(getViewportComponentData); - console.log(displaySetInstanceUIDs); - console.log(toolBarLayout); + let { toolBarLayout } = useToolbarLayout(); + if (!toolBarLayout.length) { + return null; + } const [primaryToolBarLayout, secondaryToolBarLayout] = toolBarLayout; return (
+
{ setThumbnailImageSrcMap(thumbnailImageSrcMap.set(k, v)); }; + + console.log('StudyBrowserUIData rerender'); + + return ( + + ); +} + +function StudyBrowserPanel({ + activeTabName, + getDataSources, + commandsManager, + onSetTabActive, + studyData, + setStudyData, + thumbnailImageSrcMap, + updateThumbnailMap, +}) { + console.warn('StudyBrowserPanel rerender'); + const viewModel = useViewModel(); + + const dataSource = getDataSources('dicomweb')[0]; + const viewportData = []; //useViewportGrid(); const seriesTracking = {}; //useSeriesTracking(); + // This effect useEffect(() => { if (!viewModel.displaySetInstanceUIDs.length) { return; @@ -192,7 +224,7 @@ function StudyBrowserPanel({ getDataSources, commandsManager }) { }); return () => (isSubscribed = false); - }, [viewModel.displaySetInstanceUIDs]); + }, [viewModel.displaySetInstanceUIDs, thumbnailImageSrcMap, setStudyData]); studyData.forEach(study => { study.displaySets.forEach(ds => { @@ -251,13 +283,20 @@ function StudyBrowserPanel({ getDataSources, commandsManager }) { [studyData] ); - return ; + return ( + + ); } function getPanelModule({ getDataSources, commandsManager, servicesManager }) { const wrappedStudyBrowserPanel = () => { return ( - diff --git a/platform/core/src/ToolbarLayoutContext.js b/platform/core/src/ToolbarLayoutContext.js new file mode 100644 index 000000000..c88419831 --- /dev/null +++ b/platform/core/src/ToolbarLayoutContext.js @@ -0,0 +1,39 @@ +import React, { Component, useContext } from 'react'; + +/// TODO MAKE THIS PRETTY DANNY + +const ToolbarLayoutContext = React.createContext({ + toolBarLayout: [], + setToolBarLayout: () => {}, +}); + +ToolbarLayoutContext.displayName = 'ToolbarLayoutContext'; + +class ToolbarLayoutProvider extends Component { + state = { + toolBarLayout: [], + }; + + render() { + const setToolBarLayout = toolBarLayout => { + this.setState({ toolBarLayout }); + }; + + return ( + + {this.props.children} + + ); + } +} + +const useToolbarLayout = () => useContext(ToolbarLayoutContext); + +export default ToolbarLayoutContext; + +export { ToolbarLayoutProvider, useToolbarLayout }; diff --git a/platform/core/src/ViewModelContext.js b/platform/core/src/ViewModelContext.js index b2af0dc51..4afc762bc 100644 --- a/platform/core/src/ViewModelContext.js +++ b/platform/core/src/ViewModelContext.js @@ -4,9 +4,7 @@ import React, { Component, useContext } from 'react'; const ViewModelContext = React.createContext({ displaySetInstanceUIDs: [], - setDisplaySetInstanceUids: () => {}, - toolBarLayout: [], - setToolBarLayout: () => {}, + setDisplaySetInstanceUIDs: () => {}, }); ViewModelContext.displayName = 'ViewModelContext'; @@ -17,21 +15,15 @@ class ViewModelProvider extends Component { }; render() { - const setDisplaySetInstanceUids = displaySetInstanceUIDs => { + const setDisplaySetInstanceUIDs = displaySetInstanceUIDs => { this.setState({ displaySetInstanceUIDs }); }; - const setToolBarLayout = toolBarLayout => { - this.setState({ toolBarLayout }); - }; - return ( {this.props.children} diff --git a/platform/core/src/displaySetManager.js b/platform/core/src/displaySetManager.js index 3a0f33aa4..f1437d355 100644 --- a/platform/core/src/displaySetManager.js +++ b/platform/core/src/displaySetManager.js @@ -9,11 +9,11 @@ class DisplaySetManager { const { displaySetInstanceUIDs, - setDisplaySetInstanceUids, + setDisplaySetInstanceUIDs, } = viewModelContext; this.displaySetInstanceUIDs = displaySetInstanceUIDs; - this.setDisplaySetInstanceUids = setDisplaySetInstanceUids; + this.setDisplaySetInstanceUids = setDisplaySetInstanceUIDs; // Reset displaySetInstanceUIDs this.setDisplaySetInstanceUids([]); diff --git a/platform/core/src/index.js b/platform/core/src/index.js index 8e8da6ed1..b42278007 100644 --- a/platform/core/src/index.js +++ b/platform/core/src/index.js @@ -23,6 +23,10 @@ import dicomMetadataStore from './dicomMetadataStore'; import displaySetManager from './displaySetManager'; import ToolBarManager from './ToolBarManager'; import { ViewModelProvider, useViewModel } from './ViewModelContext'; +import { + ToolbarLayoutProvider, + useToolbarLayout, +} from './ToolbarLayoutContext'; import utils, { hotkeys } from './utils/'; import { @@ -114,6 +118,8 @@ export { ToolBarManager, ViewModelProvider, useViewModel, + ToolbarLayoutProvider, + useToolbarLayout, }; export { OHIF }; diff --git a/platform/ui/src/components/StudyBrowser/StudyBrowser.jsx b/platform/ui/src/components/StudyBrowser/StudyBrowser.jsx index e04144951..1b32c4cc2 100644 --- a/platform/ui/src/components/StudyBrowser/StudyBrowser.jsx +++ b/platform/ui/src/components/StudyBrowser/StudyBrowser.jsx @@ -22,11 +22,14 @@ const getTrackedSeries = displaySets => { return trackedSeries; }; -const StudyBrowser = ({ tabs, onClickStudy, onClickThumbnail }) => { - const [tabActive, setTabActive] = useState(getInitialActiveTab(tabs)); +const StudyBrowser = ({ tabs, activeTabName, onSetTabActive, onClickStudy, onClickThumbnail }) => { + const [tabActive, setTabActive] = useState(activeTabName || getInitialActiveTab(tabs)); const [studyActive, setStudyActive] = useState(null); const [thumbnailActive, setThumbnailActive] = useState(null); + console.log('StudyBrowser rerender'); + console.log(`tabActive: ${tabActive}`); + const getTabContent = () => { const tabData = tabs.find(tab => tab.name === tabActive); @@ -105,6 +108,10 @@ const StudyBrowser = ({ tabs, onClickStudy, onClickThumbnail }) => { onClick={() => { setTabActive(name); setStudyActive(null); + + if (onSetTabActive) { + onSetTabActive(name) + } }} > {label} diff --git a/platform/viewer/src/routes/ModeRoute.js b/platform/viewer/src/routes/ModeRoute.js index c47dfb9f5..372d31f4f 100644 --- a/platform/viewer/src/routes/ModeRoute.js +++ b/platform/viewer/src/routes/ModeRoute.js @@ -1,45 +1,44 @@ import React, { useContext, useEffect, useCallback } from 'react'; -import { displaySetManager, ToolBarManager, useViewModel } from '@ohif/core'; +import { + displaySetManager, + ToolBarManager, + useViewModel, + useToolbarLayout, + ToolbarLayoutProvider, +} from '@ohif/core'; import { DragAndDropProvider } from '@ohif/ui'; import Compose from './Compose'; import ViewportGrid from './../components/ViewportGrid'; -export default function ModeRoute({ +function DisplaySetCreator({ location, mode, dataSourceName, extensionManager, }) { - const { routes, sopClassHandlers, extensions } = mode; + console.warn('DisplaySetCreator rerendering'); + const { routes, sopClassHandlers } = mode; const dataSources = extensionManager.getDataSources(dataSourceName); - - // Add toolbar state to the view model context? - const { - toolBarLayout, - setToolBarLayout, - displaySetInstanceUIDs, - setDisplaySetInstanceUids, - } = useViewModel(); - // TODO: For now assume one unique datasource. const dataSource = dataSources[0]; const route = routes[0]; - let toolBarManager; + // Add toolbar state to the view model context? + const { displaySetInstanceUIDs, setDisplaySetInstanceUIDs } = useViewModel(); + + const { toolBarLayout, setToolBarLayout } = useToolbarLayout(); useEffect(() => { - toolBarManager = new ToolBarManager(extensionManager, setToolBarLayout); + let toolBarManager = new ToolBarManager(extensionManager, setToolBarLayout); route.init({ toolBarManager }); }, [mode, dataSourceName, location]); - console.log(dataSource); - const createDisplaySets = useCallback(() => { // Add SOPClassHandlers to a new SOPClassManager. displaySetManager.init(extensionManager, sopClassHandlers, { displaySetInstanceUIDs, - setDisplaySetInstanceUids, + setDisplaySetInstanceUIDs, }); const queryParams = location.search; @@ -55,6 +54,23 @@ export default function ModeRoute({ createDisplaySets(); }, [mode, dataSourceName, location]); + return null; +} + +export default function ModeRoute({ + location, + mode, + dataSourceName, + extensionManager, +}) { + console.warn('ModeRoute rerendering'); + const { routes, extensions } = mode; + const dataSources = extensionManager.getDataSources(dataSourceName); + // TODO: For now assume one unique datasource. + + const dataSource = dataSources[0]; + const route = routes[0]; + // Only handling one route per mode for now // You can test via http://localhost:3000/example-mode/dicomweb const layoutTemplateData = route.layoutTemplate({ location }); @@ -89,19 +105,27 @@ export default function ModeRoute({ } return ( - - {/* TODO: extensionManager is already provided to the extension module. - * Use it from there instead of passing as a prop here. - */} - - + + - - + + {/* TODO: extensionManager is already provided to the extension module. + * Use it from there instead of passing as a prop here. + */} + + + + + + ); }