Fix: Add fallback url for goback() | OTP field auto focus | Add Search Feature for users #10146
Conversation
WalkthroughThis pull request improves navigation and user interaction across multiple components. Several cancel and back buttons now pass explicit fallback URLs to handle cases with no browser history. The OTP input fields in login components have been enhanced with an auto-focus attribute for immediate user input. Furthermore, the Facility Organization Users component now incorporates a more robust search feature using a custom hook with debounced API calls and improved loading states. No changes were made to any exported or public entities. Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (4)
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. |
|
@rithviknishad @Jacobjeevan @nihal467 i have implemented username searching for users in facility organization. But backend response is not filtered by username. In backend username filtering is not happening.
In response all users are coming
I made the changes in backend , it worked fined. Shall i raise a PR in backend?? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/pages/FacilityOrganization/FacilityOrganizationUsers.tsx (3)
40-53: Consider adding error handling for the query.The query configuration is well-structured with proper debouncing and pagination. However, error handling should be implemented to improve user experience when API calls fail.
Add error handling:
const { data: users, isLoading: isLoadingUsers } = useQuery({ + onError: (error) => { + // Handle error appropriately (e.g., show toast notification) + console.error('Failed to fetch users:', error); + }, queryKey: [
69-80: Enhance search input accessibility.The search implementation is well-structured with proper test attributes. Consider adding ARIA labels and announcing search results for better accessibility.
Add accessibility enhancements:
<Input type="text" placeholder="Search users..." + aria-label={t("search_users")} + role="searchbox" value={qParams.search || ""} onChange={(e) => updateQuery({ search: e.target.value as string, }) } className="w-full" data-cy="search-user" />
110-117: Consider virtualizing the user list for better performance.The empty state and loading handling are well-implemented. For better performance with large lists, consider implementing virtualization.
Consider using a virtualized list library like
react-windoworreact-virtualizedwhen dealing with large datasets:+import { FixedSizeGrid } from 'react-window'; + -<div className="grid grid-cols-1 md:grid-cols-2 xl:grid-cols-3 gap-4"> +<FixedSizeGrid + columnCount={3} + columnWidth={300} + height={800} + rowCount={Math.ceil(users?.results?.length / 3)} + rowHeight={200} + width={1000} +> {users?.results?.map((userRole: OrganizationUserRole) => (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/components/Auth/Login.tsx(2 hunks)src/components/Form/FormFields/PhoneNumberFormField.tsx(2 hunks)src/components/Patient/EncounterQuestionnaire.tsx(1 hunks)src/components/Patient/PatientRegistration.tsx(1 hunks)src/components/Resource/ResourceCreate.tsx(1 hunks)src/components/Resource/ResourceDetailsUpdate.tsx(1 hunks)src/pages/Appointments/BookAppointment.tsx(1 hunks)src/pages/FacilityOrganization/FacilityOrganizationUsers.tsx(2 hunks)src/pages/Patients/VerifyPatient.tsx(1 hunks)src/pages/PublicAppointments/Schedule.tsx(1 hunks)src/pages/PublicAppointments/auth/PatientLogin.tsx(3 hunks)
🔇 Additional comments (15)
src/components/Patient/EncounterQuestionnaire.tsx (1)
49-51: LGTM! Improved navigation with specific fallback URL.The addition of a specific fallback URL for the cancel action ensures consistent navigation behavior.
src/pages/Appointments/BookAppointment.tsx (1)
163-171: LGTM! Clear navigation path for cancel action.The cancel button now correctly navigates back to the patient's appointments list, maintaining proper context with facility and patient IDs.
src/components/Resource/ResourceDetailsUpdate.tsx (1)
282-286: LGTM! Consistent cancel navigation behavior.The cancel button properly navigates back to the resource details page, maintaining consistency with other cancel actions.
src/pages/Patients/VerifyPatient.tsx (1)
242-242: LGTM! Clear error state navigation.The error state button now correctly navigates back to the facility's patients list, maintaining proper context.
src/pages/PublicAppointments/Schedule.tsx (1)
219-219: LGTM! Navigation path is correctly specified.The back button now navigates to a specific facility page using the
facilityIdprop, providing a more predictable navigation experience.src/components/Resource/ResourceCreate.tsx (1)
442-446: LGTM! Navigation path is correctly specified.The cancel button now navigates to the resource requests page for a specific patient using both
facilityIdandrelated_patientparameters, providing a more contextual navigation experience.src/components/Patient/PatientRegistration.tsx (1)
731-731: LGTM! Navigation path is correctly specified.The cancel button now navigates to the patients list page using the
facilityIdprop, providing a more predictable navigation experience.src/pages/FacilityOrganization/FacilityOrganizationUsers.tsx (2)
30-33: Well-structured filter configuration!Good use of
useFiltershook with appropriate caching strategy. ThecacheBlacklistfor search prevents unwanted persistence of search terms.
190-190: LGTM! Pagination implementation is clean and effective.The pagination is well-integrated with the API response and filter hooks.
src/pages/PublicAppointments/auth/PatientLogin.tsx (3)
179-179: LGTM! Good UX improvement.Adding autoFocus to the phone number input field enhances user experience by allowing immediate input without requiring an extra click.
224-229: LGTM! Smooth transition to OTP input.The autoFocus attribute on the InputOTP component creates a seamless flow from phone number entry to OTP verification.
283-283: LGTM! Improved navigation reliability.Using a specific fallback URL (
/facility/${facilityId}) instead of relying on browser history provides more predictable navigation behavior.src/components/Form/FormFields/PhoneNumberFormField.tsx (2)
40-40: LGTM! Well-typed prop addition.The autoFocus property is correctly added to the Props interface with an optional boolean type.
155-155: LGTM! Proper prop implementation.The autoFocus prop is correctly passed to the underlying input element.
src/components/Auth/Login.tsx (1)
649-649: LGTM! Enhanced initial focus.Adding autoFocus to the phone input improves the login flow by immediately allowing phone number entry.
Jacobjeevan
added
waiting for backend
and removed
question
|
👋 Hi, @Rishith25, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/pages/FacilityOrganization/FacilityOrganizationUsers.tsx (2)
67-81: Add accessibility attributes to the search input.While the search implementation looks good, consider adding these accessibility improvements:
<Input type="text" placeholder="Search users..." value={qParams.search || ""} onChange={(e) => updateQuery({ search: e.target.value as string, }) } className="w-full" data-cy="search-user" + aria-label={t("search_users")} + role="searchbox" />
105-187: Enhance semantic HTML structure in user cards.The user list implementation looks good, but consider these semantic HTML improvements:
-<h1 className="text-base font-bold break-words pr-2"> +<h3 className="text-base font-bold break-words pr-2"> {userRole.user.first_name}{" "} {userRole.user.last_name} -</h1> +</h3>Rationale:
- Use
h3instead ofh1as these are sub-headings within the pageh1should be used for the main page heading only
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/Auth/Login.tsx(1 hunks)src/components/Patient/PatientRegistration.tsx(1 hunks)src/components/Resource/ResourceCreate.tsx(1 hunks)src/pages/Appointments/BookAppointment.tsx(1 hunks)src/pages/FacilityOrganization/FacilityOrganizationUsers.tsx(2 hunks)src/pages/PublicAppointments/Schedule.tsx(1 hunks)src/pages/PublicAppointments/auth/PatientLogin.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- src/components/Patient/PatientRegistration.tsx
- src/pages/PublicAppointments/auth/PatientLogin.tsx
- src/components/Auth/Login.tsx
- src/pages/Appointments/BookAppointment.tsx
- src/components/Resource/ResourceCreate.tsx
- src/pages/PublicAppointments/Schedule.tsx
🔇 Additional comments (3)
src/pages/FacilityOrganization/FacilityOrganizationUsers.tsx (3)
8-8: LGTM! Good setup of the useFilters hook.The hook configuration with
cacheBlacklist: ["search"]is a good practice to prevent persisting search terms across sessions.Also applies to: 11-11, 14-14, 30-33
190-190: LGTM! Clean pagination implementation.The pagination setup with total count from the API response is clean and follows good practices.
40-53: Verify the offset calculation for edge cases.The offset calculation looks good, but let's ensure it handles all edge cases correctly:
- When
qParams.pageis undefined (first page)- When
qParams.pageis 0 or negative✅ Verification successful
Offset calculation is implemented correctly ✅
The current implementation
((qParams.page ?? 1) - 1) * resultsPerPageproperly handles all edge cases:
- Defaults to page 1 when
qParams.pageis undefined- Cannot produce negative offsets due to the nullish coalescing operator
- Correctly converts from 1-based page numbers to 0-based offset calculation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other pagination implementations in the codebase rg -A 2 "offset.*page.*resultsPerPage" --type typescriptLength of output: 93
Script:
#!/bin/bash # Search for pagination-related patterns rg -t ts -t tsx -A 3 "offset.*=.*page" # Search for page parameter usage in query configurations rg -t ts -t tsx -A 5 "queryParams.*page" # Search specifically for resultsPerPage usage rg -t ts -t tsx -A 2 "resultsPerPage"Length of output: 205
|
👋 Hi, @Rishith25, |
|
@Rishith25 what is the status on this PR |
@nihal467 I have to write test case for this functionality in the backend. |
|
@Rishith25 fix the deploy failure and the cypress tests, lets get the frontend merged, you can do the username filter asynchronously. |
khavinshankar
added
changes required
and removed
needs-triage
question
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
@khavinshankar Resolved the merge conflicts and deploy is now successful |
Jacobjeevan
removed
changes required
Deploy-Failed
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit