-
Notifications
You must be signed in to change notification settings - Fork 281
O3-4740: Refactor Location Selector Component for Reusability and Move to esm-core Styleguide #1368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 9 commits
6bda39a
416f229
16fe8ce
7050f56
b3f7ac2
7bf81e4
512e97b
01b53cf
90e2911
6a76baf
d1d5b41
a3ae3a0
98f4911
6ca139d
6370269
cdfbc33
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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'; | ||||||
| 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; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
locationUuid shouldn't be null |
||||||
| Locationlabel: string; | ||||||
| comBoxLabel: string; | ||||||
| } | ||||||
|
|
||||||
| export const LocationSelector: React.FC<LocationSelectorProps> = ({ | ||||||
| selectedLocationUuid, | ||||||
| defaultLocationUuid, | ||||||
| locationTag, | ||||||
| locationsPerRequest = 50, | ||||||
| onChange, | ||||||
| Locationlabel, | ||||||
| comBoxLabel, | ||||||
Bawanthathilan marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| }) => { | ||||||
| const { t } = useTranslation(); | ||||||
| const [searchTerm, setSearchTerm] = useState<string>(''); | ||||||
| const [tempLocations, setTempLocations] = useState<Location[]>([]); | ||||||
Bawanthathilan marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
|
|
||||||
| 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; | ||||||
|
||||||
| 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')} | ||||||
|
||||||
| items={items} | ||||||
| initialSelectedItem={items.find((i) => i.id === defaultLocationUuid)} | ||||||
| selectedItem={items.find((i) => i.id === selectedLocationUuid)} | ||||||
|
Comment on lines
+67
to
+68
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when |
||||||
| itemToString={(item) => item?.label || ''} | ||||||
| onChange={(evt) => onChange(evt.selectedItem?.id)} | ||||||
| onInputChange={(value) => setSearchTerm(value)} | ||||||
ibacher marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| 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); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
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-utilsand not left in the styleguide.