Patient Tabs Fixes #10642
Patient Tabs Fixes #10642
Conversation
WalkthroughThe changes update localization strings to replace consultation terminology with encounter terminology across English and Malayalam locale files. Several components have been modified to integrate internationalization using the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
src/components/Encounter/CreateEncounterForm.tsx (2)
151-161:⚠️ Potential issueAdd error handling to mutation.
The error handling has been removed from the mutation. This could lead to a poor user experience when errors occur.
const { mutate: createEncounter, isPending } = useMutation({ mutationFn: mutate(routes.encounter.create), onSuccess: (data: Encounter) => { toast.success("Encounter created successfully"); setIsOpen(false); form.reset(); queryClient.invalidateQueries({ queryKey: ["encounters", patientId] }); onSuccess?.(); navigate(`/facility/${facilityId}/encounter/${data.id}/updates`); }, + onError: (error: any) => { + toast.error(error?.message || t("error_creating_encounter")); + }, });
255-304: 🛠️ Refactor suggestionTranslate SelectContent options.
The options in SelectContent are still in English and need to be internationalized.
<SelectContent> - <SelectItem value="in_progress">In Progress</SelectItem> - <SelectItem value="planned">Planned</SelectItem> - <SelectItem value="on_hold">On Hold</SelectItem> + <SelectItem value="in_progress">{t("in_progress")}</SelectItem> + <SelectItem value="planned">{t("planned")}</SelectItem> + <SelectItem value="on_hold">{t("on_hold")}</SelectItem> </SelectContent>Similarly, update the priority options below.
🧹 Nitpick comments (3)
src/components/Patient/PatientDetailsTab/patientUpdates.tsx (1)
84-88: Consider improving error handling for user display.The current implementation could be more robust in handling missing user data.
Consider this improvement:
-by {update.created_by?.first_name || ""}{" "} -{update.created_by?.last_name || ""} -{update.created_by?.user_type && - ` (${update.created_by?.user_type})`} +by {update.created_by ? ( + <> + {[ + update.created_by.first_name, + update.created_by.last_name, + update.created_by.user_type && `(${update.created_by.user_type})` + ].filter(Boolean).join(" ")} + </> +) : "Unknown User"}src/components/Patient/PatientDetailsTab/Appointments.tsx (1)
32-33: LGTM! Consider adding error handling for missing facilityId.The changes correctly handle null/undefined
facilityIdand prevent unnecessary query execution. However, you might want to consider showing a user-friendly message whenfacilityIdis missing.const { data } = useQuery({ queryKey: ["patient-appointments", patientId], queryFn: query(scheduleApis.appointments.list, { pathParams: { facility_id: facilityId ?? "" }, queryParams: { patient: patientId, limit: 100 }, }), enabled: !!facilityId, + placeholderData: { results: [] }, });Also applies to: 35-35
src/components/Patient/PatientDetailsTab/EncounterHistory.tsx (1)
50-102: LGTM! Consider improving button accessibility.The conditional rendering logic is well-implemented, providing appropriate actions based on context. However, the create encounter button could benefit from improved accessibility.
<Button variant="outline" data-cy="create-encounter-button" + aria-label={t("create_encounter")} className="group relative w-full h-[100px] md:h-[200px] overflow-hidden border-0 p-0 shadow-md hover:shadow-xl transition-all duration-300 justify-center" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
public/locale/en.json(5 hunks)public/locale/ml.json(0 hunks)src/components/Encounter/CreateEncounterForm.tsx(7 hunks)src/components/Files/FilesTab.tsx(0 hunks)src/components/Patient/PatientDetailsTab/Appointments.tsx(1 hunks)src/components/Patient/PatientDetailsTab/Demography.tsx(1 hunks)src/components/Patient/PatientDetailsTab/EncounterHistory.tsx(3 hunks)src/components/Patient/PatientDetailsTab/PatientFiles.tsx(0 hunks)src/components/Patient/PatientDetailsTab/ResourceRequests.tsx(1 hunks)src/components/Patient/PatientDetailsTab/index.tsx(1 hunks)src/components/Patient/PatientDetailsTab/patientUpdates.tsx(3 hunks)src/pages/Encounters/tabs/EncounterFilesTab.tsx(1 hunks)
💤 Files with no reviewable changes (3)
- src/components/Patient/PatientDetailsTab/PatientFiles.tsx
- public/locale/ml.json
- src/components/Files/FilesTab.tsx
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: cypress-run (1)
🔇 Additional comments (9)
src/pages/Encounters/tabs/EncounterFilesTab.tsx (1)
5-7: LGTM! Clean and focused implementation.The component correctly passes only the necessary props to FilesTab.
src/components/Patient/PatientDetailsTab/index.tsx (2)
14-18: LGTM! Interface update aligns with PR objectives.Making
facilityIdoptional allows the interface to be used in both facility and non-facility contexts.
20-84:❓ Verification inconclusive
Verify that all tab components handle optional facilityId correctly.
The components listed in both
patientTabsandfacilityPatientTabsshould gracefully handle cases wherefacilityIdis undefined.
🏁 Script executed:
#!/bin/bash # Description: Check if all tab components properly handle optional facilityId # Search for facilityId usage in tab components echo "Checking facilityId usage in tab components..." for component in Demography EncounterHistory HealthProfileSummary Updates ResourceRequests PatientUsers PatientFilesTab Appointments QuestionnaireResponsesList; do echo "\nChecking $component..." rg -A 5 "facilityId[?]?:" --type typescript doneLength of output: 2908
ACTION REQUIRED: Verify Optional
facilityIdHandling in Tab ComponentsThe initial automated search for
facilityIdusage did not complete as expected due to an unrecognized file type. Please re-run the following refined script to verify that each tab component (listed below) gracefully handles cases wherefacilityIdis undefined. If the revised script still produces inconclusive results, perform a manual review of the components.
- Components to verify:
- Demography
- EncounterHistory
- HealthProfileSummary
- Updates
- ResourceRequests
- PatientUsers
- PatientFilesTab
- Appointments
- QuestionnaireResponsesList
You can run this script from the repository root:
Please verify that the output confirms proper handling of optional
facilityId(i.e., no erroneous assumptions or runtime errors whenfacilityIdis undefined). Adjust the components if necessary based on your findings.
🏁 Script executed:
[run_scripts] #!/bin/bash # Description: Check facilityId usage in tab components across TS/TSX files for component in Demography EncounterHistory HealthProfileSummary Updates ResourceRequests PatientUsers PatientFilesTab Appointments QuestionnaireResponsesList; do file=$(fd -I -e ts -e tsx "$component" | head -n 1) if [[ -z "$file" ]]; then echo "Component file for '$component' not found." else echo "Checking '$component' in file: $file" rg -A 5 'facilityId[?]?[:=]' "$file" fi doneLength of output: 6171
ACTION: Verify Optional
facilityIdHandling in Tab ComponentsBelow are our observations from the automated checks along with points that require your manual verification:
Demography:
Uses a fallback withfacilityId={facilityId ?? ""}, which appears to handle undefined values appropriately.EncounterHistory:
PassesfacilityIddirectly asfacilityId={facilityId}. Please double-check that the component internally handles cases whenfacilityIdis undefined.HealthProfileSummary, Updates, ResourceRequests, PatientUsers, Appointments, and QuestionnaireResponsesList:
The script did not find any usage offacilityIdin these files. Confirm that if these components are expected to operate with an optionalfacilityId, either they do not require it or they handle the undefined case gracefully.PatientFilesTab:
The file for this component was not located by the automated search. Ensure that the correct file is referenced in the tab definitions and that it manages an optionalfacilityIdappropriately.Please verify these points manually or with an updated automated check to ensure that all tab components safely handle an undefined
facilityId.src/components/Patient/PatientDetailsTab/ResourceRequests.tsx (1)
67-79: LGTM! Well-implemented conditional rendering.The "Create Resource Request" button is correctly shown only when
facilityIdis available, which aligns with the PR objectives. The implementation also maintains proper i18n support.src/components/Patient/PatientDetailsTab/patientUpdates.tsx (1)
23-25: LGTM! Clean prop usage.Removal of unused
facilityIdprop aligns with PR objectives.src/components/Patient/PatientDetailsTab/Demography.tsx (1)
146-146: LGTM! Consistent handling of facilityId.The addition of facilityId with nullish coalescing maintains consistency with other components.
public/locale/en.json (3)
292-292: Update 'add_encounter' Localization KeyThe new key
"add_encounter": "Add Encounter"replaces outdated terminology and aligns with the PR objectives. Ensure that all UI components on the patient page reference this key correctly when prompting users to add a new encounter.
469-469: Introduce 'begin_clinical_encounter' Key for Actionable GuidanceThe newly added
"begin_clinical_encounter": "Begin a new clinical encounter for {{patientName}}. Select the appropriate encounter type, status, and priority to ensure proper documentation and care delivery."key offers a clear, descriptive prompt for initiating encounters. Please verify that the placeholder{{patientName}}correctly interpolates patient names in the UI and that this message is used uniformly where clinical encounters are started.
1409-1409: Revise Encounter History MessageThe updated key
"no_encounter_history": "No encounter history available"successfully shifts the terminology from consultation to encounter. Make sure that all references in the patient tabs (and related components) now use this key and that any previous calls referencing "consultation history" have been updated accordingly.
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
Jacobjeevan
added
needs testing
needs review
and removed
merge conflict
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/Files/FilesTab.tsx (2)
675-708: Simplify duplicate conditional checks.The condition
type === "encounter" && subPage === "discharge_summary"is repeated. Consider extracting it to improve maintainability.+const isDischargeTab = type === "encounter" && subPage === "discharge_summary"; -{type === "encounter" && subPage === "discharge_summary" && ( +{isDischargeTab && ( <> <Button variant="outline_primary" // ... refresh button code /> </> )} -{type === "encounter" && subPage === "discharge_summary" && ( +{isDischargeTab && ( <> <Button variant="primary" // ... generate button code /> </> )}🧰 Tools
🪛 Biome (1.9.4)
[error] 676-690: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
[error] 693-707: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
695-705: Enhance loading state feedback.Consider adding a loading spinner to provide better visual feedback during discharge summary generation.
<Button variant="primary" className="flex flex-row items-center" onClick={() => generateDischargeSummary()} disabled={isGenerating} > - <CareIcon icon="l-file-medical" className="hidden sm:block" /> + <CareIcon + icon={isGenerating ? "l-spinner" : "l-file-medical"} + className={cn("hidden sm:block", isGenerating && "animate-spin")} + /> <span> {isGenerating ? t("generating") : t("generate_discharge_summary")} </span> </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Files/FilesTab.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
🔇 Additional comments (2)
src/components/Files/FilesTab.tsx (2)
654-654: Consider simplifying className combination.The className
"grid w-auto grid-cols-2 sm:w-fit"contains potentially conflicting width classes (w-autoandw-fit). This was also noted in a previous review.Consider using only one width class:
-<TabsList className="grid w-auto grid-cols-2 sm:w-fit"> +<TabsList className="grid grid-cols-2 sm:w-fit">
58-64:❓ Verification inconclusive
Verify the removal of facilityId prop.
The removal of
facilityIdfromFilesTabPropsaligns with the PR objectives. However, let's verify that all consumers of this component have been updated accordingly.
🏁 Script executed:
#!/bin/bash # Search for any remaining usages of facilityId prop with FilesTab ast-grep --pattern 'FilesTab $props={$$$facilityId: $_$$$}'Length of output: 61
Verify removal of
facilityIdprop usageThe removal of the
facilityIdproperty in theFilesTabPropsinterface aligns with the PR’s objectives. A preliminary AST search for patterns related tofacilityIdin FilesTab did not produce any output. However, since the earlier script output was inconclusive, please perform a manual verification (or run a supplementary search with tools likerg) to ensure there are no lingering references in the codebase.
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/Files/FilesTab.tsx (1)
673-706: Improve button accessibility and loading states.The button group implementation could be enhanced:
- Add
aria-labelto describe each button's purpose- Add loading spinner component during discharge summary generation
<Button variant="primary" className="flex flex-row items-center" onClick={() => generateDischargeSummary()} disabled={isGenerating} + aria-label={t("generate_discharge_summary")} > - <CareIcon icon="l-file-medical" className="hidden sm:block" /> + {isGenerating ? ( + <CareIcon icon="l-spinner" className="animate-spin" /> + ) : ( + <CareIcon icon="l-file-medical" className="hidden sm:block" /> + )} <span> {isGenerating ? t("generating") : t("generate_discharge_summary")} </span> </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
public/locale/en.json(5 hunks)src/components/Encounter/CreateEncounterForm.tsx(7 hunks)src/components/Files/FilesTab.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- public/locale/en.json
- src/components/Encounter/CreateEncounterForm.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (1)
src/components/Files/FilesTab.tsx (1)
58-64: LGTM! Interface changes align with PR objectives.The removal of
facilityIdfromFilesTabPropsaligns with the PR's objective of restricting facility-specific functionality to the facility/patient page.
| {type === "encounter" && ( | ||
| <TabsList className="grid w-auto grid-cols-2 sm:w-fit"> | ||
| <TabsTrigger value="all" asChild> | ||
| <Link | ||
| className="text-gray-600" | ||
| href={`/facility/${encounter?.facility.id}/patient/${patientId}/encounter/${encounter?.id}/files/all`} | ||
| > | ||
| {t("all")} | ||
| </Link> | ||
| </TabsTrigger> | ||
| <TabsTrigger value="discharge_summary" asChild> | ||
| <Link | ||
| className="text-gray-600" | ||
| href={`/facility/${encounter?.facility.id}/patient/${patientId}/encounter/${encounter?.id}/files/discharge_summary`} | ||
| > | ||
| {t("discharge_summary")} | ||
| </Link> | ||
| </TabsTrigger> | ||
| </TabsList> | ||
| )} |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Verify the accessibility of the tab navigation.
The tab navigation implementation needs accessibility improvements:
- The
TabsListshould have anaria-labelto describe its purpose - The
TabsTriggercomponents should haverole="tab"andaria-selectedattributes
🏁 Script executed:
#!/bin/bash
# Check if there are any other tab lists in the codebase that implement these accessibility attributes
ast-grep --pattern 'TabsList aria-label="$_"'Length of output: 47
Enhance Accessibility in Tab Navigation for FilesTab Component
In src/components/Files/FilesTab.tsx (lines 653–672), please address the following accessibility improvements:
- TabsList: Add an
aria-labelattribute that describes the purpose of this tab group. - TabsTrigger: Update each component to include
role="tab"and a dynamicaria-selectedattribute reflecting the current selection state.
These modifications will ensure that screen readers and keyboard navigation work as expected. Manual verification of these changes across similar components is also recommended.
github-actions
bot
added
merge conflict
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/components/Encounter/CreateEncounterForm.tsx (1)
154-166:⚠️ Potential issueAdd error handling to mutation.
The error handling callback has been removed from the useMutation hook. This could lead to unhandled errors and poor user experience when encounter creation fails.
const { mutate: createEncounter, isPending } = useMutation({ mutationFn: mutate(routes.encounter.create), + onError: (error) => { + toast.error(t("error_creating_encounter")); + console.error("Error creating encounter:", error); + }, onSuccess: (data: Encounter) => { // ... }, });
🧹 Nitpick comments (2)
src/components/Encounter/CreateEncounterForm.tsx (2)
79-81: Consider moving the translation call out of the schema definition.Using
t()directly in the schema definition could cause issues if the translation function is not available during schema initialization. Consider defining the message when creating the form instead.- organizations: z.array(z.string()).min(1, { - message: t("at_least_one_department_is_required"), - }), + organizations: z.array(z.string()).min(1),Then in the form:
const form = useForm<z.infer<typeof encounterFormSchema>>({ resolver: zodResolver(encounterFormSchema), defaultValues: { // ... }, messages: { organizations: { min: t("at_least_one_department_is_required") } } });
197-199: Add aria-label to icon-only button.The Stethoscope icon button lacks an aria-label, which is necessary for screen readers.
<Stethoscope + aria-label={t("create_encounter")} className="mr-4 size-6" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json(5 hunks)src/components/Encounter/CreateEncounterForm.tsx(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (2)
src/components/Encounter/CreateEncounterForm.tsx (1)
16-16: Well implemented i18n integration!The internationalization implementation is thorough and follows best practices:
- Proper hook usage
- Consistent translation key naming
- All user-facing strings are internationalized
Also applies to: 142-142, 198-198, 204-204, 206-206, 220-221, 257-257, 342-342
public/locale/en.json (1)
294-294: Well-structured localization changes.The translation key changes properly support the shift from consultation to encounter terminology:
- Consistent key naming
- Clear and descriptive messages
- Proper removal of obsolete consultation keys
Also applies to: 472-472, 1428-1428
|
LGTM |
|
@Jacobjeevan Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Chores