-
Notifications
You must be signed in to change notification settings - Fork 540
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
Confirm Alert for Patient Registration #10205
Confirm Alert for Patient Registration #10205
Conversation
WalkthroughThe pull request introduces improvements to patient registration and patient index components across multiple files. Key changes include the addition of a navigation prompt to prevent unintended data loss when navigating away from a dirty form, and modifications to how phone number query parameters are handled. The changes focus on enhancing user experience by providing better form state management and preserving user input across page navigation. Changes
Assessment against linked issues
Possibly related PRs
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 (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/Patient/PatientRegistration.tsx (1)
Line range hint
694-719
: Consider using consistent grid layout.The nationality field is placed in a single-column grid while other form fields maintain a two-column layout. This might affect the visual consistency of the form.
Consider using the same two-column grid layout:
-<div className="grid grid-cols-1 gap-4"> +<div className="grid grid-cols-2 gap-4">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Patient/PatientIndex.tsx
(2 hunks)src/components/Patient/PatientRegistration.tsx
(5 hunks)src/pages/PublicAppointments/PatientRegistration.tsx
(3 hunks)
🔇 Additional comments (6)
src/components/Patient/PatientIndex.tsx (1)
2-2
: LGTM! Phone number auto-fill implementation looks good.The implementation correctly retrieves the phone number from query parameters and initializes the state, addressing the requirement to auto-fill the phone number field upon redirection from duplicate patient flow.
Also applies to: 42-43
src/pages/PublicAppointments/PatientRegistration.tsx (2)
3-3
: LGTM! Navigation prompt implementation looks good.The implementation correctly tracks the form state and patient creation status to show the confirmation alert when needed.
Also applies to: 168-168
214-219
: Verify the timeline for tanstack router migration.The TODO comment suggests switching to tanstack router's
useBlocker
hook. This would provide a more robust solution for navigation blocking.Run the following script to check if there's an existing issue or PR for the router migration:
src/components/Patient/PatientRegistration.tsx (3)
303-308
: LGTM! Improved readability with showDuplicate constant.The implementation correctly consolidates the conditions for displaying the duplicate dialog into a single, readable constant.
309-318
: LGTM! Navigation prompt implementation looks good.The implementation correctly handles the form state, patient creation/update status, and duplicate dialog state to show the confirmation alert when needed.
Line range hint
720-773
: LGTM! Conditional rendering looks good.The implementation correctly:
- Uses the two-column grid layout for geo-organization field
- Conditionally renders the field based on nationality
- Uses the
showDuplicate
constant for duplicate dialog rendering
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/Patient/PatientIndex.tsx (2)
42-43
: Good use of query params for persistent state!Using URL query parameters for the phone number state is a good choice here as it:
- Enables auto-filling the phone number after duplicate patient redirection
- Makes the search state shareable and bookmarkable
- Preserves the state across page refreshes
This addresses the PR objective of fixing the duplicate patient flow.
95-97
: Consider improving phone number validation.The current validation logic has some potential issues:
- The hard-coded length check of 13 characters might not work for all phone number formats
- The validation logic is duplicated (parsePhoneNumber is used later in the query)
Consider using the existing
parsePhoneNumber
utility for consistent validation:setPhoneNumberQuery({ - phone_number: value.length >= 13 || value === "" ? value : "", + phone_number: parsePhoneNumber(value) || value === "" ? value : "", });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Patient/PatientIndex.tsx
(3 hunks)
🔇 Additional comments (1)
src/components/Patient/PatientIndex.tsx (1)
2-2
: LGTM!The addition of
useQueryParams
from 'raviger' aligns with the implementation of query parameter-based state management.
@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
Release Notes
New Features
Improvements
Technical Updates
The changes focus on improving user experience and form interaction in patient-related workflows.