-
Notifications
You must be signed in to change notification settings - Fork 646
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
Replaced React Date & Time Picker with the device's default date and Time picker #11126
base: develop
Are you sure you want to change the base?
Replaced React Date & Time Picker with the device's default date and Time picker #11126
Conversation
WalkthroughThis pull request introduces a new localization entry for date validation and refactors multiple date-related components. The changes simplify date handling by switching from complex popover calendar interfaces to native input fields and updates the date conversion logic. Both questionnaire components and UI components now use string values for dates, with helper functions like Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant DT as DateTimePicker
participant P as Parent Component
U->>DT: Enter date/time via input (datetime-local)
DT->>DT: Update internal state as a string
DT->>P: Invoke onChange with new date/time string
sequenceDiagram
participant U as User
participant DR as DateRangePicker
participant T as Toast (sonner)
participant P as Parent Component
U->>DR: Input "from" and "to" dates
DR->>DR: Validate that end date is not before start date
alt Valid Range
DR->>P: Call onChange with formatted date strings
else Invalid Range
DR->>T: Display an error toast
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (3)
src/components/ui/date-range-picker.tsx (1)
21-22
: State initialization looks good, but consider effects for prop changes.The state is properly initialized with the provided date values, but there's no effect to update the state when the
date
prop changes externally.Consider adding an effect to sync the state with prop changes:
+ import { useState, useEffect } from "react"; // Inside component const [startDate, setStartDate] = useState(date?.from || ""); const [endDate, setEndDate] = useState(date?.to || ""); + useEffect(() => { + setStartDate(date?.from || ""); + setEndDate(date?.to || ""); + }, [date?.from, date?.to]);src/components/ui/date-time-picker.tsx (2)
22-22
: State initialization needs synchronization with prop changes.Similar to the DateRangePicker, there's no mechanism to update state when the value prop changes externally.
Consider adding an effect to sync the state with prop changes:
+ import { useState, useEffect } from "react"; // Inside component const [dateTime, setDateTime] = useState(value || ""); + useEffect(() => { + setDateTime(value || ""); + }, [value]);
30-32
: Add error handling for date formatting.The code assumes dateTime will always be a valid date string, but doesn't handle invalid formats.
Consider adding error handling:
const formattedDateTime = dateTime - ? format(new Date(dateTime), "yyyy-MM-dd'T'HH:mm") + ? (() => { + try { + const date = new Date(dateTime); + return !isNaN(date.getTime()) + ? format(date, "yyyy-MM-dd'T'HH:mm") + : ""; + } catch (e) { + return ""; + } + })() : "";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
public/locale/en.json
(1 hunks)src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx
(1 hunks)src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx
(2 hunks)src/components/ui/date-picker.tsx
(1 hunks)src/components/ui/date-range-picker.tsx
(2 hunks)src/components/ui/date-time-picker.tsx
(1 hunks)src/pages/Appointments/AppointmentsPage.tsx
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (22)
public/locale/en.json (1)
1033-1033
: New Translation Entry AddedThe new key
"end_date_must_be_after_start_date"
along with its value"End Date must be after Start Date"
is clear and consistent with the existing naming conventions and message style in the file. This entry should effectively provide the user feedback when an invalid date range is selected. Please ensure that similar entries are also updated in other locale files if applicable.src/pages/Appointments/AppointmentsPage.tsx (2)
554-554
: Added key prop to improve component updatesAdding a key based on the date values will force the DateRangePicker to re-render when the dates change, which helps resolve potential stale UI issues when navigating between different date ranges.
557-562
: Correct date formatting for the device's date pickerThe date formatting has been updated to use the
dateQueryString
function, which ensures proper string format compatibility with the new native date input implementation.src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx (3)
45-45
: Added import for dateQueryString utilityThis import is necessary to support the new date handling approach.
489-496
: Using key prop and consistent date formattingAdding a key based on the effective period ensures the component refreshes properly when the date values change. The implementation correctly uses
dateQueryString
to convert dates to the proper string format needed by the device's date picker.
501-503
: Updated date handling to use string valuesChanged from using
toISOString()
totoString()
to align with the new native date picker approach. This maintains date information in a format compatible with the revised UI components.src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (1)
822-826
: Simplified date handling for DateTimePickerThe code now directly passes the string value to the DateTimePicker and uses
toString()
instead oftoISOString()
when updating the authored date. This change aligns with the shift to using the device's native date and time picker.src/components/ui/date-picker.tsx (4)
4-4
: Simplified imports for the native date picker implementationRemoved complex calendar and popover dependencies in favor of a simple Input component, which is more lightweight and uses the device's native date picker.
9-9
: Simplified disabled prop typeChanged the disabled prop from a function to a boolean, which is more appropriate for the native date input element and simplifies the component API.
13-22
: Reimplemented date handling for native date inputThe date state management has been completely reworked to:
- Store the selected date as a formatted string in "yyyy-MM-dd" format
- Handle input changes directly from the native date picker
- Convert string values to Date objects when needed for the onChange callback
This approach is more straightforward and leverages the browser's built-in date validation.
25-30
: Replaced custom calendar with native date inputThe component now uses a simple native date input instead of a complex popover calendar implementation. This:
- Improves accessibility
- Provides better mobile support
- Uses familiar date picking UI for users based on their device
- Reduces bundle size and complexity
This is a significant improvement that aligns with the PR objectives.
src/components/ui/date-range-picker.tsx (5)
2-3
: Good additions of required dependencies.Added useState for local state management and toast for error notifications, which are necessary for the new implementation.
7-8
: Appropriate UI component imports.The Input and Label components are correctly imported for the new native input-based implementation.
10-14
: Props type simplification is good.Simplifying from a complex DateRange type to a straightforward object with string properties aligns well with native date input values.
24-39
: Good validation logic for date ranges.The handler correctly validates that the end date isn't before the start date and provides appropriate error feedback.
One minor consideration:
- if (value < startDate) { + // Safe string comparison for ISO dates (YYYY-MM-DD format) + if (value < startDate) {
43-63
: Clean UI implementation with native date inputs.The UI is well-structured with appropriate labels and conditional styling for error states. Using native date inputs provides a better user experience across devices.
src/components/ui/date-time-picker.tsx (6)
2-2
: Good simplification of imports.Removing complex date-handling imports and just using useState simplifies the component.
6-6
: Appropriate UI component import.The Input component is correctly imported for the new implementation.
8-14
: Props interface update is appropriate.Changing from Date objects to strings matches the native input behavior and simplifies the API.
16-21
: Good function signature update with sensible defaults.Adding className to destructured props and providing a default value for disabled improves the component API.
24-28
: Simplified event handler looks good.The handler is much simpler now, properly updating state and calling the onChange callback.
35-42
: Clean UI implementation with native datetime input.Using the native datetime-local input significantly simplifies the UI while providing a better user experience.
Proposed Changes
@ohcnetwork/care-fe-code-reviewers


Merge Checklist
Summary by CodeRabbit