(feat) Restore recently searched patients functionality in compact search #1345
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tient search This restores the recently searched patients functionality in the compact patient search. It also introduces some refactoring of the code to make it more modular and easier to maintain. These changes include: - Tweaking the configuration schema to improve the names and descriptions of the config properties. This also includes renaming the `patientResultUrl` config property to `patientChartUrl`, which is more descriptive of what it is used for. Implementers will need to update their configurations to use the new key. - Cleaning up the logic in the `useInfinitePatientSearch`, `useRecentlyViewedPatients` and `useRestPatients` hooks to make them more modular and easier to maintain. This includes removing rendering logic from the hooks. We can now drop the `.tsx` extension for the patient search resource file as it no longer contains any JSX. - Annotating invocations of the `useConfig` hook with the `PatientSearchConfig` type for better type inference. - Adding some error handling logic to the `addViewedPatientAndCloseSearchResults` function. - Preloading user properties from the REST API when the user hovers over the search icon in the navbar using the SWR preload API. - Preserving recently searched patients when revalidating the recently viewed patients list by enabling the `keepPreviousData` option in the `useInfinitePatientSearch` hook for better UX. Without this, the UI would show a loading state when revalidating the recently viewed patients list which could be potentially jarring for the user.
|
Size Change: -21.8 kB (-0.34%) Total Size: 6.47 MB
ℹ️ View Unchanged
|
denniskigen
commented
Oct 15, 2024
| if (selectPatientAction) { | ||
| selectPatientAction(patients[index].uuid); | ||
| } else { | ||
| navigate({ | ||
| to: `${interpolateString(config.search.patientResultUrl, { |
There was a problem hiding this comment.
interpolateString returns a string, so there's no need for the additional interpolation.
Comment on lines
-35
to
-41
| const handleClear = useCallback(() => { | ||
| setSearchTerm(''); | ||
| }, [setSearchTerm]); | ||
| const handleChange = useCallback((val) => setSearchTerm(val), [setSearchTerm]); | ||
|
|
||
| const handleReset = useCallback(() => { | ||
| setSearchTerm(''); | ||
| }, [setSearchTerm]); |
There was a problem hiding this comment.
handleReset and handleClear do the same thing.
| @@ -95,6 +118,24 @@ const CompactPatientSearchComponent: React.FC<CompactPatientSearchProps> = ({ | |||
| } | |||
| }, [focusedResult, bannerContainerRef, handleFocusToInput]); | |||
|
|
|||
| useEffect(() => { | |||
| if (fetchError) { | |||
There was a problem hiding this comment.
I've moved these here so that the resource file doesn't have to deal with any rendering concerns. A post-merge commit should change the resource file extension from .tsx to .ts.
Comment on lines
+82
to
+86
| {isValidating && ( | ||
| <span className={styles.validationIcon}> | ||
| <InlineLoading className={styles.spinner} /> | ||
| </span> | ||
| )} |
There was a problem hiding this comment.
This will render a loading spinner when we're revalidating recently searched patients. Additionally, because we've specified the keepPreviousData option in the useInfinitePatientSearch hook, the previous data will be returned while the new data is being fetched.
| @@ -2,17 +2,18 @@ import { Type, validators } from '@openmrs/esm-framework'; | |||
|
|
|||
| export const configSchema = { | |||
| search: { | |||
| // TODO: Rename this config property | |||
| patientResultUrl: { | |||
| patientChartUrl: { | |||
There was a problem hiding this comment.
This config property key change is a BREAKING change for implementers' config schemas. It will be communicated appropriately in the release notes when the next release gets cut.
| shouldFetch ? getPatientUrl : null, | ||
| openmrsFetch, | ||
| { | ||
| keepPreviousData: true, |
There was a problem hiding this comment.
|
FYI @ciaranduffy @paulsonder see the explanation in the |
ibacher
approved these changes
Oct 17, 2024
packages/esm-patient-search-app/src/compact-patient-search-extension/index.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Ian <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requirements
Summary
This PR restores the recently searched patients functionality in the compact patient search. It also introduces some refactoring of the code to make it more modular and easier to maintain. These changes include:
patientResultUrlconfig property topatientChartUrl, which is more descriptive of what it is used for. Implementers will need to update their configurations to use the new key.useInfinitePatientSearch,useRecentlyViewedPatientsanduseRestPatientshooks to make them more modular and easier to maintain. This includes removing rendering logic from the hooks. We can now drop the.tsxextension for the patient search resource file as it no longer contains any JSX.useConfighook with thePatientSearchConfigtype for better type inference.addViewedPatientAndCloseSearchResultsfunction.keepPreviousDataoption in theuseInfinitePatientSearchhook for better UX. Without this, the UI would show a loading state when revalidating the recently viewed patients list which could be potentially jarring for the user.Screenshots
recently-viewed-charts.mp4
Related Issue
Other
This implementation is a slight departure from what was envisioned in the original design. It's also a departure from a similar feature in O2:
The key difference is that both of those implementations would add any recently opened charts to the top of the list (regardless of whether they were recently searched or not). This implementation does not do that. Instead, it adds recently searched patients to the top of the list, but it does not add recently viewed charts. So, for example, if you clicked on patient in the Active Visits list and launched their chart, they would not show up at the top of the recently searched list. They would show up in the list if you went back and searched for them, but that's it. The reason for this is that in O2, every "chart" page calls EMR API's
ApplicationEvent#patientViewed()service, which pushes the patient's ID into the list of recently viewed patients. The challenge is that none of this logic is exposed via the REST API. To get around this, we'd have to expose this mechanism by extending the REST API. In lieu of this, we'll have to make do with this approach for now.