Skip to content
Open
Show file tree
Hide file tree
Changes from 9 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
@@ -0,0 +1 @@
export * from './location-selector.component';
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import React, { useState, useEffect, useMemo } from 'react';
import { useTranslation } from 'react-i18next';
import { ComboBox } from '@carbon/react';
import { useLocationByUuid, useLocations } from '../location-picker/location-picker.resource';
Copy link
Member

Choose a reason for hiding this comment

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

Data fetching hooks should be moved to esm-react-utils and not left in the styleguide.

import styles from './location-selector.module.scss';

interface Location {
resource: { id: string; name: string };
}

interface LocationSelectorProps {
selectedLocationUuid?: string;
defaultLocationUuid?: string;
locationTag?: string;
locationsPerRequest?: number;
onChange: (locationUuid?: string) => void;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
onChange: (locationUuid?: string) => void;
onChange: (locationUuid: string) => void;

locationUuid shouldn't be null

Locationlabel: string;
comBoxLabel: string;
}

export const LocationSelector: React.FC<LocationSelectorProps> = ({
selectedLocationUuid,
defaultLocationUuid,
locationTag,
locationsPerRequest = 50,
onChange,
Locationlabel,
comBoxLabel,
}) => {
const { t } = useTranslation();
const [searchTerm, setSearchTerm] = useState<string>('');
const [tempLocations, setTempLocations] = useState<Location[]>([]);

const { location: defaultLocation } = useLocationByUuid(defaultLocationUuid);
const { locations: fetchedLocations = [] } = useLocations(locationTag, locationsPerRequest, searchTerm);

useEffect(() => {
if (fetchedLocations.length > 0) {
setTempLocations(fetchedLocations);
}
}, [fetchedLocations]);

const effectiveLocations = useMemo<Location[]>(() => {
const base = fetchedLocations.length > 0 ? fetchedLocations : tempLocations;
Copy link
Member

Choose a reason for hiding this comment

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

When the user searches for a non-existing location, fetchedLocations will be empty, and the component will fall back to tempLocations. However, in this case the expected behavior is to show an empty list. Can you confirm if that’s what currently happens?

Copy link
Member

Choose a reason for hiding this comment

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

Also, could we rename the variable base to something more descriptive of what it actually holds? That would make the code easier to follow.

Copy link
Member

Choose a reason for hiding this comment

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

Were you able to find a way to resolve this one?

if (defaultLocation && !searchTerm) {
return [defaultLocation, ...base.filter((loc) => loc.resource.id !== defaultLocationUuid)];
}
return base;
}, [defaultLocation, defaultLocationUuid, fetchedLocations, tempLocations, searchTerm]);

const items = useMemo(
() =>
effectiveLocations.map((loc) => ({
id: loc.resource.id,
label: loc.resource.name,
})),
[effectiveLocations],
);

return (
<section data-testid="combo">
<div className={styles.sectionTitle}>{Locationlabel}</div>
<ComboBox
aria-label={comBoxLabel}
id="location"
invalidText={t('Required')}
Copy link
Member

Choose a reason for hiding this comment

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

We can't translated text like this in the styleguide. Please use the getCoreTranslation() function from esm-translations.

items={items}
initialSelectedItem={items.find((i) => i.id === defaultLocationUuid)}
selectedItem={items.find((i) => i.id === selectedLocationUuid)}
Comment on lines +67 to +68
Copy link
Member

Choose a reason for hiding this comment

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

when selectedItem is provided, the ComboBox behaves as a controlled component, so initialSelectedItem is ignored after the first render. To keep things clearer and avoid confusion it might be better to remove initialSelectedItem here and rely only on selectedItem. What do you think?

itemToString={(item) => item?.label || ''}
onChange={(evt) => onChange(evt.selectedItem?.id)}
onInputChange={(value) => setSearchTerm(value)}
titleText={comBoxLabel}
/>
</section>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
@use '@carbon/colors';
@use '@carbon/layout';
@use '@carbon/type';
@use '@openmrs/esm-styleguide/src/vars' as *;

.sectionTitle {
@include type.type-style('heading-compact-02');
color: colors.$gray-70;
margin: 0 0 layout.$spacing-03 0;
}

.locationNotFound {
@include type.type-style('heading-compact-01');
color: $text-02;
margin-bottom: layout.$spacing-05;
}

.emptyState {
display: flex;
flex-direction: column;
justify-content: center;
margin: layout.$spacing-06 auto 0;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
/* eslint-disable */
import React from 'react';
import { describe, expect, it, vi } from 'vitest';
import { act, render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { type LoggedInUser, type Session } from '@openmrs/esm-api';
import { useConfig, useSession } from '@openmrs/esm-react-utils';
import {
inpatientWardResponse,
locationResponseForNonExistingSearch,
outpatientClinicResponse,
mockLoginLocations,
} from '../../__mocks__/locations.mock';
import { mockConfig } from '../../__mocks__/config.mock';
import { LocationSelector } from './location-selector.component';

const validLocationUuid = '1ce1b7d4-c865-4178-82b0-5932e51503d6';
const inpatientWardLocationUuid = 'ba685651-ed3b-4e63-9b35-78893060758a';
const inpatientWardLocation = 'Inpatient Ward';

const mockUseConfig = vi.mocked(useConfig);
const mockUseSession = vi.mocked(useSession);

mockUseConfig.mockReturnValue(mockConfig);
mockUseSession.mockReturnValue({
user: {
display: 'Testy McTesterface',
uuid: '90bd24b3-e700-46b0-a5ef-c85afdfededd',
userProperties: {},
} as LoggedInUser,
} as Session);

vi.mock('@openmrs/esm-api', async () => ({
...(await import('@openmrs/esm-api')),
openmrsFetch: vi.fn((url) => {
if (url === `/ws/fhir2/R4/Location?_id=${inpatientWardLocationUuid}`) {
return inpatientWardResponse;
}
if (url === '/ws/fhir2/R4/Location?_summary=data&_count=50&name%3Acontains=search_for_no_location') {
return locationResponseForNonExistingSearch;
}
if (url === '/ws/fhir2/R4/Location?_summary=data&_count=50&name%3Acontains=outpatient') {
return outpatientClinicResponse;
}
return mockLoginLocations;
}),
setSessionLocation: vi.fn().mockResolvedValue({}),
setUserProperties: vi.fn().mockResolvedValue({}),
}));

describe('LocationPicker', () => {
it('renders correctly', async () => {
await act(async () => {
render(
<LocationSelector
comBoxLabel="search for a location"
Locationlabel="Login Location"
selectedLocationUuid={inpatientWardLocationUuid}
onChange={vi.fn()}
/>,
);
});
expect(screen.getByRole('combobox')).toBeInTheDocument();
});

it('renders a list of login locations', async () => {
await act(async () => {
render(
<LocationSelector
comBoxLabel="search for a location"
Locationlabel="Login Location"
selectedLocationUuid={inpatientWardLocationUuid}
onChange={vi.fn()}
/>,
);
});

expect(
screen.getByRole('combobox', {
name: /search for a location/i,
}),
).toBeInTheDocument();

const combo = screen.getByRole('combobox', {
name: /search for a location/i,
});
await userEvent.click(combo);

const locations = screen.getAllByRole('option');
expect(locations.length).toBe(4);
const expectedLocations = [/community outreach/, /inpatient ward/, /mobile clinic/, /outpatient clinic/];
expectedLocations.forEach((row) =>
expect(screen.getByRole('option', { name: new RegExp(row, 'i') })).toBeInTheDocument(),
);
});

it('selects the provided selectedLocation when the component is rendered', async () => {
await act(async () => {
render(
<LocationSelector
comBoxLabel="search for a location"
Locationlabel="Login Location"
selectedLocationUuid={inpatientWardLocationUuid}
onChange={vi.fn()}
/>,
);
});

const combo = screen.getByRole('combobox', {
name: /search for a location/i,
});

expect(combo).toHaveValue('Inpatient Ward');
});

it('loads the default location on top of the list', async () => {
await act(async () => {
render(
<LocationSelector
comBoxLabel="search for a location"
Locationlabel="Login Location"
selectedLocationUuid={inpatientWardLocationUuid}
defaultLocationUuid={inpatientWardLocationUuid}
onChange={vi.fn()}
/>,
);
});

expect(
screen.getByRole('combobox', {
name: /search for a location/i,
}),
).toBeInTheDocument();

const combo = screen.getByRole('combobox', {
name: /search for a location/i,
});
await userEvent.click(combo);

const locations = screen.getAllByRole('option');

expect(locations.length).toBe(4);
locations[0].setAttribute('name', inpatientWardLocation);
expect(locations[0].getAttribute('name')).toBe(inpatientWardLocation);
});
});
1 change: 1 addition & 0 deletions packages/framework/esm-styleguide/src/public.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export * from './pictograms/pictograms';
export { type StyleguideConfigObject } from './config-schema';
export * from './location-picker';
export * from './diagnosis-tags';
export * from './location-selector';
export {
launchWorkspace2,
launchWorkspaceGroup2,
Expand Down
Loading