openmrs · ibacher · Jan 16, 2025 · Jan 16, 2025
diff --git a/packages/apps/esm-devtools-app/src/devtools/devtools-popup.component.tsx b/packages/apps/esm-devtools-app/src/devtools/devtools-popup.component.tsx
@@ -1,29 +1,25 @@
 import React from 'react';
-import { Button } from '@carbon/react';
-import { CloseIcon } from '@openmrs/esm-framework';
+import { IconButton } from '@carbon/react';
+import { CloseIcon, getCoreTranslation } from '@openmrs/esm-framework';
 import ImportMap from './import-map.component';
 import styles from './devtools-popup.styles.scss';
 
-export default function DevToolsPopup(props: DevToolsPopupProps) {
+type DevToolsPopupProps = {
+  close(): void;
+  toggleOverridden(isOverridden: boolean): void;
+};
+
+function DevToolsPopup(props: DevToolsPopupProps) {
   return (
     <div className={styles.popup}>
       <ImportMap toggleOverridden={props.toggleOverridden} />
       <div className={styles.farRight}>
-        <Button
-          className={styles.closeButton}
-          kind="secondary"
-          renderIcon={() => <CloseIcon size={16} />}
-          iconDescription="Close"
-          onClick={props.close}
-          hasIconOnly
-          size="sm"
-        />
+        <IconButton kind="ghost" label={getCoreTranslation('close')} onClick={props.close} size="sm">
+          <CloseIcon size={16} />
+        </IconButton>
       </div>
     </div>
   );
 }
 
-type DevToolsPopupProps = {
-  close(): void;
-  toggleOverridden(isOverridden: boolean): void;
-};
+export default DevToolsPopup;
diff --git a/packages/apps/esm-devtools-app/src/devtools/devtools-popup.styles.scss b/packages/apps/esm-devtools-app/src/devtools/devtools-popup.styles.scss
@@ -36,17 +36,3 @@
   display: flex;
   z-index: 1000;
 }
-
-.closeButton {
-  align-items: center;
-}
-
-.closeButton:focus {
-  /*
-    there's some 3rd party css that turns these off using !important, so the only way to override them is to also use important.
-    this is why you don't use !important -- it makes it hard for other people to override it when they need to :(
-  */
-  outline: solid !important;
-  outline-width: thin !important;
-  outline-color: #e5e5e6 !important;
-}
diff --git a/packages/apps/esm-devtools-app/src/devtools/devtools.component.tsx b/packages/apps/esm-devtools-app/src/devtools/devtools.component.tsx
@@ -1,41 +1,39 @@
 import React, { useState } from 'react';
 import classNames from 'classnames';
+import { IconButton } from '@carbon/react';
 import { type AppProps } from 'single-spa';
+import { getCoreTranslation } from '@openmrs/esm-framework';
 import { importMapOverridden } from './import-map.component';
 import DevToolsPopup from './devtools-popup.component';
 import styles from './devtools.styles.css';
 
+const showDevTools = () => window.spaEnv === 'development' || Boolean(localStorage.getItem('openmrs:devtools'));
+
 export default function Root(props: AppProps) {
-  return window.spaEnv === 'development' || Boolean(localStorage.getItem('openmrs:devtools')) ? (
-    <DevTools {...props} />
-  ) : null;
+  return showDevTools() ? <DevTools {...props} /> : null;
 }
 
 function DevTools(props: AppProps) {
   const [devToolsOpen, setDevToolsOpen] = useState(false);
   const [isOverridden, setIsOverridden] = useState(importMapOverridden);
 
+  const toggleDevTools = () => setDevToolsOpen((devToolsOpen) => !devToolsOpen);
+  const toggleOverridden = (overridden: boolean) => setIsOverridden(overridden);
+
   return (
     <>
-      <div
-        role="button"
-        tabIndex={0}
-        onClick={toggleDevTools}
+      <IconButton
         className={classNames(styles.devtoolsTriggerButton, {
           [styles.overridden]: isOverridden,
         })}
+        kind="ghost"
+        label={getCoreTranslation('toggleDevTools')}
+        onClick={toggleDevTools}
+        size="md"
       >
         {'{\u00B7\u00B7\u00B7}'}
-      </div>
+      </IconButton>
       {devToolsOpen && <DevToolsPopup close={toggleDevTools} toggleOverridden={toggleOverridden} />}
     </>
   );
-
-  function toggleDevTools() {
-    setDevToolsOpen(!devToolsOpen);
-  }
-
-  function toggleOverridden(overridden) {
-    setIsOverridden(overridden);
-  }
 }
