Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
color: $text-02;
width: 100%;
border-top: 1px solid $ui-03;
container-type: inline-size;
container-name: contact-details;
}

.heading {
Expand All @@ -22,6 +24,10 @@
&:first-child {
border-bottom: 1px solid $ui-03;
}

@container contact-details (max-width: 40rem) {
flex-direction: column;
}
}

.row > .col {
Expand All @@ -31,6 +37,13 @@
&:nth-of-type(2n + 1) {
border-right: 1px solid $ui-03;
}

@container contact-details (max-width: 40rem) {
&:nth-of-type(2n + 1) {
border-right: none;
border-bottom: 1px solid $ui-03;
}
}
}

.row li {
Expand All @@ -39,7 +52,7 @@

.relationship {
display: flex;
flex-flow: row nowrap;
flex-flow: row wrap;
}

.relationship div {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/** @module @category UI */
import classNames from 'classnames';
import React from 'react';
import { FormLabel, Tag } from '@carbon/react';
import { useConfig, usePrimaryIdentifierCode } from '@openmrs/esm-react-utils';
Expand All @@ -14,6 +15,7 @@ interface IdentifiersProps {
interface PatientBannerPatientIdentifiersProps {
identifiers: fhir.Identifier[] | undefined;
showIdentifierLabel: boolean;
showAllIdentifiers?: boolean;
}
Comment on lines 15 to 19

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
interface PatientBannerPatientIdentifiersProps {
identifiers: fhir.Identifier[] | undefined;
showIdentifierLabel: boolean;
showLeadingSeparator?: boolean;
showAllIdentifiers?: boolean;
}
interface PatientBannerPatientIdentifiersProps {
identifiers: fhir.Identifier[] | undefined;
showIdentifierLabel: boolean;
showAllIdentifiers?: boolean;
}

Both components go out through @openmrs/esm-framework, so this becomes public API that only its sibling component would ever set. The destructured default below and the conditional class can go with it:

<span key={value} className={classNames(styles.identifier, styles.withSeparator)}>


function PrimaryIdentifier({ showIdentifierLabel, type, value }: IdentifiersProps) {
Expand All @@ -39,6 +41,7 @@ function SecondaryIdentifier({ showIdentifierLabel, type, value }: IdentifiersPr
export function PatientBannerPatientIdentifiers({
identifiers,
showIdentifierLabel,
showAllIdentifiers = true,
}: PatientBannerPatientIdentifiersProps) {
const { excludePatientIdentifierCodeTypes } = useConfig<StyleguideConfigObject>();
const { primaryIdentifierCode } = usePrimaryIdentifierCode();
Expand All @@ -49,20 +52,26 @@ export function PatientBannerPatientIdentifiers({
return code && !excludePatientIdentifierCodeTypes?.uuids.includes(code);
}) ?? [];

const primaryIdentifiers = filteredIdentifiers.filter(
(identifier) => identifier.type?.coding?.[0]?.code === primaryIdentifierCode,
);

let visibleIdentifiers = filteredIdentifiers;
if (!showAllIdentifiers) {
visibleIdentifiers = primaryIdentifiers.length > 0 ? primaryIdentifiers : filteredIdentifiers.slice(0, 1);
}

return (
<>
{filteredIdentifiers?.length
? filteredIdentifiers.map(({ value, type }, index) => (
<React.Fragment key={value}>
<span className={styles.identifier}>
{type?.coding?.[0]?.code === primaryIdentifierCode ? (
<PrimaryIdentifier showIdentifierLabel={showIdentifierLabel} type={type} value={value} />
) : (
<SecondaryIdentifier showIdentifierLabel={showIdentifierLabel} type={type} value={value} />
)}
</span>
{index < filteredIdentifiers.length - 1 && <span className={styles.separator}>&middot;</span>}
</React.Fragment>
{visibleIdentifiers?.length
? visibleIdentifiers.map(({ value, type }, index) => (
<span key={value} className={classNames(styles.identifier, styles.withSeparator)}>
{type?.coding?.[0]?.code === primaryIdentifierCode ? (
<PrimaryIdentifier showIdentifierLabel={showIdentifierLabel} type={type} value={value} />
) : (
<SecondaryIdentifier showIdentifierLabel={showIdentifierLabel} type={type} value={value} />
)}
</span>
))
: ''}
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,36 @@ describe('PatientBannerPatientIdentifiers', () => {
expect(screen.getByText(/openmrs id/i)).toBeInTheDocument();
expect(screen.queryByText(/national id/i)).not.toBeInTheDocument();
});

it('falls back to showing the first identifier when showAllIdentifiers is false but the primary identifier code is unresolved', () => {
mockUsePrimaryIdentifierCode.mockReturnValue({
primaryIdentifierCode: undefined,
isLoading: false,
error: undefined,
});

render(
<PatientBannerPatientIdentifiers identifiers={mockIdentifiers} showIdentifierLabel showAllIdentifiers={false} />,
);

expect(screen.getByText(/openmrs id/i)).toBeInTheDocument();
expect(screen.getByText(/100gej/i)).toBeInTheDocument();
expect(screen.queryByText(/national id/i)).not.toBeInTheDocument();
});

it('shows only the primary identifier when showAllIdentifiers is false and the primary identifier code resolves', () => {
mockUsePrimaryIdentifierCode.mockReturnValue({
primaryIdentifierCode: '4281ec43-388b-4c25-8bb2-deaff0867b2c',
isLoading: false,
error: undefined,
});

render(
<PatientBannerPatientIdentifiers identifiers={mockIdentifiers} showIdentifierLabel showAllIdentifiers={false} />,
);

expect(screen.getByText(/national id/i)).toBeInTheDocument();
expect(screen.getByText(/123456789/i)).toBeInTheDocument();
expect(screen.queryByText(/openmrs id/i)).not.toBeInTheDocument();
});
Comment on lines +93 to +107

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('shows only the primary identifier when showAllIdentifiers is false and the primary identifier code resolves', () => {
render(
<PatientBannerPatientIdentifiers identifiers={mockIdentifiers} showIdentifierLabel showAllIdentifiers={false} />,
);
expect(screen.getByText(/openmrs id/i)).toBeInTheDocument();
expect(screen.getByText(/100gej/i)).toBeInTheDocument();
expect(screen.queryByText(/national id/i)).not.toBeInTheDocument();
});
it('shows only the primary identifier when showAllIdentifiers is false and the primary identifier code resolves', () => {
mockUsePrimaryIdentifierCode.mockReturnValue({
primaryIdentifierCode: '4281ec43-388b-4c25-8bb2-deaff0867b2c',
isLoading: false,
error: undefined,
});
render(
<PatientBannerPatientIdentifiers identifiers={mockIdentifiers} showIdentifierLabel showAllIdentifiers={false} />,
);
expect(screen.getByText(/national id/i)).toBeInTheDocument();
expect(screen.getByText(/123456789/i)).toBeInTheDocument();
expect(screen.queryByText(/openmrs id/i)).not.toBeInTheDocument();
});

This one can't fail as written. The primary identifier is also mockIdentifiers[0], so it passes whether we pick the primary or just take the first one. I swapped the branch for a bare filteredIdentifiers.slice(0, 1) and all six tests still passed. Making the primary the second identifier makes it fail on that mutation.

});
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ interface PatientBannerPatientInfoProps {
* affect how / if they should be rendered
*/
renderedFrom?: string;
showAllIdentifiers?: boolean;
}

type Gender = 'female' | 'male' | 'other' | 'unknown';
Expand Down Expand Up @@ -58,7 +59,11 @@ const getGender = (gender: string) => {
};
};

export function PatientBannerPatientInfo({ patient, renderedFrom }: PatientBannerPatientInfoProps) {
export function PatientBannerPatientInfo({
patient,
renderedFrom,
showAllIdentifiers = true,
}: Readonly<PatientBannerPatientInfoProps>) {
const name = getPatientName(patient);
const genderInfo = patient?.gender && getGender(patient.gender);

Expand Down Expand Up @@ -87,12 +92,14 @@ export function PatientBannerPatientInfo({ patient, renderedFrom }: PatientBanne
{patient.birthDate && (
<>
<span>{age(patient.birthDate)}</span>
<span className={styles.separator}>&middot;</span>
<span>{formatPartialDate(patient.birthDate, { time: false })}</span>
<span className={styles.separator}>&middot;</span>
<span className={styles.withSeparator}>{formatPartialDate(patient.birthDate, { time: false })}</span>
</>
)}
<PatientBannerPatientIdentifiers identifiers={patient.identifier} showIdentifierLabel />
<PatientBannerPatientIdentifiers
identifiers={patient.identifier}
showIdentifierLabel
showAllIdentifiers={showAllIdentifiers}
/>
<ExtensionSlot className={styles.extensionSlot} name="patient-banner-bottom-slot" state={extensionState} />
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@
background-color: $ui-01;
}

.withSeparator:not(:first-child)::before {
content: '\00B7';
margin-inline-end: layout.$spacing-02;
}

.patientBanner {
display: flex;
}
Expand All @@ -22,6 +27,7 @@
color: colors.$gray-100;
font-weight: 600;
margin-right: layout.$spacing-02;
overflow-wrap: anywhere;
}

.patientAvatar {
Expand All @@ -43,6 +49,7 @@
flex-direction: column;
padding: layout.$spacing-05 layout.$spacing-03 layout.$spacing-05 0;
width: 100%;
min-width: 0;
}

.demographics {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,10 @@ export interface PatientPhotoProps {
alt?: string;
}

function getInitials(name: string, maxInitials = 3): string {
return name
.split(/\s+/)
.filter(Boolean)
.slice(0, maxInitials)
.map((part) => part[0])
.join('');
function getInitials(name: string, maxInitials = 2): string {
const parts = name.split(/\s+/).filter(Boolean);
const selected = parts.length > maxInitials ? [parts[0], parts.at(-1)] : parts;
return selected.map((part) => part![0]).join('');
}
Comment on lines +16 to 20

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maxInitials no longer means "at most this many initials": getInitials(name, 4) on a five-part name returns two, not four. It reads as a threshold now, and the only call site (line 104) passes no second argument, so the parameter is mostly something for the next reader to puzzle over. Dropping it also removes the need for the non-null assertion.

Suggested change
function getInitials(name: string, maxInitials = 2): string {
const parts = name.split(/\s+/).filter(Boolean);
const selected = parts.length > maxInitials ? [parts[0], parts.at(-1)] : parts;
return selected.map((part) => part![0]).join('');
}
function getInitials(name: string): string {
const parts = name.split(/\s+/).filter(Boolean);
const selected = parts.length > 2 ? [parts[0], parts[parts.length - 1]] : parts;
return selected.map((part) => part[0]).join('');
}

Identical output for every input, so take it or leave it.


/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,4 +184,17 @@ describe('PatientPhoto', () => {
// Restore the original Image constructor
window.Image = originalImage;
});

it('caps initials at two characters, using first and last name parts, for multi-part names', () => {
mockUsePatientPhoto.mockReturnValue({
isLoading: false,
data: null,
error: undefined,
});

render(<PatientPhoto patientUuid={patientUuid} patientName="Wanjiru Achieng Nakato" />);

const avatar = screen.getByTitle('Wanjiru Achieng Nakato');
expect(avatar).toHaveTextContent('WN');
});
});
Loading