fix(hotkeys): preserve hotkeys if changed, and reduce re-rendering (#3635)

extensions/default/src/ViewerLayout/ViewerHeader.tsx

@@ -0,0 +1,118 @@
+import React from 'react';
+import { useNavigate } from 'react-router-dom';
+import { useTranslation } from 'react-i18next';
+import { useLocation } from 'react-router';
+
+import { ErrorBoundary, UserPreferences, AboutModal, Header, useModal } from '@ohif/ui';
+import i18n from '@ohif/i18n';
+import { hotkeys } from '@ohif/core';
+import { useAppConfig } from '@state';
+import Toolbar from '../Toolbar/Toolbar';
+
+const { availableLanguages, defaultLanguage, currentLanguage } = i18n;
+
+function ViewerHeader({ hotkeysManager, extensionManager, servicesManager }) {
+  const [appConfig] = useAppConfig();
+  const navigate = useNavigate();
+  const location = useLocation();
+
+  const onClickReturnButton = () => {
+    const { pathname } = location;
+    const dataSourceIdx = pathname.indexOf('/', 1);
+    const query = new URLSearchParams(window.location.search);
+    const configUrl = query.get('configUrl');
+
+    const dataSourceName = pathname.substring(dataSourceIdx + 1);
+    const existingDataSource = extensionManager.getDataSources(dataSourceName);
+
+    const searchQuery = new URLSearchParams();
+    if (dataSourceIdx !== -1 && existingDataSource) {
+      searchQuery.append('datasources', pathname.substring(dataSourceIdx + 1));
+    }
+
+    if (configUrl) {
+      searchQuery.append('configUrl', configUrl);
+    }
+
+    navigate({
+      pathname: '/',
+      search: decodeURIComponent(searchQuery.toString()),
+    });
+  };
+
+  const { t } = useTranslation();
+  const { show, hide } = useModal();
+  const { hotkeyDefinitions, hotkeyDefaults } = hotkeysManager;
+  const versionNumber = process.env.VERSION_NUMBER;
+  const commitHash = process.env.COMMIT_HASH;
+
+  const menuOptions = [
+    {
+      title: t('Header:About'),
+      icon: 'info',
+      onClick: () =>
+        show({
+          content: AboutModal,
+          title: 'About OHIF Viewer',
+          contentProps: { versionNumber, commitHash },
+        }),
+    },
+    {
+      title: t('Header:Preferences'),
+      icon: 'settings',
+      onClick: () =>
+        show({
+          title: t('UserPreferencesModal:User Preferences'),
+          content: UserPreferences,
+          contentProps: {
+            hotkeyDefaults: hotkeysManager.getValidHotkeyDefinitions(hotkeyDefaults),
+            hotkeyDefinitions,
+            currentLanguage: currentLanguage(),
+            availableLanguages,
+            defaultLanguage,
+            onCancel: () => {
+              hotkeys.stopRecord();
+              hotkeys.unpause();
+              hide();
+            },
+            onSubmit: ({ hotkeyDefinitions, language }) => {
+              if (language.value !== currentLanguage().value) {
+                i18n.changeLanguage(language.value);
+              }
+              hotkeysManager.setHotkeys(hotkeyDefinitions);
+              hide();
+            },
+            onReset: () => hotkeysManager.restoreDefaultBindings(),
+            hotkeysModule: hotkeys,
+          },
+        }),
+    },
+  ];
+
+  if (appConfig.oidc) {
+    menuOptions.push({
+      title: t('Header:Logout'),
+      icon: 'power-off',
+      onClick: async () => {
+        navigate(`/logout?redirect_uri=${encodeURIComponent(window.location.href)}`);
+      },
+    });
+  }
+
+  return (
+    <Header
+      menuOptions={menuOptions}
+      isReturnEnabled={!!appConfig.showStudyList}
+      onClickReturnButton={onClickReturnButton}
+      WhiteLabeling={appConfig.whiteLabeling}
+    >
+      <ErrorBoundary context="Primary Toolbar">
+        <div className="relative flex justify-center">
+          <Toolbar servicesManager={servicesManager} />
+        </div>
+      </ErrorBoundary>
+    </Header>
+  );
+}
+
+export default ViewerHeader;

extensions/default/src/ViewerLayout/index.tsx

@@ -1,24 +1,10 @@
 import React, { useEffect, useState } from 'react';
 import PropTypes from 'prop-types';
-import { useNavigate } from 'react-router-dom';
-import { useTranslation } from 'react-i18next';
-import { useLocation } from 'react-router';
 
-import {
+import { SidePanel, ErrorBoundary, LoadingIndicatorProgress } from '@ohif/ui';
-  SidePanel,
+import { ServicesManager, HangingProtocolService, CommandsManager } from '@ohif/core';
-  ErrorBoundary,
-  UserPreferences,
-  AboutModal,
-  Header,
-  useModal,
-  LoadingIndicatorProgress,
-} from '@ohif/ui';
-import i18n from '@ohif/i18n';
-import { ServicesManager, HangingProtocolService, hotkeys, CommandsManager } from '@ohif/core';
 import { useAppConfig } from '@state';
-import Toolbar from '../Toolbar/Toolbar';
+import ViewerHeader from './ViewerHeader';
-
-const { availableLanguages, defaultLanguage, currentLanguage } = i18n;
 
 function ViewerLayout({
   // From Extension Module Params
@@ -35,100 +21,9 @@ function ViewerLayout({
   rightPanelDefaultClosed = false,
 }): React.FunctionComponent {
   const [appConfig] = useAppConfig();
-  const navigate = useNavigate();
-  const location = useLocation();
-
-  const onClickReturnButton = () => {
-    const { pathname } = location;
-    const dataSourceIdx = pathname.indexOf('/', 1);
-    // const search =
-    //   dataSourceIdx === -1
-    //     ? undefined
-    //     : `datasources=${pathname.substring(dataSourceIdx + 1)}`;
-
-    // Todo: Handle parameters in a better way.
-    const query = new URLSearchParams(window.location.search);
-    const configUrl = query.get('configUrl');
-
-    const dataSourceName = pathname.substring(dataSourceIdx + 1);
-    const existingDataSource = extensionManager.getDataSources(dataSourceName);
-
-    const searchQuery = new URLSearchParams();
-    if (dataSourceIdx !== -1 && existingDataSource) {
-      searchQuery.append('datasources', pathname.substring(dataSourceIdx + 1));
-    }
-
-    if (configUrl) {
-      searchQuery.append('configUrl', configUrl);
-    }
-
-    navigate({
-      pathname: '/',
-      search: decodeURIComponent(searchQuery.toString()),
-    });
-  };
-
-  const { t } = useTranslation();
-  const { show, hide } = useModal();
-
-  const [showLoadingIndicator, setShowLoadingIndicator] = useState(appConfig.showLoadingIndicator);
 
   const { hangingProtocolService } = servicesManager.services;
-
+  const [showLoadingIndicator, setShowLoadingIndicator] = useState(appConfig.showLoadingIndicator);
-  const { hotkeyDefinitions, hotkeyDefaults } = hotkeysManager;
-  const versionNumber = process.env.VERSION_NUMBER;
-  const commitHash = process.env.COMMIT_HASH;
-
-  const menuOptions = [
-    {
-      title: t('Header:About'),
-      icon: 'info',
-      onClick: () =>
-        show({
-          content: AboutModal,
-          title: 'About OHIF Viewer',
-          contentProps: { versionNumber, commitHash },
-        }),
-    },
-    {
-      title: t('Header:Preferences'),
-      icon: 'settings',
-      onClick: () =>
-        show({
-          title: t('UserPreferencesModal:User Preferences'),
-          content: UserPreferences,
-          contentProps: {
-            hotkeyDefaults: hotkeysManager.getValidHotkeyDefinitions(hotkeyDefaults),
-            hotkeyDefinitions,
-            currentLanguage: currentLanguage(),
-            availableLanguages,
-            defaultLanguage,
-            onCancel: () => {
-              hotkeys.stopRecord();
-              hotkeys.unpause();
-              hide();
-            },
-            onSubmit: ({ hotkeyDefinitions, language }) => {
-              i18n.changeLanguage(language.value);
-              hotkeysManager.setHotkeys(hotkeyDefinitions);
-              hide();
-            },
-            onReset: () => hotkeysManager.restoreDefaultBindings(),
-            hotkeysModule: hotkeys,
-          },
-        }),
-    },
-  ];
-
-  if (appConfig.oidc) {
-    menuOptions.push({
-      title: t('Header:Logout'),
-      icon: 'power-off',
-      onClick: async () => {
-        navigate(`/logout?redirect_uri=${encodeURIComponent(window.location.href)}`);
-      },
-    });
-  }
 
   /**
    * Set body classes (tailwindcss) that don't allow vertical
@@ -210,18 +105,11 @@ function ViewerLayout({
 
   return (
     <div>
-      <Header
+      <ViewerHeader
-        menuOptions={menuOptions}
+        hotkeysManager={hotkeysManager}
-        isReturnEnabled={!!appConfig.showStudyList}
+        extensionManager={extensionManager}
-        onClickReturnButton={onClickReturnButton}
+        servicesManager={servicesManager}
-        WhiteLabeling={appConfig.whiteLabeling}
+      />
-      >
-        <ErrorBoundary context="Primary Toolbar">
-          <div className="relative flex justify-center">
-            <Toolbar servicesManager={servicesManager} />
-          </div>
-        </ErrorBoundary>
-      </Header>
       <div
         className="relative flex w-full flex-row flex-nowrap items-stretch overflow-hidden bg-black"
         style={{ height: 'calc(100vh - 52px' }}
@@ -281,6 +169,7 @@ ViewerLayout.propTypes = {
   rightPanelDefaultClosed: PropTypes.bool.isRequired,
   /** Responsible for rendering our grid of viewports; provided by consuming application */
   children: PropTypes.oneOfType([PropTypes.node, PropTypes.func]).isRequired,
+  viewports: PropTypes.array,
 };
 
 export default ViewerLayout;

platform/app/src/routes/Mode/Mode.tsx

@@ -164,7 +164,7 @@ export default function ModeRoute({
 
   // Preserve the old array interface for hotkeys
   const hotkeys = Array.isArray(hotkeyObj) ? hotkeyObj : hotkeyObj?.hotkeys;
-  const hotkeyName = hotkeyObj?.name || 'hotkey-definitions-v2';
+  const hotkeyName = hotkeyObj?.name || 'hotkey-definitions';
 
   // An undefined dataSourceName implies that the active data source that is already set in the ExtensionManager should be used.
   if (dataSourceName !== undefined) {

platform/app/src/routes/WorkList/WorkList.tsx

@@ -425,7 +425,9 @@ function WorkList({
             availableLanguages,
             defaultLanguage,
             onSubmit: state => {
-              i18n.changeLanguage(state.language.value);
+              if (state.language.value !== currentLanguage().value) {
+                i18n.changeLanguage(state.language.value);
+              }
               hotkeysManager.setHotkeys(state.hotkeyDefinitions);
               hide();
             },