diff --git a/packages/apps/esm-devtools-app/src/devtools/devtools.styles.css b/packages/apps/esm-devtools-app/src/devtools/devtools.styles.css
@@ -1,17 +1,13 @@
 .devtoolsTriggerButton {
   z-index: 7900;
-  background-color: #c6c6c6;
-  height: 2.5rem !important;
-  width: 2.5rem !important;
+  background: #c6c6c6;
   bottom: 5rem;
   right: 0.25rem;
   position: fixed;
   border-radius: 0.25rem;
-  display: flex;
-  justify-content: center;
-  align-items: center;
 }
 
 .overridden {
   background-color: #da1e28;
+  color: white;
 }
diff --git a/.../src/devtools/devtools.component.test.tsx → ...vtools-app/src/devtools/devtools.test.tsx b/.../src/devtools/devtools.component.test.tsx → ...vtools-app/src/devtools/devtools.test.tsx
@@ -37,15 +37,15 @@ describe('DevTools', () => {
 
       render(<Root {...defaultProps} />);
 
-      expect(screen.getByRole('button', { name: '{···}' })).toBeInTheDocument();
+      expect(screen.getByRole('button', { name: /toggle dev tools/i })).toBeInTheDocument();
     });
 
     it('should render DevTools when the devtools localStorage flag is set', () => {
       localStorage.setItem('openmrs:devtools', 'true');
 
       render(<Root {...defaultProps} />);
 
-      expect(screen.getByRole('button', { name: '{···}' })).toBeInTheDocument();
+      expect(screen.getByRole('button', { name: /toggle dev tools/i })).toBeInTheDocument();
     });
   });
 
