Replaced all TextAreaForm component with Textarea shadcn ui component #10277
Conversation
WalkthroughThis pull request focuses on modernizing UI components by replacing the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/components/ui/date-field.tsx (1)
44-63: Consider adding debounce to onChange events.The current implementation triggers the
onChangecallback on every valid input. Consider debouncing these events to reduce unnecessary re-renders, especially when integrated with form libraries.+import { debounce } from "lodash"; + export default function DateField({ date, onChange, disabled, id = "date-field", }: DateFieldProps) { const { t } = useTranslation(); + const debouncedOnChange = useMemo( + () => debounce((date?: Date) => onChange?.(date), 300), + [onChange] + ); + + useEffect(() => { + return () => { + debouncedOnChange.cancel(); + }; + }, [debouncedOnChange]); // ... rest of the component code const handleDayChange = (e: React.ChangeEvent<HTMLInputElement>) => { // ... validation logic - onChange?.(updatedDate); + debouncedOnChange(updatedDate); };Also applies to: 65-85, 87-101
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
public/locale/en.json(4 hunks)src/components/Form/FormFields/DateFormField.tsx(0 hunks)src/components/Patient/PatientRegistration.tsx(2 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(1 hunks)src/components/Resource/ResourceCommentSection.tsx(2 hunks)src/components/Resource/ResourceCreate.tsx(2 hunks)src/components/Resource/ResourceDetailsUpdate.tsx(6 hunks)src/components/ui/autocomplete.tsx(1 hunks)src/components/ui/date-field.tsx(1 hunks)src/components/ui/date-picker.tsx(2 hunks)src/hooks/useFileManager.tsx(2 hunks)src/pages/PublicAppointments/PatientRegistration.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- src/components/Form/FormFields/DateFormField.tsx
🧰 Additional context used
📓 Learnings (1)
src/components/Patient/PatientRegistration.tsx (1)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#10260
File: src/components/Patient/PatientRegistration.tsx:286-289
Timestamp: 2025-01-28T15:50:07.442Z
Learning: For patient registration in the frontend, either year_of_birth or date_of_birth is required for successful registration. If date_of_birth is not available, year_of_birth must be present.
🔇 Additional comments (16)
src/components/ui/date-picker.tsx (1)
19-19: LGTM! The disabled prop is well-implemented.The addition of the
disabledprop enhances the component's flexibility by allowing dynamic date disabling based on custom logic. The prop is correctly typed and properly passed through to the underlyingCalendarcomponent.Also applies to: 22-22, 48-48
src/components/ui/date-field.tsx (3)
9-14: LGTM! Props interface is well-defined.The
DateFieldPropsinterface is clean and includes all necessary props with proper typing.
16-19: LGTM! Date validation is robust.The
isValidDatefunction uses dayjs for reliable date validation, ensuring that only valid dates are accepted.
103-151: LGTM! Accessibility and UX are well-implemented.The component demonstrates good practices:
- Uses semantic HTML with proper labels
- Includes data-cy attributes for testing
- Provides clear placeholders
- Handles disabled state consistently
- Uses appropriate input types and constraints
src/pages/PublicAppointments/PatientRegistration.tsx (1)
321-328: LGTM! DateField integration is clean and correct.The DateField component is properly integrated with:
- Correct date conversion and ISO string formatting
- Proper id attribute for accessibility
- Clean onChange handler implementation
src/components/Patient/PatientRegistration.tsx (1)
517-525: LGTM! DateField integration aligns with requirements.The DateField component is properly integrated and aligns with the requirement that either year_of_birth or date_of_birth must be present for successful registration.
Note: Based on the retrieved learning from PR #10260, the implementation correctly handles the date_of_birth requirement.
src/components/ui/autocomplete.tsx (1)
21-21: LGTM! Good practice to export the interface.The change to export the
AutoCompleteOptioninterface improves code reusability and type consistency across components.src/components/Resource/ResourceCommentSection.tsx (2)
45-49: LGTM! Clean migration to shadcn/ui Textarea component.The component replacement and event handler update follow the correct patterns.
52-52: LGTM! Good internationalization practices.The changes properly implement i18n translations for user-facing strings and maintain consistent spacing with
mt-2.Also applies to: 60-60, 66-66
src/components/Resource/ResourceDetailsUpdate.tsx (3)
229-246: LGTM! Clean migration to shadcn/ui Select component.The status selection implementation is correct and maintains proper type safety.
267-276: LGTM! Good integration of Autocomplete component.The facility assignment implementation correctly uses the fetched data.
321-342: LGTM! Clean implementation of emergency radio group.The radio group implementation with error handling is correct.
src/components/Resource/ResourceCreate.tsx (1)
33-33: LGTM! Clean migration to shadcn Textarea component.The onChange handler has been correctly updated to use
e.target.valueinstead of the previous value object pattern, maintaining proper form state management.Also applies to: 353-357
src/hooks/useFileManager.tsx (2)
4-4: LGTM! Well-structured imports for the new implementation.The addition of Trans, cn utility, and shadcn UI components provides all necessary tools for localization and conditional styling.
Also applies to: 7-8, 12-13
297-319: LGTM! Enhanced form field implementation.The new implementation offers several improvements:
- Better accessibility with explicit Label component
- Proper error state handling with conditional styling
- Enhanced internationalization support using Trans component
- Clear error message display
public/locale/en.json (1)
1022-1022: LGTM! Well-structured localization entries.The new entries are properly formatted and provide clear, contextual translations. The HTML tags in the archive reason message are correctly escaped for proper rendering.
Also applies to: 1398-1398, 1692-1692, 2017-2017
| <Label className="text-gray-700 mb-3 mt-1"> | ||
| {t("request_title")} | ||
| </Label> | ||
| <Input | ||
| name="title" | ||
| type="text" | ||
| label="Request Title*" | ||
| placeholder="Type your title here" | ||
| value={state.form.title} | ||
| onChange={handleChange} | ||
| error={state.errors.title} | ||
| onChange={(e) => | ||
| handleChange({ name: e.target.name, value: e.target.value }) | ||
| } | ||
| /> | ||
| {state.errors.title && ( | ||
| <p className="text-red-500 text-sm mt-2"> | ||
| {state.errors.emergency} | ||
| </p> | ||
| )} |
There was a problem hiding this comment.
Fix error message display.
The error messages are displaying the wrong field's error:
Apply this diff to fix the error messages:
- {state.errors.emergency}
+ {state.errors.title}- {state.errors.emergency}
+ {state.errors.reason}Also applies to: 301-317
| const [data, setData] = useState<AutoCompleteOption[]>([]); | ||
| const query = { | ||
| limit: 50, | ||
| offset: 0, | ||
| search_text: "", | ||
| all: true, | ||
| facility_type: 1510, | ||
| exclude_user: "", | ||
| }; | ||
| useEffect(() => { | ||
| const retriveData = async () => { | ||
| const { data } = await request(facilityApi.getAllFacilities, { query }); | ||
| const facilities: AutoCompleteOption[] = []; | ||
| data?.results!.map((facility: FacilityData) => { | ||
| facilities.push({ | ||
| label: facility.name, | ||
| value: facility.id, | ||
| }); | ||
| }); | ||
| setData(facilities); | ||
| }; | ||
|
|
||
| retriveData(); | ||
| }, [data]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider optimizing the facility data fetching logic.
The current implementation has potential issues:
- The
useEffectdependency ondatacould cause an infinite loop - The facility type is hardcoded to
1510
Apply this diff to fix the issues:
- useEffect(() => {
+ useEffect(() => {
const retriveData = async () => {
const { data } = await request(facilityApi.getAllFacilities, { query });
const facilities: AutoCompleteOption[] = [];
data?.results!.map((facility: FacilityData) => {
facilities.push({
label: facility.name,
value: facility.id,
});
});
setData(facilities);
};
retriveData();
- }, [data]);
+ }, []);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [data, setData] = useState<AutoCompleteOption[]>([]); | |
| const query = { | |
| limit: 50, | |
| offset: 0, | |
| search_text: "", | |
| all: true, | |
| facility_type: 1510, | |
| exclude_user: "", | |
| }; | |
| useEffect(() => { | |
| const retriveData = async () => { | |
| const { data } = await request(facilityApi.getAllFacilities, { query }); | |
| const facilities: AutoCompleteOption[] = []; | |
| data?.results!.map((facility: FacilityData) => { | |
| facilities.push({ | |
| label: facility.name, | |
| value: facility.id, | |
| }); | |
| }); | |
| setData(facilities); | |
| }; | |
| retriveData(); | |
| }, [data]); | |
| const [data, setData] = useState<AutoCompleteOption[]>([]); | |
| const query = { | |
| limit: 50, | |
| offset: 0, | |
| search_text: "", | |
| all: true, | |
| facility_type: 1510, | |
| exclude_user: "", | |
| }; | |
| useEffect(() => { | |
| const retriveData = async () => { | |
| const { data } = await request(facilityApi.getAllFacilities, { query }); | |
| const facilities: AutoCompleteOption[] = []; | |
| data?.results!.map((facility: FacilityData) => { | |
| facilities.push({ | |
| label: facility.name, | |
| value: facility.id, | |
| }); | |
| }); | |
| setData(facilities); | |
| }; | |
| retriveData(); | |
| }, []); |
| @@ -157,6 +157,7 @@ export function QuestionnaireForm({ | |||
| }; | |||
|
|
|||
| const handleSubmissionError = (results: ValidationErrorResponse[]) => { | |||
| toast.error("Form Errr"); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant error toast with typo.
This toast message is redundant as there's already a "questionnaire_submission_failed" error toast in the mutation's onError handler. Additionally, it contains a typo ("Errr").
Apply this diff to remove the redundant toast:
- toast.error("Form Errr");982787b to
77c3b4f
Compare
| @@ -1,3 +1,4 @@ | |||
| import { Textarea } from "@headlessui/react"; | |||
There was a problem hiding this comment.
ig this was accidental, import form ui instead.
| <Label className="text-gray-700 mb-3 mt-1"> | ||
| {t("request_reason")} | ||
| </Label> | ||
| <Textarea | ||
| rows={5} | ||
| name="reason" | ||
| placeholder="Type your description here" |
|
@NikhilA8606 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! 🙌 |
Replaced all TextAreaForm component with Textarea shadcn ui component
@rithviknishad
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
Improvements
Localization
Refactor