New Patient Search and Register UI #9400
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces comprehensive updates across multiple components of a healthcare application. The changes span dependency version updates, internationalization improvements, UI component enhancements, and patient management functionalities. Key modifications include introducing new components like Changes
Suggested labels
Suggested reviewers
Poem
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 (
|
Deploying care-fe with
|
| Latest commit: |
4f17160
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://470907f0.care-fe.pages.dev |
| Branch Preview URL: | https://patient-registration.care-fe.pages.dev |
|
👋 Hi, @shivankacker, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
Bug ReportMajor Functionality
Minor Functionality
Question needs clarification
|
|
@nihal467 we are not including insurance and volunteer details right now.
|
|
Made all changes, except :
This will add a lot of translations to manage. The label of the field is sufficient enough i think
I let it be as what it already was (specified in translations)
Will completely replace with a new phone number field after this PR.
Will fix these later |
|
👋 Hi, @shivankacker, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
Fix the below Issue before requesting another review
Issue that can be taken in another PR
Question
|
| @@ -64,7 +64,7 @@ | |||
| "DOMESTIC_HEALTHCARE_SUPPORT__PAID_CAREGIVER": "Paid caregiver", | |||
| "GENDER__1": "Male", | |||
| "GENDER__2": "Female", | |||
| "GENDER__3": "Non-binary", | |||
| "GENDER__3": "Transgender", | |||
There was a problem hiding this comment.
This reminds me of something I came across while testing the user redesign (forgot about it 😅 ) - Back end designates 3 as Non-binary, so we need to update it there as well.
|
👋 Hi, @shivankacker, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
Just noticed
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🔭 Outside diff range comments (2)
src/pages/Facility/FacilityDetailsPage.tsx (2)
Line range hint
44-54: Enhance error handling for user data fetchingThe current error handling could be improved in several ways:
- The generic error message doesn't provide enough context for debugging
- The
silent: trueflag might suppress important errorsConsider this improvement:
const { data: docResponse } = useQuery< RequestResult<PaginatedResponse<UserAssignedModel>> >({ queryKey: [routes.getFacilityUsers, id], queryFn: async () => { const response = await request(routes.getFacilityUsers, { pathParams: { facility_id: id }, - silent: true, }); if (response.res?.status !== 200) { - Notification.Error({ msg: "Error while fetching users data" }); + Notification.Error({ + msg: t("error_fetching_users", { + status: response.res?.status, + facility: id + }) + }); } return response; }, });
Line range hint
37-39: Fix pagination implementationThere are two issues with the current pagination:
- Using
users.lengthfortotalCountis incorrect as it only represents the current page count- The hard-coded limit of 18 might not be optimal for all screen sizes
Consider these fixes:
const { Pagination } = useFilters({ - limit: 18, + limit: 12, // Standard grid-friendly number }); // Later in the code -<Pagination totalCount={users.length ?? 0} /> +<Pagination totalCount={docResponse?.data?.count ?? 0} />Also applies to: 146-146
♻️ Duplicate comments (2)
src/Utils/utils.ts (1)
243-243:⚠️ Potential issueReconsider phone number length validation
The strict length check might reject valid phone numbers:
- Toll-free numbers and landlines can have different lengths
- Returning an empty string for invalid numbers could mask validation errors
Consider this alternative implementation:
-if (parsedNumber.length < 12) return ""; +// Allow numbers between 8 and 15 digits (inclusive) to support various formats +if (parsedNumber.length < 8 || parsedNumber.length > 15) return undefined;public/locale/en.json (1)
67-67:⚠️ Potential issueConsider updating backend enum to match frontend gender options.
The gender option for key
"GENDER__3"has been changed from "Non-binary" to "Transgender". As noted in a previous review comment by Jacobjeevan, the backend designates 3 as "Non-binary", so this needs to be synchronized.
🧹 Nitpick comments (18)
src/components/Patient/PatientIndex.tsx (3)
1-8: Optimize and group imports to reduce overhead.While these imports are correct and necessary, consider whether you can lazy-load certain components or group related common utilities together. This can help streamline the import process and potentially improve application performance.
207-227: Add a keydown event fallback for SHIFT+P.Although you use useKeyboardShortcut, some environments or custom browsers might not support it well. Consider adding a fallback keydown listener or instructions for alternative input methods.
236-361: Consider caution with large queries and frequent re-renders.The tabbed interface triggers patient queries once isValidSearch is true. If the user toggles tabs frequently, it could lead to repeated fetches. Consider memoizing or implementing query caching to reduce redundant calls.
src/components/Patient/PatientFilter.tsx (1)
795-799: Children passing practice.At line 1044 in the static analysis hints, there's a recommendation to avoid passing children as a prop. React convention encourages directly embedding children. Consider refactoring if the usage can be replaced with a direct rendering pattern.
src/components/Patient/PatientRegistration.tsx (1)
355-404: Validate date_of_birth thoroughly for boundary checks.Although there's a regex check for date format, consider validating real feasible dates (e.g., year range, closing unrealistic entries). Tools like date-fns or dayjs can help. This helps avoid invalid but properly formatted dates.
src/components/ui/input.tsx (1)
18-23: Consider improving label accessibility.While the label implementation is good, consider adding
aria-required="true"when the field is required for better screen reader support.<Label className="mb-2"> {label} - {required && <span className="text-red-500">*</span>} + {required && ( + <span className="text-red-500" aria-hidden="true"> + * + </span> + )} </Label>src/components/ui/radio-group.tsx (2)
19-24: Maintain consistent label accessibility.For consistency with the Input component, apply the same accessibility improvements to the label implementation.
<Label className="mb-4"> {label} - {props.required && <span className="text-red-500">*</span>} + {props.required && ( + <span className="text-red-500" aria-hidden="true"> + * + </span> + )} </Label>
25-31: Consider adding role attribute for better accessibility.While using Radix UI primitives provides good accessibility, consider adding an explicit role for the error messages container.
- <InputErrors errors={errors} /> + <div role="alert" aria-live="polite"> + <InputErrors errors={errors} /> + </div>src/Routers/routes/PatientRoutes.tsx (1)
Line range hint
1-53: Consider adding route documentation.Given the significant changes to the routing structure and the introduction of new components, consider adding JSDoc comments to document the purpose and requirements of each route.
src/CAREUI/misc/SectionNavigator.tsx (2)
28-37: Consider adjusting IntersectionObserver thresholdThe current threshold of 0.1 means sections are considered "visible" when only 10% is in view. This might lead to premature section activation, especially for shorter sections.
Consider adjusting the configuration:
- { rootMargin: "0px 0px -80% 0px", threshold: 0.1 }, + { rootMargin: "0px 0px -80% 0px", threshold: 0.3 },
15-56: Consider debouncing scroll updatesThe current implementation might trigger frequent state updates during rapid scrolling, which could impact performance.
Consider adding debounce to the scroll updates:
import { debounce } from "lodash"; // Inside useEffect const debouncedUpdate = debounce(updateActiveSection, 100); // Use debouncedUpdate instead of updateActiveSectionsrc/components/ui/tabs.tsx (1)
75-96: Consider adding error boundary for section contentThe TabbedSections component directly renders user-provided section content without error handling.
Consider wrapping the section content in an error boundary to gracefully handle rendering errors:
import { ErrorBoundary } from 'react-error-boundary'; const TabbedSections = (props: { tabs: { label: string; value: string; section: React.ReactNode }[]; }) => { // ... existing code ... return ( <> <SectionTabs tabs={props.tabs} activeTab={activeTab} onChange={(t) => setActiveTab(t)} /> <div> {props.tabs.map(({ value, section }) => ( <ErrorBoundary key={value} fallback={<div>Error loading section</div>}> <div hidden={activeTab !== value}> {section} </div> </ErrorBoundary> ))} </div> </> ); };src/components/Common/Breadcrumbs.tsx (1)
Line range hint
10-16: Consider moving MENU_TAGS to a configuration fileThe MENU_TAGS mapping contains business logic that might need frequent updates as new routes are added.
Consider moving this to a dedicated configuration file for better maintainability:
// src/config/navigation.ts export const MENU_TAGS: Record<string, string> = { facility: "Facilities", patients: "Patients", // ... rest of the tags };src/components/ui/select.tsx (1)
27-50: Consider extracting label rendering to a separate componentThe label rendering logic could be extracted to improve maintainability and reusability across other form components.
+const SelectLabel: React.FC<{ label?: string; required?: boolean }> = ({ + label, + required, +}) => + label ? ( + <Label className="mb-2"> + {label} + {required && <span className="text-red-500">*</span>} + </Label> + ) : null; const SelectTrigger = React.forwardRef< React.ElementRef<typeof SelectPrimitive.Trigger>, React.ComponentPropsWithoutRef<typeof SelectPrimitive.Trigger> & { label?: string; errors?: string[]; required?: boolean; } >(({ className, children, label, errors, required, ...props }, ref) => ( <> - {label && ( - <Label className="mb-2"> - {label} - {required && <span className="text-red-500">*</span>} - </Label> - )} + <SelectLabel label={label} required={required} /> <SelectPrimitive.Trigger // ... rest of the component /> </> ));src/components/Common/SearchByMultipleFields.tsx (1)
254-254: Consider adding flex properties to the wrapper divThe wrapper div might need flex properties to ensure proper layout alignment with the search icon button.
-<div className="w-full">{renderSearchInput}</div> +<div className="w-full flex items-center">{renderSearchInput}</div>src/components/Patient/ManagePatients.tsx (2)
842-857: Improve accessibility of Doctor Connect buttonThe button needs accessibility improvements:
- Missing aria-label for better screen reader support
- Missing keyboard shortcut
<ButtonV2 className="w-full lg:w-fit" id="doctor-connect-patient-button" + aria-label="Connect with doctor" + accessKey="d" onClick={() => { triggerGoal("Doctor Connect Clicked", { facilityId: qParams.facility, userId: authUser.id, page: "FacilityPatientsList", }); setShowDoctors(true); }} >
884-905: Simplify export menu implementationThe export menu implementation could be simplified by moving the data transformation logic to a separate utility function.
+const transformExportData = (data: string) => { + const lines = data.split("\n"); + const idsMap = new Map(); + lines.forEach((line, i) => { + const patientId = line.split(",")[0]; + if (idsMap.has(patientId)) { + idsMap.get(patientId).push(i); + } else { + idsMap.set(patientId, [i]); + } + }); + // ... rest of the transformation logic + return cleanedData; +}; <ExportMenu disabled={!isExportAllowed} exportItems={[ { label: "Export Live patients", action: async () => { const query = { ...params, csv: true, facility: qParams.facility, }; - delete qParams.is_active; + const { is_active, ...restParams } = qParams; const { data } = await request(routes.patientList, { query, }); return data ?? null; }, - parse: preventDuplicatePatientsDuetoPolicyId, + parse: transformExportData, }, ]} />🧰 Tools
🪛 Biome (1.9.4)
[error] 895-895: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
public/locale/en.json (1)
1221-1221: Inconsistent phone number field labels.There are inconsistencies in how phone number fields are labeled:
- "Primary ph. no." vs "Primary Ph No."
- Different punctuation and capitalization patterns
Consider standardizing the format:
- "phone_number_min_error": "Phone number must be at least 10 characters long", - "primary_phone_no": "Primary ph. no.", + "phone_number_min_error": "Phone number must be at least 10 digits", + "primary_phone_no": "Primary Phone Number"Also applies to: 1277-1277
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (24)
package.json(2 hunks)public/locale/en.json(20 hunks)src/CAREUI/misc/SectionNavigator.tsx(1 hunks)src/Routers/routes/PatientRoutes.tsx(2 hunks)src/Utils/utils.ts(1 hunks)src/components/Common/Breadcrumbs.tsx(1 hunks)src/components/Common/SearchByMultipleFields.tsx(2 hunks)src/components/Facility/FacilityList.tsx(0 hunks)src/components/Patient/ManagePatients.tsx(3 hunks)src/components/Patient/PatientFilter.tsx(4 hunks)src/components/Patient/PatientIndex.tsx(1 hunks)src/components/Patient/PatientRegistration.tsx(1 hunks)src/components/Patient/Utils.ts(1 hunks)src/components/Patient/models.tsx(0 hunks)src/components/ui/checkbox.tsx(1 hunks)src/components/ui/errors.tsx(1 hunks)src/components/ui/input.tsx(1 hunks)src/components/ui/radio-group.tsx(1 hunks)src/components/ui/select.tsx(2 hunks)src/components/ui/tabs.tsx(1 hunks)src/components/ui/textarea.tsx(1 hunks)src/hooks/useFilters.tsx(1 hunks)src/pages/Facility/FacilitiesPage.tsx(0 hunks)src/pages/Facility/FacilityDetailsPage.tsx(1 hunks)
💤 Files with no reviewable changes (3)
- src/components/Patient/models.tsx
- src/pages/Facility/FacilitiesPage.tsx
- src/components/Facility/FacilityList.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Patient/PatientFilter.tsx
[error] 1044-1044: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
src/CAREUI/misc/SectionNavigator.tsx
[error] 61-72: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/components/Patient/PatientRegistration.tsx
[error] 533-536: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 537-537: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 555-555: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 571-571: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 749-749: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 780-780: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 815-815: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 848-848: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 873-873: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 911-911: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 929-929: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 953-953: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 954-954: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 977-977: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 978-978: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/components/Patient/ManagePatients.tsx
[error] 895-895: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (19)
src/pages/Facility/FacilityDetailsPage.tsx (2)
20-20: Verify the FeatureBadge component implementation
The import looks correct, but let's ensure the component is properly implemented since it's being used for facility features display.
✅ Verification successful
Import and implementation of FeatureBadge is correct
The FeatureBadge component is properly implemented in Utils.tsx and correctly used in FacilityDetailsPage.tsx. It:
- Has proper TypeScript typing with
featureIdprop - Uses the FACILITY_FEATURE_TYPES constant from common constants
- Implements proper UI rendering with Badge component and styling
- Is correctly imported and used with required props in FacilityDetailsPage
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the implementation of FeatureBadge component
ast-grep --pattern 'export const FeatureBadge = $_' src/pages/Facility/Utils.ts*
Length of output: 1838
Script:
#!/bin/bash
# Check if FeatureBadge is used in FacilityDetailsPage
ast-grep --pattern 'FeatureBadge' src/pages/Facility/FacilityDetailsPage.tsx
# Check for FACILITY_FEATURE_TYPES constant
ast-grep --pattern 'const FACILITY_FEATURE_TYPES = $_' src/pages/Facility/Utils.tsx
Length of output: 376
Script:
#!/bin/bash
# Let's check the FACILITY_FEATURE_TYPES with a broader pattern to catch possible variations
ast-grep --pattern 'FACILITY_FEATURE_TYPES' src/pages/Facility/Utils.tsx
# Also check for any type definitions related to facility features
ast-grep --pattern 'type $_FacilityFeature' src/pages/Facility/Utils.tsx
Length of output: 345
Line range hint 1-150: Verify integration with new Patient Search/Register UI
This facility details page seems tangential to the main PR objective of implementing a new patient search and registration UI. We should verify how this component integrates with the new patient flow.
src/components/Patient/PatientIndex.tsx (1)
100-122: Recommend clarifying input sanitation for dynamic query parameters.
The logic for searching phone numbers, name, and patient_no is robust at a glance. However, we recommend sanitizing input to handle potential edge cases at the boundaries (like partial or special-character inputs).
Would you like a deeper codebase scan to check whether these fields are validated or sanitized in downstream services?
src/components/Patient/PatientRegistration.tsx (1)
755-882: **** Switch to absolute imports.
A prior review comment had suggestions to use absolute imports consistently. This code segment mostly uses absolute imports (like "@/hooks/..."), but ensure consistency across all new lines.
🧰 Tools
🪛 Biome (1.9.4)
[error] 780-780: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 815-815: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 848-848: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 873-873: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/components/ui/errors.tsx (1)
1-11: Good approach to error rendering.
Using the InputErrors component is a clean way to handle validations without cluttering individual inputs. This approach is easily composable for various form fields.
src/components/Patient/Utils.ts (1)
21-37:
Verify URL generation logic for edge cases
The URL generation logic needs additional validation to prevent potential issues:
- The function assumes
patient.idexists but doesn't validate it - There's no validation for
patient.facilitybefore using it in the URL - The
last_consultationcheck could be simplified
This might be related to the reported issue where "Patient creation with 20-digit number redirects correctly but shows 'undefined' on patient details page".
export const getPatientUrl = (patient: PatientModel) => {
+ if (!patient?.id) {
+ throw new Error("Patient ID is required");
+ }
let patientUrl = "";
if (!isPatientMandatoryDataFilled(patient)) {
- patientUrl = `/facility/${patient.facility}/patient/${patient.id}`;
+ patientUrl = patient.facility
+ ? `/facility/${patient.facility}/patient/${patient.id}`
+ : `/patient/${patient.id}`;
} else if (
patient.last_consultation &&
patient.last_consultation?.facility === patient.facility &&
!(patient.last_consultation?.discharge_date && patient.is_active)
) {
patientUrl = `/facility/${patient.facility}/patient/${patient.id}/consultation/${patient.last_consultation.id}`;
} else if (patient.facility) {
patientUrl = `/facility/${patient.facility}/patient/${patient.id}`;
} else {
patientUrl = `/patient/${patient.id}`;
}
return patientUrl;
};Let's verify the usage:
src/components/ui/input.tsx (2)
9-13: LGTM! Well-structured props interface.
The InputProps interface properly extends React's input props while adding type-safe label and errors properties.
28-30: LGTM! Clear error state handling.
The error state styling is well-implemented with clear visual feedback through border color changes.
src/components/ui/radio-group.tsx (1)
11-15: LGTM! Consistent props interface.
The props interface follows the same pattern as other form components, maintaining consistency across the UI library.
src/Routers/routes/PatientRoutes.tsx (1)
20-22: Verify route parameter changes.
The switch from PatientRegister to PatientRegistration includes parameter changes (id → patientId). Let's verify this change is consistent across the codebase.
Also applies to: 40-40
✅ Verification successful
Let me gather more specific information about the parameter usage.
Let me gather more focused information about the parameter usage.
Let me try a different search pattern to get the component definition and usage.
Based on the evidence gathered, I can now provide the final response:
Parameter changes are correctly implemented.
The codebase shows consistent usage of parameters:
- The
PatientRegistrationcomponent acceptsfacilityIdand optionalpatientIdas defined in its interface - All usages in routes follow this pattern:
- New patient route:
facilityIdonly - Update route: both
facilityIdandpatientId(mapped from route paramid)
- New patient route:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to PatientRegister and old parameter patterns
rg -l 'PatientRegister|patientId.*PatientRegistration'
# Search for potential parameter mismatches in component usage
ast-grep --pattern 'PatientRegistration facilityId={$_} patientId={$_}'
Length of output: 413
Script:
#!/bin/bash
# Check the actual props interface and usage in PatientRegistration
rg -A 5 "interface.*PatientRegistration" src/components/Patient/PatientRegistration.tsx
# Look for actual usage patterns of PatientRegistration
rg -B 2 -A 2 "<PatientRegistration"
# Check if there are any remaining PatientRegister component definitions
ast-grep --pattern 'const PatientRegister = $_'
Length of output: 67855
Script:
#!/bin/bash
# Check the actual component props and usage in PatientRegistration
rg -B 2 -A 2 "PatientRegistration.*=.*{" src/components/Patient/PatientRegistration.tsx
# Look for any references to old parameter names
rg "id.*PatientRegistration|patientId.*PatientRegistration" src/components/Patient/PatientRegistration.tsx src/Routers/routes/PatientRoutes.tsx
Length of output: 349
Script:
#!/bin/bash
# Check the component definition and interface
rg "interface.*PatientRegistration|export.*function.*PatientRegistration" src/components/Patient/PatientRegistration.tsx
# Look for actual usage of PatientRegistration component with parameters
rg -B 1 -A 1 "<PatientRegistration.*?>" -U --multiline
Length of output: 757
src/components/Common/Breadcrumbs.tsx (1)
112-112: LGTM: Padding adjustment improves visual alignment
The padding reduction from p-1 to p-0 helps align the Home link with other navigation elements.
src/components/ui/select.tsx (2)
11-12: LGTM: Good separation of concerns
The imports for error and label components are appropriately separated, following the component-based architecture.
22-26: LGTM: Well-typed props interface
The new props are properly typed and optional, following TypeScript best practices.
package.json (1)
61-61: Verify compatibility with i18next-browser-languagedetector downgrade
While the Radix UI updates are minor version bumps, the downgrade of i18next-browser-languagedetector from 8.0.2 to 8.0.0 requires verification.
Also applies to: 67-67, 96-96
✅ Verification successful
Downgrade of i18next-browser-languagedetector is safe
The codebase uses basic language detection functionality through a simple import, and the downgrade from 8.0.2 to 8.0.0 within the same major version is safe as:
- The usage pattern is straightforward with no complex configurations
- Both versions are compatible with the current i18next v23.16.4
- No breaking changes exist between 8.0.0 and 8.0.2 as they are patch versions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for breaking changes in i18next-browser-languagedetector
# Search for usage patterns that might be affected by the downgrade
rg -l "languageDetector|i18next.*detect"
# Check package.json in node_modules for peer dependency conflicts
cat node_modules/i18next-browser-languagedetector/package.json | jq '.peerDependencies'
Length of output: 317
Script:
#!/bin/bash
# Let's check the actual usage of the language detector in the codebase
rg -A 5 "languageDetector|i18next.*detect" src/i18n.ts
# Also check the current version of i18next being used
cat package.json | jq '.dependencies["i18next"]'
Length of output: 346
src/components/Common/SearchByMultipleFields.tsx (1)
245-245: LGTM: Improved keyboard shortcut display
Using option.shortcutKey directly is more maintainable than deriving it from the label.
src/hooks/useFilters.tsx (1)
212-212: LGTM! UI spacing adjustment
The removal of mt-6 class helps maintain consistent spacing in the filter badges container.
public/locale/en.json (3)
338-339: Clear and informative age input warning messages.
The warning messages effectively communicate:
- That age input is optional
- Only year of birth will be captured
- When age input is recommended (when DOB is unknown)
835-835: Well-structured section descriptions for patient registration form.
The descriptions for each section (general info, insurance details, social profile, etc.) provide clear guidance to users about the information needed and its purpose.
Also applies to: 900-900, 954-954, 1455-1455, 1652-1652
1587-1587: Clear checkbox label texts for address and phone number reuse.
The checkbox labels are clear and unambiguous:
- "Use this address for permanent address"
- "Use this phone number for emergency contact"
Also applies to: 1589-1589
| @@ -64,7 +64,7 @@ | |||
| "DOMESTIC_HEALTHCARE_SUPPORT__PAID_CAREGIVER": "Paid caregiver", | |||
| "GENDER__1": "Male", | |||
| "GENDER__2": "Female", | |||
| "GENDER__3": "Non-binary", | |||
| "GENDER__3": "Transgender", | |||
| <Select | ||
| {...selectProps("local_body")} | ||
| onValueChange={(value) => | ||
| setForm((f) => ({ | ||
| ...f, | ||
| local_body: Number(value), | ||
| ward: undefined, | ||
| })) | ||
| } | ||
| disabled={ | ||
| !form.district || | ||
| localBodyQuery.isLoading || | ||
| !localBodyQuery.data?.length | ||
| } | ||
| > | ||
| <SelectTrigger | ||
| label={t("local_body")} | ||
| required | ||
| errors={errors["local_body"]} | ||
| className="w-full" | ||
| > | ||
| <SelectValue /> | ||
| </SelectTrigger> | ||
| <SelectContent> | ||
| {localBodyQuery.data?.map((localbody) => ( | ||
| <SelectItem value={localbody.id.toString()}> | ||
| {localbody.name} | ||
| </SelectItem> | ||
| ))} | ||
| </SelectContent> | ||
| </Select> |
There was a problem hiding this comment.
All of these should be a search select, its really hard to find something from this large list.
| !!duplicatePatients?.length && | ||
| !!parsePhoneNumber(debouncedNumber || "") && | ||
| !suppressDuplicateWarning && ( | ||
| <DuplicatePatientDialog |
There was a problem hiding this comment.
Since we are updating it, why not change it to use shadcn buttons. would look prittier.
| @@ -4,24 +4,38 @@ import * as React from "react"; | |||
|
|
|||
| import { cn } from "@/lib/utils"; | |||
|
|
|||
| import { Label } from "./label"; | |||
There was a problem hiding this comment.
We should not be updating ShadCN components. If we want a different version that clubs both, we make a component.
It will be painfull to upgrade at a later stage if we keep editing shadcn components.
| @@ -2,18 +2,39 @@ import * as React from "react"; | |||
|
|
|||
| import { cn } from "@/lib/utils"; | |||
|
|
|||
| const Input = React.forwardRef<HTMLInputElement, React.ComponentProps<"input">>( | |||
| ({ className, type, ...props }, ref) => { | |||
| import { Label } from "@/components/ui/label"; | |||
There was a problem hiding this comment.
Lets not extend input, maybe make a InputWithLabel 🤔
| @@ -50,4 +50,50 @@ const TabsContent = React.forwardRef< | |||
| )); | |||
| TabsContent.displayName = TabsPrimitive.Content.displayName; | |||
|
|
|||
| export { Tabs, TabsList, TabsTrigger, TabsContent }; | |||
| interface SectionTabsProps { | |||
There was a problem hiding this comment.
Lets not exapnd Shadcn components
|
Reminder: To add the "needs testing" label, comment "ready for testing" on this PR. |
|
👋 Hi, @shivankacker, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
@shivankacker can you update the status here |
|
@bodhish working on converting Scribe to support microfrontends. I will take this up after it. Or whatever is in more priority 😅 |
|
I would say wrap this 👆, just minor Cc @gigincg |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
SectionNavigatorcomponent for improved navigation within sections.PatientIndexandPatientRegistrationcomponents for enhanced patient management and registration.FacilityListandManagePatientscomponents.Enhancements
Bug Fixes
Chores