@@ -59,7 +59,7 @@ describe('DevTools', () => {
     it('should toggle DevToolsPopup when clicking trigger button', async () => {
       render(<Root {...defaultProps} />);
 
-      const triggerButton = screen.getByRole('button', { name: '{···}' });
+      const triggerButton = screen.getByRole('button', { name: /toggle dev tools/i });
       // Initially, popup should not be present
       expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
 

diff --git a/packages/apps/esm-devtools-app/src/devtools/import-map-list/import-map.modal.tsx b/packages/apps/esm-devtools-app/src/devtools/import-map-list/import-map.modal.tsx
@@ -1,9 +1,8 @@
 import React, { useCallback, useEffect, useRef, useState } from 'react';
 import { useTranslation } from 'react-i18next';
+import { Button, Form, ModalHeader, ModalBody, ModalFooter, Stack, TextInput } from '@carbon/react';
 import { addRoutesOverride, removeRoutesOverride } from '@openmrs/esm-framework/src/internal';
-import { Button, Form, ModalHeader, ModalBody, ModalFooter, TextInput } from '@carbon/react';
 import type { Module } from './types';
-import styles from './import-map.scss';
 
 type ImportMapModalProps = ({ module: Module; isNew: false } | { module: never; isNew: true }) & { close: () => void };
 
@@ -30,6 +29,7 @@ const ImportMapModal: React.FC<ImportMapModalProps> = ({ module, isNew, close })
   const [moduleName, setModuleName] = useState<string | undefined>(module?.moduleName);
   const moduleNameRef = useRef<HTMLInputElement>();
   const inputRef = useRef<HTMLInputElement>();
+
   const handleSubmit = useCallback(
     async (evt: Event) => {
       evt.preventDefault();
@@ -94,22 +94,28 @@ const ImportMapModal: React.FC<ImportMapModalProps> = ({ module, isNew, close })
       />
       <Form onSubmit={handleSubmit}>
         <ModalBody>
-          {isNew && (
-            <TextInput
-              id="module-name"
-              ref={moduleNameRef}
-              onChange={(evt: React.ChangeEvent<HTMLInputElement>) => {
-                evt.preventDefault();
-                setModuleName(evt.target.value);
-              }}
-              labelText={t('moduleName', 'Module Name')}
-            />
-          )}
-          {!isNew && (
-            <TextInput id="default-url" labelText={t('defaultUrl', 'Default URL')} value={module.defaultUrl} readOnly />
-          )}
-          <div className={styles.spacer} />
-          <TextInput id="override-url" ref={inputRef} labelText={t('overrideUrl', 'Override URL')} />
+          <Stack gap={5}>
+            {isNew && (
+              <TextInput
+                id="module-name"
+                ref={moduleNameRef}
+                onChange={(evt: React.ChangeEvent<HTMLInputElement>) => {
+                  evt.preventDefault();
+                  setModuleName(evt.target.value);
+                }}
+                labelText={t('moduleName', 'Module Name')}
+              />
+            )}
+            {!isNew && (
+              <TextInput
+                id="default-url"
+                labelText={t('defaultUrl', 'Default URL')}
+                value={module.defaultUrl}
+                readOnly
+              />
+            )}
+            <TextInput id="override-url" ref={inputRef} labelText={t('overrideUrl', 'Override URL')} />
+          </Stack>
         </ModalBody>
         <ModalFooter>
           <Button kind="secondary" onClick={close}>

diff --git a/packages/apps/esm-devtools-app/src/devtools/import-map-list/import-map.scss b/packages/apps/esm-devtools-app/src/devtools/import-map-list/import-map.scss
diff --git a/packages/apps/esm-devtools-app/src/devtools/import-map-list/list.component.tsx b/packages/apps/esm-devtools-app/src/devtools/import-map-list/list.component.tsx
@@ -239,6 +239,7 @@ const ImportMapList = forwardRef<HTMLDivElement>((props, ref) => {
           <TableBody className={styles.imoTableBody}>
             {nextOverriddenModules.map((mod) => (
               <TableRow
+                key={`${mod.moduleName}-next-override`}
                 role="button"
                 tabIndex={0}
                 onClick={() => dispatch({ type: 'show_edit_module_dialog', module: mod })}
@@ -263,6 +264,7 @@ const ImportMapList = forwardRef<HTMLDivElement>((props, ref) => {
             ))}
             {pendingRefreshDefaultModules.map((mod) => (
               <TableRow
+                key={`${mod.moduleName}-pending-refresh-default`}
                 role="button"
                 tabIndex={0}
                 onClick={() => dispatch({ type: 'show_edit_module_dialog', module: mod })}
@@ -287,6 +289,7 @@ const ImportMapList = forwardRef<HTMLDivElement>((props, ref) => {
             ))}
             {disabledOverrides.map((mod) => (
               <TableRow
+                key={`${mod.moduleName}-disabled-override`}
                 role="button"
                 tabIndex={0}
                 onClick={() => dispatch({ type: 'show_edit_module_dialog', module: mod })}
@@ -302,6 +305,7 @@ const ImportMapList = forwardRef<HTMLDivElement>((props, ref) => {
             ))}
             {overriddenModules.map((mod) => (
               <TableRow
+                key={`${mod.moduleName}-current-override`}
                 role="button"
                 tabIndex={0}
                 onClick={() => dispatch({ type: 'show_edit_module_dialog', module: mod })}
@@ -317,6 +321,7 @@ const ImportMapList = forwardRef<HTMLDivElement>((props, ref) => {
             ))}
             {externalOverrideModules.map((mod) => (
               <TableRow
+                key={`${mod.moduleName}-external-override`}
                 role="button"
                 tabIndex={0}
                 onClick={() => dispatch({ type: 'show_edit_module_dialog', module: mod })}
@@ -332,6 +337,7 @@ const ImportMapList = forwardRef<HTMLDivElement>((props, ref) => {
             ))}
             {defaultModules.map((mod) => (
               <TableRow
+                key={`${mod.moduleName}-default`}
                 role="button"
                 tabIndex={0}
                 onClick={() => dispatch({ type: 'show_edit_module_dialog', module: mod })}

diff --git a/packages/apps/esm-devtools-app/src/devtools/import-map-list/list.scss b/packages/apps/esm-devtools-app/src/devtools/import-map-list/list.scss
@@ -66,7 +66,7 @@
 }
 
 .imoCurrentOverrideText {
-  color: colors.$red-40;
+  color: colors.$red-70-hover;
 }
 
 .imoExternalOverride {

diff --git a/packages/apps/esm-devtools-app/src/devtools/import-map.component.tsx b/packages/apps/esm-devtools-app/src/devtools/import-map.component.tsx
@@ -2,17 +2,23 @@ import React, { useRef, useEffect } from 'react';
 import ImportMapList from './import-map-list/list.component';
 import styles from './import-map.styles.css';
 
-export default function ImportMap(props: ImportMapProps) {
+type ImportMapProps = {
+  toggleOverridden(overridden: boolean): void;
+};
+
+const IMPORT_MAP_CHANGE_EVENT = 'import-map-overrides:change';
+
+export default function ImportMap({ toggleOverridden }: ImportMapProps) {
   const importMapListRef = useRef<HTMLDivElement>(null);
 
   useEffect(() => {
-    window.addEventListener('import-map-overrides:change', handleImportMapChange);
-    return () => window.removeEventListener('import-map-overrides:change', handleImportMapChange);
+    const handleImportMapChange = () => {
+      toggleOverridden(importMapOverridden());
+    };
 
-    function handleImportMapChange() {
-      props.toggleOverridden(importMapOverridden());
-    }
-  }, [props]);
+    window.addEventListener(IMPORT_MAP_CHANGE_EVENT, handleImportMapChange);
+    return () => window.removeEventListener(IMPORT_MAP_CHANGE_EVENT, handleImportMapChange);
+  }, [toggleOverridden]);
 
   return <div className={styles.importMap}>{<ImportMapList ref={importMapListRef} />}</div>;
 }
@@ -24,7 +30,3 @@ export function importMapOverridden(): boolean {
 export function isOverriddenInImportMap(esmName: string): boolean {
   return window.importMapOverrides.getOverrideMap().imports.hasOwnProperty(esmName);
 }
-
-type ImportMapProps = {
-  toggleOverridden(overridden: boolean): void;
-};
diff --git a/packages/framework/esm-framework/docs/API.md b/packages/framework/esm-framework/docs/API.md
@@ -3380,7 +3380,7 @@ for more information about defining a config schema.
 
 #### Defined in
 
-[packages/framework/esm-config/src/module-config/module-config.ts:166](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-config/src/module-config/module-config.ts#L166)
+[packages/framework/esm-config/src/module-config/module-config.ts:165](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-config/src/module-config/module-config.ts#L165)
 
 ___
 
@@ -3412,7 +3412,7 @@ for more information about defining a config schema.
 
 #### Defined in
 
-[packages/framework/esm-config/src/module-config/module-config.ts:242](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-config/src/module-config/module-config.ts#L242)
+[packages/framework/esm-config/src/module-config/module-config.ts:241](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-config/src/module-config/module-config.ts#L241)
 
 ___
 
@@ -3444,7 +3444,7 @@ of the execution of a function.
 
 #### Defined in
 
-[packages/framework/esm-config/src/module-config/module-config.ts:275](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-config/src/module-config/module-config.ts#L275)
+[packages/framework/esm-config/src/module-config/module-config.ts:274](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-config/src/module-config/module-config.ts#L274)
 
 ___
 
@@ -3465,7 +3465,7 @@ ___
 
 #### Defined in
 
-[packages/framework/esm-config/src/module-config/module-config.ts:259](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-config/src/module-config/module-config.ts#L259)
+[packages/framework/esm-config/src/module-config/module-config.ts:258](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-config/src/module-config/module-config.ts#L258)
 
 ___
 
@@ -6902,7 +6902,7 @@ invalid key to this function will result in a type error.
 
 | Name | Type | Description |
 | :------ | :------ | :------ |
-| `key` | ``"error"`` \| ``"change"`` \| ``"close"`` \| ``"other"`` \| ``"Clinic"`` \| ``"actions"`` \| ``"address"`` \| ``"age"`` \| ``"cancel"`` \| ``"confirm"`` \| ``"contactAdministratorIfIssuePersists"`` \| ``"contactDetails"`` \| ``"errorCopy"`` \| ``"female"`` \| ``"loading"`` \| ``"male"`` \| ``"patientIdentifierSticker"`` \| ``"patientLists"`` \| ``"print"`` \| ``"printError"`` \| ``"printErrorExplainer"`` \| ``"printIdentifierSticker"`` \| ``"printing"`` \| ``"relationships"`` \| ``"resetOverrides"`` \| ``"save"`` \| ``"scriptLoadingFailed"`` \| ``"scriptLoadingError"`` \| ``"seeMoreLists"`` \| ``"sex"`` \| ``"showLess"`` \| ``"showMore"`` \| ``"unknown"`` \| ``"closeAllOpenedWorkspaces"`` \| ``"closingAllWorkspacesPromptBody"`` \| ``"closingAllWorkspacesPromptTitle"`` \| ``"discard"`` \| ``"hide"`` \| ``"maximize"`` \| ``"minimize"`` \| ``"openAnyway"`` \| ``"unsavedChangesInOpenedWorkspace"`` \| ``"unsavedChangesInWorkspace"`` \| ``"unsavedChangesTitleText"`` \| ``"workspaceHeader"`` \| ``"address1"`` \| ``"address2"`` \| ``"address3"`` \| ``"address4"`` \| ``"address5"`` \| ``"address6"`` \| ``"city"`` \| ``"cityVillage"`` \| ``"country"`` \| ``"countyDistrict"`` \| ``"district"`` \| ``"postalCode"`` \| ``"state"`` \| ``"stateProvince"`` | - |
+| `key` | ``"error"`` \| ``"change"`` \| ``"close"`` \| ``"other"`` \| ``"Clinic"`` \| ``"actions"`` \| ``"address"`` \| ``"age"`` \| ``"cancel"`` \| ``"confirm"`` \| ``"contactAdministratorIfIssuePersists"`` \| ``"contactDetails"`` \| ``"errorCopy"`` \| ``"female"`` \| ``"loading"`` \| ``"male"`` \| ``"patientIdentifierSticker"`` \| ``"patientLists"`` \| ``"print"`` \| ``"printError"`` \| ``"printErrorExplainer"`` \| ``"printIdentifierSticker"`` \| ``"printing"`` \| ``"relationships"`` \| ``"resetOverrides"`` \| ``"save"`` \| ``"scriptLoadingFailed"`` \| ``"scriptLoadingError"`` \| ``"seeMoreLists"`` \| ``"sex"`` \| ``"showLess"`` \| ``"showMore"`` \| ``"toggleDevTools"`` \| ``"unknown"`` \| ``"closeAllOpenedWorkspaces"`` \| ``"closingAllWorkspacesPromptBody"`` \| ``"closingAllWorkspacesPromptTitle"`` \| ``"discard"`` \| ``"hide"`` \| ``"maximize"`` \| ``"minimize"`` \| ``"openAnyway"`` \| ``"unsavedChangesInOpenedWorkspace"`` \| ``"unsavedChangesInWorkspace"`` \| ``"unsavedChangesTitleText"`` \| ``"workspaceHeader"`` \| ``"address1"`` \| ``"address2"`` \| ``"address3"`` \| ``"address4"`` \| ``"address5"`` \| ``"address6"`` \| ``"city"`` \| ``"cityVillage"`` \| ``"country"`` \| ``"countyDistrict"`` \| ``"district"`` \| ``"postalCode"`` \| ``"state"`` \| ``"stateProvince"`` | - |
 | `defaultText?` | `string` | - |
 | `options?` | `Omit`<`TOptions`<`StringMap`\>, ``"defaultValue"`` \| ``"ns"``\> | Object passed to the i18next `t` function. See https://www.i18next.com/translation-function/essentials#overview-options           for more information. `ns` and `defaultValue` are already set and may not be used. |
 

diff --git a/packages/framework/esm-translations/src/translations.ts b/packages/framework/esm-translations/src/translations.ts
@@ -72,5 +72,6 @@ export const coreTranslations = {
   sex: 'Sex',
   showLess: 'Show less',
   showMore: 'Show more',
+  toggleDevTools: 'Toggle dev tools',
   unknown: 'Unknown',
 };
diff --git a/packages/framework/esm-translations/translations/en.json b/packages/framework/esm-translations/translations/en.json
@@ -53,6 +53,7 @@
   "showMore": "Show more",
   "state": "State",
   "stateProvince": "State",
+  "toggleDevTools": "Toggle dev tools",
   "unknown": "Unknown",
   "unsavedChangesInWorkspace": "There are unsaved changes in {{workspaceName}}. Please save them before opening another workspace.",
   "unsavedChangesTitleText": "Unsaved changes",