From 7c4ee734fab23940d74e96dbe79c08dec25e8798 Mon Sep 17 00:00:00 2001 From: ladeirarodolfo <39910206+ladeirarodolfo@users.noreply.github.com> Date: Fri, 6 Sep 2019 15:31:26 -0300 Subject: [PATCH] =?UTF-8?q?refactor:=20=F0=9F=92=A1=20React=20components?= =?UTF-8?q?=20to=20consume=20appConfig=20using=20Context=20(#852)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: 💡 React components to consume appConfig using Context React components to consume app configuration using React Context and React Hooks. Non React components to continue using global variable window.config. Related documentation also changed. Closes: #725 * refactor: 💡 Removing unecessary code On current React component there is no need to import useContext method from React, so, removing it. * refactor: 💡 Code review Code review. Minor changes based on review inputs and moving userManager to an init method BREAKING CHANGE: #725 Closes: #725 * docs: don't include implementation detail in docs * docs: don't include implementation detail in docs * docs: no need to specify implementation details in employment recipe * docs: no need to specify implementation details in deployment recipe --- README.md | 3 +- .../recipes/nginx--image-archive.md | 6 +- .../recipes/user-account-control.md | 6 +- platform/viewer/src/App.js | 113 ++++++++++-------- platform/viewer/src/OHIFStandaloneViewer.js | 10 +- .../viewer/src/components/Header/Header.js | 19 +-- .../viewer/src/connectedComponents/Viewer.js | 7 +- platform/viewer/src/context/AppContext.js | 5 + .../src/{ => context}/UserManagerContext.js | 0 .../{ => context}/WhiteLabellingContext.js | 2 +- .../viewer/src/studylist/StudyListRouting.js | 21 ++-- .../viewer/src/studylist/StudyListWithData.js | 15 ++- 12 files changed, 121 insertions(+), 86 deletions(-) create mode 100644 platform/viewer/src/context/AppContext.js rename platform/viewer/src/{ => context}/UserManagerContext.js (100%) rename platform/viewer/src/{ => context}/WhiteLabellingContext.js (77%) diff --git a/README.md b/README.md index edc142726..6f74abd22 100644 --- a/README.md +++ b/README.md @@ -119,8 +119,7 @@ window.config = { }; ``` -- Install the viewer: - `window.OHIFStandaloneViewer.installViewer(window.config);` +- Install the viewer: `window.OHIFStandaloneViewer.installViewer(window.config);` This exact setup is demonstrated in this [CodeSandbox](https://codesandbox.io/s/ohif-viewer-script-tag-usage-c4u4t) and diff --git a/docs/latest/deployment/recipes/nginx--image-archive.md b/docs/latest/deployment/recipes/nginx--image-archive.md index baaf64ae4..beb1fec33 100644 --- a/docs/latest/deployment/recipes/nginx--image-archive.md +++ b/docs/latest/deployment/recipes/nginx--image-archive.md @@ -121,11 +121,7 @@ likely want to update: #### OHIF Viewer -The OHIF Viewer's configuration is imported from a static `.js` file and made -available globally at `window.config`. The configuration we use is set to a -specific file when we build the viewer, and determined by the env variable: -`APP_CONFIG`. You can see where we set its value in the `dockerfile` for this -solution: +The OHIF Viewer's configuration is imported from a static `.js` file. The configuration we use is set to a specific file when we build the viewer, and determined by the env variable: `APP_CONFIG`. You can see where we set its value in the `dockerfile` for this solution: `ENV APP_CONFIG=config/docker_openresty-orthanc.js` diff --git a/docs/latest/deployment/recipes/user-account-control.md b/docs/latest/deployment/recipes/user-account-control.md index 609c54995..bed05787d 100644 --- a/docs/latest/deployment/recipes/user-account-control.md +++ b/docs/latest/deployment/recipes/user-account-control.md @@ -122,11 +122,7 @@ likely want to update: #### OHIF Viewer -The OHIF Viewer's configuration is imported from a static `.js` file and made -available globally at `window.config`. The configuration we use is set to a -specific file when we build the viewer, and determined by the env variable: -`APP_CONFIG`. You can see where we set its value in the `dockerfile` for this -solution: +The OHIF Viewer's configuration is imported from a static `.js` file. The configuration we use is set to a specific file when we build the viewer, and determined by the env variable: `APP_CONFIG`. You can see where we set its value in the `dockerfile` for this solution: `ENV APP_CONFIG=config/docker_openresty-orthanc-keycloak.js` diff --git a/platform/viewer/src/App.js b/platform/viewer/src/App.js index b39dc6421..d20504700 100644 --- a/platform/viewer/src/App.js +++ b/platform/viewer/src/App.js @@ -25,12 +25,15 @@ import { OidcProvider } from 'redux-oidc'; import PropTypes from 'prop-types'; import { Provider } from 'react-redux'; import { BrowserRouter as Router } from 'react-router-dom'; -import WhiteLabellingContext from './WhiteLabellingContext'; import { getActiveContexts } from './store/layout/selectors.js'; import i18n from '@ohif/i18n'; import setupTools from './setupTools.js'; import store from './store'; -import UserManagerContext from './UserManagerContext'; + +// Contexts +import WhiteLabellingContext from './context/WhiteLabellingContext'; +import UserManagerContext from './context/UserManagerContext'; +import AppContext from './context/AppContext'; // ~~~~ APP SETUP initCornerstoneTools({ @@ -74,10 +77,66 @@ class App extends Component { extensions: [], }; + _appConfig; + _userManager; + constructor(props) { super(props); - if (this.props.oidc.length) { + this.appConfig = props; + const { servers, extensions, hotkeys, oidc } = props; + + this.initUserManager(oidc); + _initExtensions(extensions, hotkeys); + _initServers(servers); + initWebWorkers(); + } + + render() { + const userManager = this._userManager; + const config = { + appConfig: this._appConfig, + }; + + if (userManager) { + return ( + + + + + + + + + + + + + + + + ); + } + + return ( + + + + + + + + + + + + ); + } + + initUserManager(oidc) { + if (oidc && !!oidc.length) { const firstOpenIdClient = this.props.oidc[0]; const { protocol, host } = window.location; @@ -102,58 +161,18 @@ class App extends Component { ), }); - this.userManager = getUserManagerForOpenIdConnectClient( + this._userManager = getUserManagerForOpenIdConnectClient( store, openIdConnectConfiguration ); } - - _initExtensions(this.props.extensions); - _initServers(this.props.servers); - initWebWorkers(); - } - - render() { - const userManager = this.userManager; - - if (userManager) { - return ( - - - - - - - - - - - - - - ); - } - - return ( - - - - - - - - - - ); } } /** * @param */ -function _initExtensions(extensions) { +function _initExtensions(extensions, hotkeys) { const defaultExtensions = [ GenericViewerCommands, MeasurementsPanel, @@ -163,8 +182,8 @@ function _initExtensions(extensions) { extensionManager.registerExtensions(mergedExtensions); // Must run after extension commands are registered - if (window.config.hotkeys) { - hotkeysManager.setHotkeys(window.config.hotkeys, true); + if (hotkeys) { + hotkeysManager.setHotkeys(hotkeys, true); } } diff --git a/platform/viewer/src/OHIFStandaloneViewer.js b/platform/viewer/src/OHIFStandaloneViewer.js index c8f1007da..9d6bdd685 100644 --- a/platform/viewer/src/OHIFStandaloneViewer.js +++ b/platform/viewer/src/OHIFStandaloneViewer.js @@ -20,6 +20,9 @@ import './OHIFStandaloneViewer.css'; import './variables.css'; import './theme-tide.css'; +// Contexts +import AppContext from './context/AppContext'; + // Dynamic Import Routes (CodeSplitting) // const IHEInvokeImageDisplay = asyncComponent(() => // import('./routes/IHEInvokeImageDisplay.js') @@ -37,6 +40,7 @@ import './theme-tide.css'; const reload = () => window.location.reload(); class OHIFStandaloneViewer extends Component { + static contextType = AppContext; state = { isLoading: false, }; @@ -63,7 +67,7 @@ class OHIFStandaloneViewer extends Component { render() { const { user, userManager } = this.props; - + const { appConfig = {} } = this.context; const userNotLoggedIn = userManager && (!user || user.expired); if (userNotLoggedIn) { const pathname = this.props.location.pathname; @@ -140,9 +144,7 @@ class OHIFStandaloneViewer extends Component { ]; const showStudyList = - window.config && window.config.showStudyList !== undefined - ? window.config.showStudyList - : true; + appConfig.showStudyList !== undefined ? appConfig.showStudyList : true; if (showStudyList) { routes.push({ path: '/studylist', diff --git a/platform/viewer/src/components/Header/Header.js b/platform/viewer/src/components/Header/Header.js index d618606c7..4ac7d81f6 100644 --- a/platform/viewer/src/components/Header/Header.js +++ b/platform/viewer/src/components/Header/Header.js @@ -11,13 +11,17 @@ import { AboutModal } from '@ohif/ui'; import { hotkeysManager } from './../../App.js'; import { withTranslation } from 'react-i18next'; +// Context +import AppContext from './../../context/AppContext'; + class Header extends Component { + static contextType = AppContext; static propTypes = { home: PropTypes.bool.isRequired, location: PropTypes.object.isRequired, children: PropTypes.node, t: PropTypes.func.isRequired, - userManager: PropTypes.object + userManager: PropTypes.object, }; static defaultProps = { @@ -63,10 +67,10 @@ class Header extends Component { if (this.props.user && this.props.userManager) { this.options.push({ title: t('Logout'), - icon: { name: 'power-off' }, - onClick: () => { - this.props.userManager.signoutRedirect(); - }, + icon: { name: 'power-off' }, + onClick: () => { + this.props.userManager.signoutRedirect(); + }, }); } @@ -82,10 +86,9 @@ class Header extends Component { render() { const { t } = this.props; + const { appConfig = {} } = this.context; const showStudyList = - window.config.showStudyList !== undefined - ? window.config.showStudyList - : true; + appConfig.showStudyList !== undefined ? appConfig.showStudyList : true; return (
diff --git a/platform/viewer/src/connectedComponents/Viewer.js b/platform/viewer/src/connectedComponents/Viewer.js index f0363ed65..fe2a0ee8b 100644 --- a/platform/viewer/src/connectedComponents/Viewer.js +++ b/platform/viewer/src/connectedComponents/Viewer.js @@ -5,7 +5,6 @@ import classNames from 'classnames'; import { MODULE_TYPES } from '@ohif/core'; import OHIF from '@ohif/core'; import moment from 'moment'; -import WhiteLabellingContext from '../WhiteLabellingContext.js'; import ConnectedHeader from './ConnectedHeader.js'; import ConnectedToolbarRow from './ConnectedToolbarRow.js'; import ConnectedLabellingOverlay from './ConnectedLabellingOverlay'; @@ -13,7 +12,11 @@ import ConnectedStudyBrowser from './ConnectedStudyBrowser.js'; import ConnectedViewerMain from './ConnectedViewerMain.js'; import SidePanel from './../components/SidePanel.js'; import { extensionManager } from './../App.js'; -import UserManagerContext from '../UserManagerContext'; + +// Contexts +import WhiteLabellingContext from '../context/WhiteLabellingContext.js'; +import UserManagerContext from '../context/UserManagerContext'; + import './Viewer.css'; /** * Inits OHIF Hanging Protocol's onReady. diff --git a/platform/viewer/src/context/AppContext.js b/platform/viewer/src/context/AppContext.js new file mode 100644 index 000000000..82bec40c4 --- /dev/null +++ b/platform/viewer/src/context/AppContext.js @@ -0,0 +1,5 @@ +import React from 'react'; + +let AppContext = React.createContext({}); + +export default AppContext; diff --git a/platform/viewer/src/UserManagerContext.js b/platform/viewer/src/context/UserManagerContext.js similarity index 100% rename from platform/viewer/src/UserManagerContext.js rename to platform/viewer/src/context/UserManagerContext.js diff --git a/platform/viewer/src/WhiteLabellingContext.js b/platform/viewer/src/context/WhiteLabellingContext.js similarity index 77% rename from platform/viewer/src/WhiteLabellingContext.js rename to platform/viewer/src/context/WhiteLabellingContext.js index 345b767fc..d13f9246b 100644 --- a/platform/viewer/src/WhiteLabellingContext.js +++ b/platform/viewer/src/context/WhiteLabellingContext.js @@ -1,4 +1,4 @@ -import OHIFLogo from './components/OHIFLogo/OHIFLogo.js'; +import OHIFLogo from '../components/OHIFLogo/OHIFLogo.js'; import React from 'react'; const defaultContextValues = { diff --git a/platform/viewer/src/studylist/StudyListRouting.js b/platform/viewer/src/studylist/StudyListRouting.js index cd23827e6..8023df157 100644 --- a/platform/viewer/src/studylist/StudyListRouting.js +++ b/platform/viewer/src/studylist/StudyListRouting.js @@ -1,9 +1,12 @@ -import React from 'react'; +import React, { useContext } from 'react'; import PropTypes from 'prop-types'; import { withRouter } from 'react-router-dom'; import queryString from 'query-string'; import ConnectedStudyList from './ConnectedStudyList'; +// Contexts +import AppContext from '../context/AppContext'; + // TODO: Move to @ohif/ui function toLowerCaseFirstLetter(word) { @@ -22,16 +25,20 @@ function getFilters({ search }) { } function StudyListRouting({ location }) { + const { appConfig = {} } = useContext(AppContext); + const filters = location ? getFilters(location) : undefined; let studyListFunctionsEnabled = false; - if (window.config && window.config.studyListFunctionsEnabled) { - studyListFunctionsEnabled = window.config.studyListFunctionsEnabled; + if (appConfig.studyListFunctionsEnabled) { + studyListFunctionsEnabled = appConfig.studyListFunctionsEnabled; } - return ; + return ( + + ); } StudyListRouting.propTypes = { diff --git a/platform/viewer/src/studylist/StudyListWithData.js b/platform/viewer/src/studylist/StudyListWithData.js index ddf5956ed..656cf64f0 100644 --- a/platform/viewer/src/studylist/StudyListWithData.js +++ b/platform/viewer/src/studylist/StudyListWithData.js @@ -10,10 +10,14 @@ import moment from 'moment'; import ConnectedDicomFilesUploader from '../googleCloud/ConnectedDicomFilesUploader'; import ConnectedDicomStorePicker from '../googleCloud/ConnectedDicomStorePicker'; import filesToStudies from '../lib/filesToStudies.js'; -import UserManagerContext from '../UserManagerContext'; -import WhiteLabellingContext from '../WhiteLabellingContext'; + +// Contexts +import UserManagerContext from '../context/UserManagerContext'; +import WhiteLabellingContext from '../context/WhiteLabellingContext'; +import AppContext from '../context/AppContext'; class StudyListWithData extends Component { + static contextType = AppContext; state = { searchData: {}, studies: [], @@ -51,9 +55,10 @@ class StudyListWithData extends Component { }; componentDidMount() { + const { appConfig = {} } = this.context; // TODO: Avoid using timepoints here //const params = { studyInstanceUids, seriesInstanceUids, timepointId, timepointsFilter={} }; - if (!this.props.server && window.config.enableGoogleCloudAdapter) { + if (!this.props.server && appConfig.enableGoogleCloudAdapter) { this.setState({ modalComponentId: 'DicomStorePicker', }); @@ -184,6 +189,7 @@ class StudyListWithData extends Component { }; render() { + const { appConfig = {} } = this.context; const onDrop = async acceptedFiles => { try { const studies = await filesToStudies(acceptedFiles); @@ -203,8 +209,7 @@ class StudyListWithData extends Component { let healthCareApiButtons = null; let healthCareApiWindows = null; - // TODO: This should probably be a prop - if (window.config.enableGoogleCloudAdapter) { + if (appConfig.enableGoogleCloudAdapter) { healthCareApiWindows = (