Deleted errors.tsx and input-with-error.tsx, and updated PatientRegis… #9737
Conversation
…tration.tsx, FacilityOrganizationSelector.tsx, and OrganizationSelector.tsx to replace InputWithError and InputError with Shadcn components accordingly.
WalkthroughThis pull request focuses on removing the custom 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. |
![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
🧹 Nitpick comments (12)
src/pages/FacilityOrganization/components/FacilityOrganizationSelector.tsx (2)
108-114: Localized translation mapping.
Thetfunction for mapping translations is straightforward. If the application grows, consider leveraging a centralized i18n solution to keep translations consistent and track them more easily.
197-199: Follow through on error handling.
The comment suggests future error handling. Confirm if or when error messages will be displayed here to make sure end users are informed of invalid states.Do you want help incorporating a standardized error display mechanism for these fields?
src/components/Patient/PatientRegistration.tsx (9)
317-338: Optional chaining to handle potential undefined errors.
According to the static analysis, you can replace repeated usage likeerrors.name && errors.name.map(...)with optional chaining:errors.name?.map(...). This avoids lengthier condition checks.- {errors.name && - errors.name.map((error, index) => ( - <span key={index} className="text-sm text-red-500 mt-1"> - {error} - </span> - )) - } + {errors.name?.map((error, index) => ( + <span key={index} className="text-sm text-red-500 mt-1"> + {error} + </span> + ))}🧰 Tools
🪛 Biome (1.9.4)
[error] 332-337: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
364-370: Apply optional chaining in phone number errors.
Similar to the recommendation above, using optional chaining can streamline error handling code forerrors.phone_number.🧰 Tools
🪛 Biome (1.9.4)
[error] 364-369: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Line range hint
373-387: Maintain consistent naming and spacing for the checkbox state.
The logic is correct, but ensure variable naming or comments clarify the rationale for tying the emergency phone number to the main phone number.🧰 Tools
🪛 Biome (1.9.4)
[error] 364-369: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Line range hint
395-428: Optional chaining forerrors.emergency_phone_number.
As recommended by static analysis, apply optional chaining to simplify this pattern.
Line range hint
482-509: Use optional chaining for blood group errors.
Consolidate the checks by applying optional chaining.
Line range hint
724-758: Optional chaining for address errors.
As above, replacing explicit checks likeerrors["address"] && errors["address"].map(...)witherrors.address?.map(...)streamlines error rendering.
Line range hint
761-794: Consider central logic for same address default.
Since the address duplication logic is repeated, extracting it into a reusable helper or custom hook might simplify the code.🧰 Tools
🪛 Biome (1.9.4)
[error] 786-791: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
797-826: Optional chaining for permanent address errors.
All these conditional checks can use optional chaining.
834-858: Use optional chaining for pincode errors.
As before, useerrors.pincode?.map(...).src/pages/Organization/components/OrganizationSelector.tsx (1)
Line range hint
116-141: ReplacingInputWithErrorwithLabeland direct error handling.
This refactor is consistent with the rest of the PR’s changes (removal of InputWithError). If more complex validations are required later, consider a dedicated form library or reuse your existing pattern inPatientRegistration.tsx.
📜 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 (5)
src/components/Patient/PatientRegistration.tsx(15 hunks)src/components/ui/errors.tsx(0 hunks)src/components/ui/input-with-error.tsx(0 hunks)src/pages/FacilityOrganization/components/FacilityOrganizationSelector.tsx(4 hunks)src/pages/Organization/components/OrganizationSelector.tsx(5 hunks)
💤 Files with no reviewable changes (2)
- src/components/ui/errors.tsx
- src/components/ui/input-with-error.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Patient/PatientRegistration.tsx
[error] 332-337: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 364-369: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 786-791: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (8)
src/pages/FacilityOrganization/components/FacilityOrganizationSelector.tsx (3)
9-9: Good addition of Shadcn's Label.
This promotes consistency across all form fields by consistently using the Label component from the shared UI library.
34-34: Renamingrequiredto_required.
Renaming the prop helps avoid potential naming conflicts, but ensure that_requiredis not needed for conditional checks or validations in this component.
116-119: Proper labeling for organization selection.
Using the<Label>component improves accessibility, as screen readers can associate the label with the corresponding form field.src/components/Patient/PatientRegistration.tsx (2)
529-569: Improve date-of-birth calculations.
Consider verifying numeric boundaries for days (1–31) and months (1–12). For example, a user might enter “00” or “32” for days. Although you have boundaries in place, ensure there's a fallback or validation.
Line range hint
874-902: Autocomplete usage for nationality.
Implementation looks good. Always ensure the country list is complete or updated as needed.src/pages/Organization/components/OrganizationSelector.tsx (3)
8-8: Label import consistency.
Good move to align this import with the approach used elsewhere.
31-31: Renaming prop to_required.
Make sure to validate whether_requiredmight be needed for future usage or form library integration.
Line range hint
149-175: Error display placeholder.
Right now, the error UI is hardcoded tofalse. If you plan to display errors here eventually, ensure the correct condition is set.
|
👋 Hi, @sanjayh-2022, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
most of the diff doesn't make sense. do read the docs and readme and other examples in existing codebase and follow the merge checklist |
| @@ -31,7 +31,7 @@ interface AutoCompleteOption { | |||
| export default function FacilityOrganizationSelector( | |||
| props: FacilityOrganizationSelectorProps, | |||
| ) { | |||
| const { onChange, required, facilityId } = props; | |||
| const { onChange, required: _required, facilityId } = props; | |||
| function t(key: string): string { | ||
| const translations: { [key: string]: string } = { | ||
| Organization: "Organization", | ||
| }; | ||
| return translations[key] || key; | ||
| } | ||
|
|
| <Label htmlFor="organization" className=""> | ||
| {t("Organization")} | ||
| </Label> |
There was a problem hiding this comment.
i don't see where required is handled here
| @@ -185,6 +194,8 @@ export default function FacilityOrganizationSelector( | |||
| </div> | |||
| )} | |||
| </div> | |||
| </InputWithError> | |||
|
|
|||
| {/* Handle errors if needed */} | |||
There was a problem hiding this comment.
and where is it being handled?
| @@ -28,7 +28,7 @@ interface AutoCompleteOption { | |||
| } | |||
|
|
|||
| export default function OrganizationSelector(props: OrganizationSelectorProps) { | |||
| const { onChange, required } = props; | |||
| const { onChange, required: _required } = props; | |||
|
|
||
| {false && ( | ||
| <span className="text-sm text-red-500 mt-1"> | ||
| {"This field is required"} |
There was a problem hiding this comment.
i18n, follow translations guidelines in readme/docs
| @@ -135,18 +132,28 @@ export default function OrganizationSelector(props: OrganizationSelectorProps) { | |||
| <CareIcon icon="l-trash" className="h-4 w-4" /> | |||
| </Button> | |||
| </div> | |||
| </InputWithError> | |||
|
|
|||
| {false && ( | |||
There was a problem hiding this comment.
false &&? doesn't make sense
| > | ||
| <div className="mb-4"> | ||
| <Label | ||
| htmlFor={`organization-level-${selectedLevels.length}`} |
There was a problem hiding this comment.
where is the equivalent id being passed for the input?
…tration.tsx, FacilityOrganizationSelector.tsx, and OrganizationSelector.tsx to replace InputWithError and InputError with Shadcn components accordingly.
Proposed Changes
InputWithErrorandInputErrorscomponent in favour of shadcn's components being used directly #9702errors.tsxandinput-with-error.tsxPatientRegistration.tsx,FacilityOrganizationSelector.tsx, andOrganizationSelector.tsxwith shadcn components@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Refactor
InputWithErrorcomponentBug Fixes