From c0cc9752c6e4587335bf6be36a2fcdf52747f599 Mon Sep 17 00:00:00 2001 From: dannyrb Date: Fri, 21 Feb 2020 16:08:15 -0500 Subject: [PATCH 01/11] chore: updated autofix settings --- .vscode/settings.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 6d87c6be1..b9d598ba3 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -28,5 +28,8 @@ ], "prettier.disableLanguages": ["html"], "prettier.endOfLine": "lf", - "workbench.colorCustomizations": {} + "workbench.colorCustomizations": {}, + "editor.codeActionsOnSave": { + "source.fixAll.eslint": true + } } From 4cf9f2c9cb487b6f81edd702c236f8f2dfdc79fe Mon Sep 17 00:00:00 2001 From: dannyrb Date: Fri, 21 Feb 2020 16:08:48 -0500 Subject: [PATCH 02/11] chore: remove dead code --- .../viewer/src/connectedComponents/Viewer.js | 35 ------------------- 1 file changed, 35 deletions(-) diff --git a/platform/viewer/src/connectedComponents/Viewer.js b/platform/viewer/src/connectedComponents/Viewer.js index 922b5660c..efb171d63 100644 --- a/platform/viewer/src/connectedComponents/Viewer.js +++ b/platform/viewer/src/connectedComponents/Viewer.js @@ -18,41 +18,6 @@ import WhiteLabellingContext from '../context/WhiteLabellingContext.js'; import UserManagerContext from '../context/UserManagerContext'; import './Viewer.css'; -/** - * Inits OHIF Hanging Protocol's onReady. - * It waits for OHIF Hanging Protocol to be ready to instantiate the ProtocolEngine - * Hanging Protocol will use OHIF LayoutManager to render viewports properly - */ -/*const initHangingProtocol = () => { - // When Hanging Protocol is ready - HP.ProtocolStore.onReady(() => { - - // Gets all StudyMetadata objects: necessary for Hanging Protocol to access study metadata - const studyMetadataList = OHIF.viewer.StudyMetadataList.all(); - - // Instantiate StudyMetadataSource: necessary for Hanging Protocol to get study metadata - const studyMetadataSource = new OHIF.studies.classes.OHIFStudyMetadataSource(); - - // Get prior studies map - const studyPriorsMap = OHIF.studylist.functions.getStudyPriorsMap(studyMetadataList); - - // Creates Protocol Engine object with required arguments - const ProtocolEngine = new HP.ProtocolEngine(layoutManager, studyMetadataList, studyPriorsMap, studyMetadataSource); - - // Sets up Hanging Protocol engine - HP.setEngine(ProtocolEngine); - }); -};*/ - -/*const viewportUtils = OHIF.viewerbase.viewportUtils; - -OHIF.viewer.functionList = { - toggleCineDialog: viewportUtils.toggleCineDialog, - toggleCinePlay: viewportUtils.toggleCinePlay, - clearTools: viewportUtils.clearTools, - resetViewport: viewportUtils.resetViewport, - invert: viewportUtils.invert -};*/ class Viewer extends Component { static propTypes = { From f04d459a63bfc3b7dd81ae16b1dacd91003eaccd Mon Sep 17 00:00:00 2001 From: dannyrb Date: Fri, 21 Feb 2020 16:09:05 -0500 Subject: [PATCH 03/11] chore: further identify study schema/props --- .../viewer/src/connectedComponents/Viewer.js | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/platform/viewer/src/connectedComponents/Viewer.js b/platform/viewer/src/connectedComponents/Viewer.js index efb171d63..ad8ff6790 100644 --- a/platform/viewer/src/connectedComponents/Viewer.js +++ b/platform/viewer/src/connectedComponents/Viewer.js @@ -21,7 +21,27 @@ import './Viewer.css'; class Viewer extends Component { static propTypes = { - studies: PropTypes.array, + studies: PropTypes.arrayOf( + PropTypes.shape({ + studyInstanceUid: PropTypes.string.isRequired, + studyDate: PropTypes.string, + displaySets: PropTypes.arrayOf( + PropTypes.shape({ + displaySetInstanceUid: PropTypes.string.isRequired, + seriesDescription: PropTypes.string, + seriesNumber: PropTypes.number, + instanceNumber: PropTypes.number, + numImageFrames: PropTypes.number, + modality: PropTypes.string.isRequired, + images: PropTypes.arrayOf( + PropTypes.shape({ + getImageId: PropTypes.func.isRequired, + }) + ), + }) + ), + }) + ), studyInstanceUids: PropTypes.array, activeServer: PropTypes.shape({ type: PropTypes.string, From f68aaf973cefab09abe3e925aaa611e4c3523556 Mon Sep 17 00:00:00 2001 From: dannyrb Date: Fri, 21 Feb 2020 16:09:55 -0500 Subject: [PATCH 04/11] chore: sketch out beginnings of isDisabled method for sidePanel menuOptions --- .../src/connectedComponents/ToolbarRow.js | 39 ++++++++++++------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/platform/viewer/src/connectedComponents/ToolbarRow.js b/platform/viewer/src/connectedComponents/ToolbarRow.js index 29ba3798f..e5057d20e 100644 --- a/platform/viewer/src/connectedComponents/ToolbarRow.js +++ b/platform/viewer/src/connectedComponents/ToolbarRow.js @@ -25,9 +25,14 @@ class ToolbarRow extends Component { isRightSidePanelOpen: PropTypes.bool.isRequired, selectedLeftSidePanel: PropTypes.string.isRequired, selectedRightSidePanel: PropTypes.string.isRequired, - handleSidePanelChange: PropTypes.func, + handleSidePanelChange: PropTypes.func.isRequired, activeContexts: PropTypes.arrayOf(PropTypes.string).isRequired, studies: PropTypes.array, + t: PropTypes.func.isRequired, + }; + + static defaultProps = { + studies: [], }; constructor(props) { @@ -50,30 +55,26 @@ class ToolbarRow extends Component { this._handleBuiltIn = _handleBuiltIn.bind(this); const panelModules = extensionManager.modules[MODULE_TYPES.PANEL]; + this.buttonGroups = { - left: [ - // TODO: This should come from extensions, instead of being baked in - { - value: 'studies', - icon: 'th-large', - bottomLabel: this.props.t('Series'), - }, - ], + left: [], right: [], }; + // ~ FIND MENU OPTIONS panelModules.forEach(panelExtension => { const panelModule = panelExtension.module; const defaultContexts = Array.from(panelModule.defaultContext); - // MENU OPTIONS panelModule.menuOptions.forEach(menuOption => { const contexts = Array.from(menuOption.context || defaultContexts); - - const activeContextIncludesAnyPanelContexts = this.props.activeContexts.some( - actx => contexts.includes(actx) + const hasActiveContext = this.props.activeContexts.some(actx => + contexts.includes(actx) ); - if (activeContextIncludesAnyPanelContexts) { + // TODO: what should `isDisabled` receive? + const isDisabled = menuOption.isDisabled(); + + if (hasActiveContext && !isDisabled) { const menuOptionEntry = { value: menuOption.target, icon: menuOption.icon, @@ -85,6 +86,16 @@ class ToolbarRow extends Component { } }); }); + + // }); + // }); + + // TODO: This should come from extensions, instead of being baked in + this.buttonGroups.right.unshift({ + value: 'studies', + icon: 'th-large', + bottomLabel: this.props.t('Series'), + }); } componentDidUpdate(prevProps) { From 3d0357c9c0f6de1193a4b223d027731421c6b755 Mon Sep 17 00:00:00 2001 From: dannyrb Date: Mon, 24 Feb 2020 11:16:24 -0500 Subject: [PATCH 05/11] chore: set API as studies object --- platform/viewer/src/connectedComponents/ToolbarRow.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/platform/viewer/src/connectedComponents/ToolbarRow.js b/platform/viewer/src/connectedComponents/ToolbarRow.js index e5057d20e..b012e6eb0 100644 --- a/platform/viewer/src/connectedComponents/ToolbarRow.js +++ b/platform/viewer/src/connectedComponents/ToolbarRow.js @@ -72,7 +72,9 @@ class ToolbarRow extends Component { contexts.includes(actx) ); // TODO: what should `isDisabled` receive? - const isDisabled = menuOption.isDisabled(); + // studies? activeViewport? + // It's a bit beefy to pass studies; probably only need to be reactive on `studyInstanceUIDs` and activeViewport? + const isDisabled = menuOption.isDisabled(this.props.studies); if (hasActiveContext && !isDisabled) { const menuOptionEntry = { From ea2ab1a25e305043deea4ad6a51c929dbc309ad7 Mon Sep 17 00:00:00 2001 From: dannyrb Date: Mon, 24 Feb 2020 11:18:55 -0500 Subject: [PATCH 06/11] docs: update docs to include isDisabled computed --- docs/latest/extensions/modules/panel.md | 4 ++++ extensions/_example/src/index.js | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/latest/extensions/modules/panel.md b/docs/latest/extensions/modules/panel.md index 57b63719a..6254824b5 100644 --- a/docs/latest/extensions/modules/panel.md +++ b/docs/latest/extensions/modules/panel.md @@ -40,6 +40,10 @@ export default { from: 'right', // The target component to toggle open/close target: 'target-component-id', + // UI Hint; If the target panel is in a "disabled" state + isDisabled: studies => { + return false; + }, // Overrides `defaultContext`, if specified context: ['ACTIVE_VIEWPORT:MAGIC'], }, diff --git a/extensions/_example/src/index.js b/extensions/_example/src/index.js index bafd2ae32..de0d09128 100644 --- a/extensions/_example/src/index.js +++ b/extensions/_example/src/index.js @@ -21,7 +21,6 @@ export default { /** * MODULE GETTERS */ - getViewportModule() { return '... react component ...'; }, @@ -98,6 +97,9 @@ const panelModule = { icon: 'th-list', label: 'Segments', target: 'segment-panel', + isDisabled: studies => { + return false; + }, }, ], components: [ From 141e1cf64845c0f1dd5f7eb1ff67767e41228307 Mon Sep 17 00:00:00 2001 From: dannyrb Date: Mon, 24 Feb 2020 11:20:10 -0500 Subject: [PATCH 07/11] chore: note regarding reactivity --- platform/viewer/src/connectedComponents/ToolbarRow.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/platform/viewer/src/connectedComponents/ToolbarRow.js b/platform/viewer/src/connectedComponents/ToolbarRow.js index b012e6eb0..2dca5835f 100644 --- a/platform/viewer/src/connectedComponents/ToolbarRow.js +++ b/platform/viewer/src/connectedComponents/ToolbarRow.js @@ -71,9 +71,9 @@ class ToolbarRow extends Component { const hasActiveContext = this.props.activeContexts.some(actx => contexts.includes(actx) ); - // TODO: what should `isDisabled` receive? - // studies? activeViewport? + // It's a bit beefy to pass studies; probably only need to be reactive on `studyInstanceUIDs` and activeViewport? + // Note: This does not cleanly handle `studies` prop updating with panel open const isDisabled = menuOption.isDisabled(this.props.studies); if (hasActiveContext && !isDisabled) { From 298da03e2f3acadcbdb7ddad4dc739b70fe4d817 Mon Sep 17 00:00:00 2001 From: dannyrb Date: Mon, 24 Feb 2020 11:22:41 -0500 Subject: [PATCH 08/11] chore: handle function not set (preserve non-breaking) --- platform/viewer/src/connectedComponents/ToolbarRow.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/platform/viewer/src/connectedComponents/ToolbarRow.js b/platform/viewer/src/connectedComponents/ToolbarRow.js index 2dca5835f..f7b9841b9 100644 --- a/platform/viewer/src/connectedComponents/ToolbarRow.js +++ b/platform/viewer/src/connectedComponents/ToolbarRow.js @@ -74,7 +74,9 @@ class ToolbarRow extends Component { // It's a bit beefy to pass studies; probably only need to be reactive on `studyInstanceUIDs` and activeViewport? // Note: This does not cleanly handle `studies` prop updating with panel open - const isDisabled = menuOption.isDisabled(this.props.studies); + const isDisabled = + typeof menuOption.isDisabled === 'function' && + menuOption.isDisabled(this.props.studies); if (hasActiveContext && !isDisabled) { const menuOptionEntry = { From 85fed49eb13db7e41d2c696c507acffed9cc9a35 Mon Sep 17 00:00:00 2001 From: dannyrb Date: Mon, 24 Feb 2020 11:28:35 -0500 Subject: [PATCH 09/11] chore: clarify how props are added --- .../viewer/src/connectedComponents/ToolbarRow.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/platform/viewer/src/connectedComponents/ToolbarRow.js b/platform/viewer/src/connectedComponents/ToolbarRow.js index f7b9841b9..ab89a1d4c 100644 --- a/platform/viewer/src/connectedComponents/ToolbarRow.js +++ b/platform/viewer/src/connectedComponents/ToolbarRow.js @@ -29,6 +29,9 @@ class ToolbarRow extends Component { activeContexts: PropTypes.arrayOf(PropTypes.string).isRequired, studies: PropTypes.array, t: PropTypes.func.isRequired, + // NOTE: withDialog, withModal HOCs + dialog: PropTypes.any, + modal: PropTypes.any, }; static defaultProps = { @@ -91,9 +94,6 @@ class ToolbarRow extends Component { }); }); - // }); - // }); - // TODO: This should come from extensions, instead of being baked in this.buttonGroups.right.unshift({ value: 'studies', @@ -237,6 +237,10 @@ function _getButtonComponents(toolbarButtons, activeButtons) { } /** + * TODO: DEPRECATE + * This is used exclusively in `extensions/cornerstone/src` + * We have better ways with new UI Services to trigger "builtin" behaviors + * * A handy way for us to handle different button types. IE. firing commands for * buttons, or initiation built in behavior. * @@ -288,7 +292,7 @@ function _getVisibleToolbarButtons() { function _handleBuiltIn(button) { /* TODO: Keep cine button active until its unselected. */ - const { dialog, modal, t } = this.props; + const { dialog, t } = this.props; const { dialogId } = this.state; const { id, options } = button; From 846c9c1c54851cde142438882f2620b59ed8b161 Mon Sep 17 00:00:00 2001 From: dannyrb Date: Mon, 24 Feb 2020 11:35:34 -0500 Subject: [PATCH 10/11] chore: specify left side for built-in series button (thumbnails) --- platform/viewer/src/connectedComponents/ToolbarRow.js | 1 + 1 file changed, 1 insertion(+) diff --git a/platform/viewer/src/connectedComponents/ToolbarRow.js b/platform/viewer/src/connectedComponents/ToolbarRow.js index ab89a1d4c..bea301bfb 100644 --- a/platform/viewer/src/connectedComponents/ToolbarRow.js +++ b/platform/viewer/src/connectedComponents/ToolbarRow.js @@ -98,6 +98,7 @@ class ToolbarRow extends Component { this.buttonGroups.right.unshift({ value: 'studies', icon: 'th-large', + from: 'left', bottomLabel: this.props.t('Series'), }); } From 6829542c005fdc7d2054ccd6ca16f4df0da1e6c5 Mon Sep 17 00:00:00 2001 From: dannyrb Date: Mon, 24 Feb 2020 11:36:10 -0500 Subject: [PATCH 11/11] chore: remove 'from', and push to correct key --- platform/viewer/src/connectedComponents/ToolbarRow.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/platform/viewer/src/connectedComponents/ToolbarRow.js b/platform/viewer/src/connectedComponents/ToolbarRow.js index bea301bfb..86839fe07 100644 --- a/platform/viewer/src/connectedComponents/ToolbarRow.js +++ b/platform/viewer/src/connectedComponents/ToolbarRow.js @@ -95,10 +95,9 @@ class ToolbarRow extends Component { }); // TODO: This should come from extensions, instead of being baked in - this.buttonGroups.right.unshift({ + this.buttonGroups.left.unshift({ value: 'studies', icon: 'th-large', - from: 'left', bottomLabel: this.props.t('Series'), }); }