Review feedback

openmrs · Nov 6, 2024 · 3d6ec39 · 3d6ec39
1 parent 4956f90
commit 3d6ec39
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 71 deletions.
diff --git a/packages/framework/esm-framework/docs/API.md b/packages/framework/esm-framework/docs/API.md
@@ -695,7 +695,7 @@ ___
 
 #### Defined in
 
-[packages/framework/esm-styleguide/src/icons/icons.tsx:648](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/icons/icons.tsx#L648)
+[packages/framework/esm-styleguide/src/icons/icons.tsx:664](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/icons/icons.tsx#L664)
 
 ___
 
@@ -1173,7 +1173,7 @@ ___
 
 #### Defined in
 
-[packages/framework/esm-styleguide/src/icons/icons.tsx:630](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/icons/icons.tsx#L638)
+[packages/framework/esm-styleguide/src/icons/icons.tsx:638](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/icons/icons.tsx#L638)
 
 ___
 
@@ -1233,7 +1233,7 @@ ___
 
 #### Defined in
 
-[packages/framework/esm-styleguide/src/icons/icons.tsx:635](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/icons/icons.tsx#L643)
+[packages/framework/esm-styleguide/src/icons/icons.tsx:643](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/icons/icons.tsx#L643)
 
 ___
 
@@ -1427,7 +1427,7 @@ Note this is an alias for ListCheckedIcon
 
 #### Defined in
 
-[packages/framework/esm-styleguide/src/icons/icons.tsx:642](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/icons/icons.tsx#L650)
+[packages/framework/esm-styleguide/src/icons/icons.tsx:650](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/icons/icons.tsx#L650)
 
 ___
 
@@ -1549,7 +1549,7 @@ This is a utility type for custom icons that use the svg-sprite-loader to bundle
 
 #### Defined in
 
-[packages/framework/esm-styleguide/src/icons/icons.tsx:656](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/icons/icons.tsx#L656)
+[packages/framework/esm-styleguide/src/icons/icons.tsx:672](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/icons/icons.tsx#L672)
 
 ___
 
@@ -1815,7 +1815,7 @@ ___
 
 #### Defined in
 
-[packages/framework/esm-styleguide/src/icons/icons.tsx:639](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/icons/icons.tsx#L655)
+[packages/framework/esm-styleguide/src/icons/icons.tsx:655](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/icons/icons.tsx#L655)
 
 ___
 
@@ -1899,7 +1899,7 @@ Note this is an alias for ShoppingCartArrowDownIcon
 
 #### Defined in
 
-[packages/framework/esm-styleguide/src/icons/icons.tsx:654](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/icons/icons.tsx#L662)
+[packages/framework/esm-styleguide/src/icons/icons.tsx:662](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/icons/icons.tsx#L662)
 
 ___
 
@@ -1955,9 +1955,10 @@ ___
 
 #### Defined in
 
-[packages/framework/esm-styleguide/src/icons/icons.tsx:471](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/icons/icons.tsx#L627)
+[packages/framework/esm-styleguide/src/icons/icons.tsx:627](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/icons/icons.tsx#L627)
 
 ___
+
 ### TableIcon
 
 • `Const` **TableIcon**: `MemoExoticComponent`<`ForwardRefExoticComponent`<[`IconProps`](API.md#iconprops) & `RefAttributes`<`SVGSVGElement`\>\>\>
@@ -6850,7 +6851,7 @@ ___
 
 #### Defined in
 
-[packages/framework/esm-styleguide/src/patient-banner/patient-info/patient-banner-patient-identifiers.component.tsx:43](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/patient-banner/patient-info/patient-banner-patient-identifiers.component.tsx#L43)
+[packages/framework/esm-styleguide/src/patient-banner/patient-info/patient-banner-patient-identifiers.component.tsx:41](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/patient-banner/patient-info/patient-banner-patient-identifiers.component.tsx#L41)
 
 ___
 
@@ -6870,7 +6871,7 @@ ___
 
 #### Defined in
 
-[packages/framework/esm-styleguide/src/patient-banner/patient-info/patient-banner-patient-info.component.tsx:48](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/patient-banner/patient-info/patient-banner-patient-info.component.tsx#L48)
+[packages/framework/esm-styleguide/src/patient-banner/patient-info/patient-banner-patient-info.component.tsx:43](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/patient-banner/patient-info/patient-banner-patient-info.component.tsx#L43)
 
 ___
 

diff --git a/packages/framework/esm-styleguide/src/patient-banner/patient-info/gender-icons.component.tsx b/packages/framework/esm-styleguide/src/patient-banner/patient-info/gender-icons.component.tsx
@@ -1,38 +1,30 @@
 import React from 'react';
 
-interface IconProps {
-  height?: number;
-  width?: number;
-}
-
-export const FemaleIcon: React.FC<IconProps> = ({ height = 20, width = 20 }) => (
-  <svg height={height} width={width} viewBox="0 0 20 20" fill="none" xmlns="http://www.w3.org/2000/svg">
-    <path
-      d="M10.625 12.457a5 5 0 1 0-1.25 0v1.293H6.25V15h3.125v2.5h1.25V15h3.125v-1.25h-3.125v-1.293zM6.25 7.5a3.75 3.75 0 1 1 7.5 0 3.75 3.75 0 0 1-7.5 0z"
-      fill="#525252"
-    />
+export const FemaleIcon: React.FC = () => (
+  <svg width="20" height="20" xmlns="http://www.w3.org/2000/svg">
+    <path d="M10.625 12.457a5 5 0 1 0-1.25 0v1.293H6.25V15h3.125v2.5h1.25V15h3.125v-1.25h-3.125v-1.293zM6.25 7.5a3.75 3.75 0 1 1 7.5 0 3.75 3.75 0 0 1-7.5 0z" />
   </svg>
 );
 
-export const MaleIcon: React.FC<IconProps> = ({ height = 20, width = 20 }) => (
-  <svg height={height} width={width} viewBox="0 0 20 20">
+export const MaleIcon: React.FC = () => (
+  <svg height="20" width="20" viewBox="0 0 20 20">
     <circle cx="8" cy="12" r="5" stroke="#525252" strokeWidth="1" fill="none" />
     <path d="M12 8L17 3M17 3H13M17 3V7" stroke="#525252" strokeWidth="1" strokeLinecap="round" />
   </svg>
 );
 
-export const UnknownIcon: React.FC<IconProps> = ({ height = 20, width = 20 }) => (
-  <svg height={height} width={width} xmlns="http://www.w3.org/2000/svg">
+export const UnknownIcon: React.FC = () => (
+  <svg height="20" width="20" xmlns="http://www.w3.org/2000/svg">
     <circle cx="16" cy="22.5" r="1.5" />
     <path d="M17 19h-2v-4h2c1.103 0 2-.897 2-2s-.897-2-2-2h-2c-1.103 0-2 .897-2 2v.5h-2V13c0-2.206 1.794-4 4-4h2c2.206 0 4 1.794 4 4s-1.794 4-4 4v2z" />
     <path d="M29.391 14.527 17.473 2.609A2.078 2.078 0 0 0 16 2c-.533 0-1.067.203-1.473.609L2.609 14.527C2.203 14.933 2 15.466 2 16s.203 1.067.609 1.473L14.526 29.39c.407.407.941.61 1.474.61s1.067-.203 1.473-.609L29.39 17.474c.407-.407.61-.94.61-1.474s-.203-1.067-.609-1.473zM16 28.036 3.965 16 16 3.964 28.036 16 16 28.036z" />
-    <path d="M0 0h32v32H0z" fill="currentColor" />
+    <path d="M0 0h32v32H0z" />
   </svg>
 );
 
-export const OtherIcon: React.FC<IconProps> = ({ height = 20, width = 20 }) => (
-  <svg height={height} width={width} xmlns="http://www.w3.org/2000/svg">
+export const OtherIcon: React.FC = () => (
+  <svg height="20" width="20" xmlns="http://www.w3.org/2000/svg">
     <path d="M22 2v2h4.586l-6.402 6.401a6.947 6.947 0 0 0-8.368 0L10.414 9 13 6.414 11.586 5 9 7.586 5.414 4H10V2H2v8h2V5.414L7.586 9 5 11.585 6.414 13 9 10.414l1.401 1.401A6.979 6.979 0 0 0 15 22.92V25h-4v2h4v3h2v-3h4v-2h-4v-2.08a6.979 6.979 0 0 0 4.598-11.104L28 5.414V10h2V2Zm-6 19a5 5 0 1 1 5-5 5.006 5.006 0 0 1-5 5Z" />
-    <path d="M0 0h32v32H0z" fill="currentColor" />
+    <path d="M0 0h32v32H0z" />
   </svg>
 );
diff --git a/...yleguide/src/patient-banner/patient-info/patient-banner-patient-identifiers.component.tsx b/...yleguide/src/patient-banner/patient-info/patient-banner-patient-identifiers.component.tsx
@@ -1,6 +1,6 @@
 /** @module @category UI */
 import React from 'react';
-import { Tag } from '@carbon/react';
+import { FormLabel, Tag } from '@carbon/react';
 import { useConfig, usePrimaryIdentifierCode } from '@openmrs/esm-react-utils';
 import { type StyleguideConfigObject } from '../../config-schema';
 import styles from './patient-banner-patient-info.module.scss';
@@ -21,7 +21,7 @@ function PrimaryIdentifier({ showIdentifierLabel, type, value }: IdentifierProps
     <div className={styles.primaryIdentifier}>
       {showIdentifierLabel && (
         <Tag className={styles.tag} type="gray" title={type?.text}>
-          <span className={styles.label}>{type?.text}: </span>
+          {type?.text && <span className={styles.label}>{type.text}: </span>}
           <span className={styles.value}>{value}</span>
         </Tag>
       )}
@@ -31,12 +31,10 @@ function PrimaryIdentifier({ showIdentifierLabel, type, value }: IdentifierProps
 
 function SecondaryIdentifier({ showIdentifierLabel, type, value }: IdentifierProps) {
   return (
-    <label htmlFor="identifier" className={styles.secondaryIdentifier}>
+    <FormLabel className={styles.secondaryIdentifier} id={`patient-banner-identifier-${value}`}>
       {showIdentifierLabel && <span className={styles.label}>{type?.text}: </span>}
-      <span id="identifier" className={styles.value}>
-        {value}
-      </span>
-    </label>
+      <span className={styles.value}>{value}</span>
+    </FormLabel>
   );
 }
 
@@ -57,15 +55,13 @@ export function PatientBannerPatientIdentifier({
     <div className={styles.identifiers}>
       {filteredIdentifiers?.length
         ? filteredIdentifiers.map(({ value, type }, index) => (
-            <div key={value}>
-              <div key={value} className={styles.identifierTag}>
-                {index > 0 && <span className={styles.separator}>&middot;</span>}
-                {type?.coding?.[0]?.code === primaryIdentifierCode ? (
-                  <PrimaryIdentifier value={value} type={type} showIdentifierLabel={showIdentifierLabel} />
-                ) : (
-                  <SecondaryIdentifier value={value} type={type} showIdentifierLabel={showIdentifierLabel} />
-                )}
-              </div>
+            <div className={styles.identifierTag} key={value}>
+              {index > 0 && <span className={styles.separator}>&middot;</span>}
+              {type?.coding?.[0]?.code === primaryIdentifierCode ? (
+                <PrimaryIdentifier value={value} type={type} showIdentifierLabel={showIdentifierLabel} />
+              ) : (
+                <SecondaryIdentifier value={value} type={type} showIdentifierLabel={showIdentifierLabel} />
+              )}
             </div>
           ))
         : ''}

diff --git a/.../esm-styleguide/src/patient-banner/patient-info/patient-banner-patient-info.component.tsx b/.../esm-styleguide/src/patient-banner/patient-info/patient-banner-patient-info.component.tsx
@@ -1,5 +1,5 @@
 /** @module @category UI */
-import React from 'react';
+import React, { useMemo } from 'react';
 import classNames from 'classnames';
 import { ExtensionSlot } from '@openmrs/esm-react-utils';
 import { getCoreTranslation } from '@openmrs/esm-translations';
@@ -12,19 +12,21 @@ export interface PatientBannerPatientInfoProps {
   patient: fhir.Patient;
 }
 
+type Gender = 'female' | 'male' | 'other' | 'unknown';
+
 interface GenderIconProps {
-  gender: string;
+  gender: keyof typeof GENDER_ICONS;
 }
 
-const GenderIcon = ({ gender }: GenderIconProps) => {
-  const GENDER_ICONS = {
-    Female: FemaleIcon,
-    Male: MaleIcon,
-    Other: OtherIcon,
-    Unknown: UnknownIcon,
-  } as const;
+const GENDER_ICONS = {
+  Female: FemaleIcon,
+  Male: MaleIcon,
+  Other: OtherIcon,
+  Unknown: UnknownIcon,
+} as const;
 
-  const IconComponent = GENDER_ICONS[gender as keyof typeof GENDER_ICONS];
+const GenderIcon = ({ gender }: GenderIconProps) => {
+  const IconComponent = GENDER_ICONS[gender];
 
   if (!IconComponent) {
     return null;
@@ -34,21 +36,16 @@ const GenderIcon = ({ gender }: GenderIconProps) => {
 };
 
 const getGender = (gender: string): string => {
-  const genderTranslations = {
-    male: 'male',
-    female: 'female',
-    other: 'other',
-    unknown: 'unknown',
-  } as const;
-
-  const key = gender.toLowerCase() as keyof typeof genderTranslations;
-  return getCoreTranslation(genderTranslations[key], gender);
+  const key = gender.toLowerCase() as Gender;
+  return getCoreTranslation(key, gender);
 };
 
 export function PatientBannerPatientInfo({ patient }: PatientBannerPatientInfoProps) {
   const name = `${patient?.name?.[0]?.given?.join(' ')} ${patient?.name?.[0]?.family}`;
   const gender = patient?.gender && getGender(patient.gender);
 
+  const extensionState = useMemo(() => ({ patientUuid: patient.id, patient }), [patient.id, patient]);
+
   return (
     <div className={styles.patientInfo}>
       <div className={classNames(styles.row, styles.patientNameRow)}>
@@ -57,16 +54,12 @@ export function PatientBannerPatientInfo({ patient }: PatientBannerPatientInfoPr
 
           {gender && (
             <div className={styles.gender}>
-              <GenderIcon gender={gender} />
+              <GenderIcon gender={gender as keyof typeof GENDER_ICONS} />
               <span>{gender}</span>
             </div>
           )}
 
-          <ExtensionSlot
-            className={styles.flexRow}
-            name="patient-banner-tags-slot"
-            state={{ patientUuid: patient.id, patient }}
-          />
+          <ExtensionSlot className={styles.tagsSlot} name="patient-banner-tags-slot" state={extensionState} />
         </div>
       </div>
       <div className={styles.demographics}>

diff --git a/...rk/esm-styleguide/src/patient-banner/patient-info/patient-banner-patient-info.module.scss b/...rk/esm-styleguide/src/patient-banner/patient-info/patient-banner-patient-info.module.scss
@@ -1,7 +1,7 @@
 @use '@carbon/colors';
 @use '@carbon/layout';
 @use '@carbon/type';
-@use '~@openmrs/esm-styleguide/src/vars' as *;
+@use '@openmrs/esm-styleguide/src/vars' as *;
 
 .container {
   border-top: 1px solid $ui-03;
@@ -66,6 +66,10 @@
   display: flex;
   flex-flow: row wrap;
   align-items: center;
+}
+
+.tagsSlot {
+  @extend .flexRow;
   margin: 0 layout.$spacing-03;
 }