Add DateRangePicker Component for Date Range Selection #2757
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DateRangePicker
participant HeadlessUIPopover
participant WooDateRange
User->>DateRangePicker: Interacts (open/close, select dates)
DateRangePicker->>HeadlessUIPopover: Controls popover visibility
HeadlessUIPopover->>WooDateRange: Renders date selection UI
WooDateRange-->>DateRangePicker: Returns selected date range
DateRangePicker-->>User: Triggers onUpdate callback with new range
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/components/DateRangePicker/style.scss (1)
1-29: Add missing semicolons for consistency and consider reducing !important usage.The SCSS implementation looks good overall, but there are some style consistency issues and potential specificity concerns.
Apply this diff to add missing semicolons for consistency:
.dokan-layout { .dokan-date-range-picker { .woocommerce-calendar__react-dates .CalendarDay__selected_span { background: var(--dokan-button-background-color, #7047EB) !important; - border: 1px solid #ccc + border: 1px solid #ccc; } .woocommerce-calendar__react-dates .CalendarDay__selected_span:hover { background: var(--dokan-button-hover-background-color, #502BBF) !important; - border: 1px solid #f0f0f0 + border: 1px solid #f0f0f0; } .woocommerce-calendar__react-dates .CalendarDay__selected { background: var(--dokan-button-background-color, #7047EB) !important; - border: 1px solid #ccc + border: 1px solid #ccc; } .woocommerce-calendar__react-dates .CalendarDay__selected:hover { background: var(--dokan-button-hover-background-color, #502BBF) !important; - border: 1px solid #f0f0f0 + border: 1px solid #f0f0f0; } .woocommerce-calendar__react-dates .CalendarDay__hovered_span { background: var(--dokan-button-hover-background-color, #502BBF) !important; border: 1px solid #f0f0f0; - color: #fff + color: #fff; } } }Note on !important usage: While the
!importantdeclarations are likely necessary to override WooCommerce's calendar styles, consider if the specificity can be increased instead to avoid potential future styling conflicts.src/components/DateRangePicker/index.tsx (1)
57-57: Use consistent prop source for children.For consistency, use
updatedProps.childrensince other props use the filtered version.Apply this diff for consistency:
- { props.children ?? '' } + { updatedProps.children ?? '' }docs/frontend/components.md (1)
739-740: Fix grammar issues in the best practices section.Apply the grammar corrections identified by static analysis.
Apply this diff to fix the grammar:
2. **Date Validation** - - Use `isInvalidDate` prop to restrict date selection where needed - - Consider business rules when implementing validation logic + - Use the `isInvalidDate` prop to restrict date selection where needed + - Consider to implement business rules when implementing validation logic🧰 Tools
🪛 LanguageTool
[uncategorized] ~739-~739: You might be missing the article “the” here.
Context: ...anges 2. Date Validation - UseisInvalidDateprop to restrict date sel...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~740-~740: Did you mean “to Consider”?
Context: ...trict date selection where needed - Consider business rules when implementing valida...(MISSING_TO_BEFORE_A_VERB)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/frontend/components.md(3 hunks)src/components/DateRangePicker/index.tsx(1 hunks)src/components/DateRangePicker/style.scss(1 hunks)src/components/index.tsx(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/frontend/components.md
[uncategorized] ~739-~739: You might be missing the article “the” here.
Context: ...anges 2. Date Validation - Use isInvalidDate prop to restrict date sel...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~740-~740: Did you mean “to Consider”?
Context: ...trict date selection where needed - Consider business rules when implementing valida...
(MISSING_TO_BEFORE_A_VERB)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (2)
src/components/index.tsx (1)
27-27: LGTM!The export addition follows the existing pattern and correctly exposes the DateRangePicker component.
docs/frontend/components.md (1)
573-750: Excellent comprehensive documentation.The documentation is thorough and well-structured, covering all essential aspects including features, usage examples, props, customization, and best practices. This will be very helpful for developers using the component.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~739-~739: You might be missing the article “the” here.
Context: ...anges 2. Date Validation - UseisInvalidDateprop to restrict date sel...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~740-~740: Did you mean “to Consider”?
Context: ...trict date selection where needed - Consider business rules when implementing valida...(MISSING_TO_BEFORE_A_VERB)
| const updatedProps: any = applyFilters( 'dokan_date_range_picker_props', { | ||
| ...props, | ||
| } ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety by avoiding 'any' type.
Consider using a more specific type instead of any for better type safety.
Apply this diff to improve type safety:
- const updatedProps: any = applyFilters( 'dokan_date_range_picker_props', {
+ const updatedProps: DateRangePickerProps = applyFilters( 'dokan_date_range_picker_props', {
...props,
- } );
+ } ) as DateRangePickerProps;This provides better TypeScript support while acknowledging that applyFilters returns unknown type.
📝 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 updatedProps: any = applyFilters( 'dokan_date_range_picker_props', { | |
| ...props, | |
| } ); | |
| const updatedProps: DateRangePickerProps = applyFilters( 'dokan_date_range_picker_props', { | |
| ...props, | |
| } ) as DateRangePickerProps; |
🤖 Prompt for AI Agents
In src/components/DateRangePicker/index.tsx around lines 47 to 49, replace the
use of 'any' type for the variable updatedProps with a more specific type that
matches the expected shape of the props object. You can define or import an
appropriate interface or type for the props and use it instead of 'any' to
improve type safety. If applyFilters returns an unknown type, use a type
assertion or a generic type parameter to cast the result safely to the expected
props type.
| // @ts-ignore | ||
| // eslint-disable-next-line import/no-unresolved | ||
| import { DateRange } from '@woocommerce/components'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Address TypeScript import issue for WooCommerce components.
The @ts-ignore comment suggests type definition issues. Consider adding proper type declarations.
Verify if WooCommerce provides official type declarations or if custom types need to be created:
🏁 Script executed:
#!/bin/bash
# Search for any existing type declarations for WooCommerce components
fd -e .d.ts | xargs rg -l "woocommerce.*DateRange|DateRange.*woocommerce" || echo "No existing type declarations found"
# Check if there are any other WooCommerce component imports in the codebase
rg -A 2 -B 2 "@woocommerce/components"Length of output: 17855
Add custom type declarations for DateRange instead of using @ts-ignore
No .d.ts files were found for the DateRange export in @woocommerce/components. Rather than silencing the import error, please:
- Create an ambient declaration file (e.g.
src/types/woocommerce-components.d.ts) with a module for@woocommerce/componentsthat declaresDateRangeand its props. - Remove the
@ts-ignoreandeslint-disable-next-line import/no-unresolvedcomments insrc/components/DateRangePicker/index.tsx.
Example src/types/woocommerce-components.d.ts:
declare module '@woocommerce/components' {
import type { FC } from 'react';
export interface DateRangeProps {
/** selected start date */
startDate: Date;
/** selected end date */
endDate: Date;
onChange: (range: { startDate: Date; endDate: Date }) => void;
/** add other props as needed */
}
export const DateRange: FC<DateRangeProps>;
}Then update src/components/DateRangePicker/index.tsx:
- // @ts-ignore
- // eslint-disable-next-line import/no-unresolved
import { DateRange } from '@woocommerce/components';Removing these comments once the types are declared will restore full type safety.
🤖 Prompt for AI Agents
In src/components/DateRangePicker/index.tsx around lines 9 to 11, the import of
DateRange from '@woocommerce/components' uses @ts-ignore and eslint-disable
comments due to missing type declarations. To fix this, create a new ambient
declaration file, for example src/types/woocommerce-components.d.ts, declaring
the module '@woocommerce/components' and defining the DateRange component and
its props with proper TypeScript types. After adding this declaration file,
remove the @ts-ignore and eslint-disable-next-line comments from the import
statement in index.tsx to restore full type safety and proper linting.
| const ref = useRef( null ); | ||
| useOnClickOutside( ref, () => props.setShow( ! props.show ) ); |
There was a problem hiding this comment.
Fix the useOnClickOutside hook logic.
The current implementation has inverted logic that will toggle the popover state instead of closing it when clicking outside.
Apply this diff to fix the hook logic:
const DateRangePicker = ( props: DateRangePickerProps ): JSX.Element => {
const ref = useRef( null );
- useOnClickOutside( ref, () => props.setShow( ! props.show ) );
+ useOnClickOutside( ref, () => props.setShow( false ) );The hook should always close the popover when clicking outside, not toggle its current state.
📝 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 ref = useRef( null ); | |
| useOnClickOutside( ref, () => props.setShow( ! props.show ) ); | |
| const DateRangePicker = ( props: DateRangePickerProps ): JSX.Element => { | |
| const ref = useRef( null ); | |
| - useOnClickOutside( ref, () => props.setShow( ! props.show ) ); | |
| + useOnClickOutside( ref, () => props.setShow( false ) ); | |
| // ...rest of implementation |
🤖 Prompt for AI Agents
In src/components/DateRangePicker/index.tsx around lines 44 to 45, the
useOnClickOutside hook currently toggles the popover visibility state when
clicking outside, which is incorrect. Change the callback function passed to
useOnClickOutside so that it always sets the popover visibility to false,
ensuring the popover closes on outside clicks instead of toggling.
| ```jsx | ||
| import {useState} from '@wordpress/element'; | ||
| import {DateRangePicker} from '@dokan/components'; | ||
| import {dateI18n, getSettings} from '@wordpress/date'; | ||
|
|
||
| const ReportFilter = () => { | ||
| const [isOpen, setIsOpen] = useState(false); | ||
| const [after, setAfter] = useState('2023-01-01'); | ||
| const [afterText, setAfterText] = useState(''); | ||
| const [before, setBefore] = useState('2023-01-31'); | ||
| const [beforeText, setBeforeText] = useState(''); | ||
| const [focusedInput, setFocusedInput] = useState('startDate'); | ||
|
|
||
| return ( | ||
| <DateRangePicker | ||
| show={isOpen} | ||
| after={after} | ||
| setShow={setIsOpen} | ||
| afterText={afterText} | ||
| before={before} | ||
| beforeText={beforeText} | ||
| onUpdate={(update) => { | ||
| if (update.after) { | ||
| setAfter(update.after); | ||
| } | ||
| if (update.afterText) { | ||
| setAfterText(update.afterText); | ||
| } | ||
| if (update.before) { | ||
| setBefore(update.before); | ||
| } | ||
| if (update.beforeText) { | ||
| setBeforeText(update.beforeText); | ||
| } | ||
| if (update.focusedInput) { | ||
| setFocusedInput(update.focusedInput); | ||
|
|
||
| if (update.focusedInput !== 'endDate') { | ||
| setIsOpen(false); | ||
| } | ||
|
|
||
| if (update.focusedInput === 'endDate' && after) { | ||
| setBefore(''); | ||
| setBeforeText(''); | ||
| setIsOpen(true); | ||
| } | ||
| } | ||
| }} | ||
| shortDateFormat="MM/DD/YYYY" | ||
| focusedInput={focusedInput} | ||
| isInvalidDate={(date) => new Date(date) > new Date()} | ||
| > | ||
| <div className="date-range-button"> | ||
| <span className="components-button"> | ||
| <span className="date-range-label"> | ||
| {dateI18n(getSettings().formats.date, after)} - {dateI18n(getSettings().formats.date, before)} | ||
| </span> | ||
| </span> | ||
| </div> | ||
| </DateRangePicker> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the example implementation logic.
The example shows complex state management logic that should be validated for correctness.
Please verify this usage example works correctly, particularly the focusedInput logic in lines 627-636:
🏁 Script executed:
#!/bin/bash
# Check if there are similar patterns in the codebase for reference
ast-grep --pattern $'onUpdate={($$$) => {
$$$
if (update.focusedInput) {
$$$
}
$$$
}}'
# Search for other DateRange usage patterns
rg -A 10 "focusedInput.*endDate"Length of output: 1943
Review and adjust focusedInput logic in ReportFilter example
The current onUpdate handler will immediately close the picker whenever focus changes to anything other than "endDate". On initial open (when focus defaults to "startDate"), this will cause the picker to close instantly, preventing the user from selecting the start date.
To fix:
- Only close the picker after a date selection, not on every focus change.
- Remove or guard the
setIsOpen(false)call so it only runs when a date has actually been picked.
Suggested patch in docs/frontend/components.md around lines 627–636:
- if (update.focusedInput) {
- setFocusedInput(update.focusedInput);
-
- if (update.focusedInput !== 'endDate') {
- setIsOpen(false);
- }
-
- if (update.focusedInput === 'endDate' && after) {
- setBefore('');
- setBeforeText('');
- setIsOpen(true);
- }
- }
+ if (update.focusedInput) {
+ setFocusedInput(update.focusedInput);
+
+ // Only close after selecting the start date
+ if (update.focusedInput === 'endDate' && after) {
+ // Reset end-date fields when switching to end-date selection
+ setBefore('');
+ setBeforeText('');
+ setIsOpen(true);
+ }
+ }Now the picker stays open through the start-date selection and only resets/opens when switching to the end date.
Tagging as a fix required since the existing example logic will immediately close on open.
📝 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.
| ```jsx | |
| import {useState} from '@wordpress/element'; | |
| import {DateRangePicker} from '@dokan/components'; | |
| import {dateI18n, getSettings} from '@wordpress/date'; | |
| const ReportFilter = () => { | |
| const [isOpen, setIsOpen] = useState(false); | |
| const [after, setAfter] = useState('2023-01-01'); | |
| const [afterText, setAfterText] = useState(''); | |
| const [before, setBefore] = useState('2023-01-31'); | |
| const [beforeText, setBeforeText] = useState(''); | |
| const [focusedInput, setFocusedInput] = useState('startDate'); | |
| return ( | |
| <DateRangePicker | |
| show={isOpen} | |
| after={after} | |
| setShow={setIsOpen} | |
| afterText={afterText} | |
| before={before} | |
| beforeText={beforeText} | |
| onUpdate={(update) => { | |
| if (update.after) { | |
| setAfter(update.after); | |
| } | |
| if (update.afterText) { | |
| setAfterText(update.afterText); | |
| } | |
| if (update.before) { | |
| setBefore(update.before); | |
| } | |
| if (update.beforeText) { | |
| setBeforeText(update.beforeText); | |
| } | |
| if (update.focusedInput) { | |
| setFocusedInput(update.focusedInput); | |
| if (update.focusedInput !== 'endDate') { | |
| setIsOpen(false); | |
| } | |
| if (update.focusedInput === 'endDate' && after) { | |
| setBefore(''); | |
| setBeforeText(''); | |
| setIsOpen(true); | |
| } | |
| } | |
| }} | |
| shortDateFormat="MM/DD/YYYY" | |
| focusedInput={focusedInput} | |
| isInvalidDate={(date) => new Date(date) > new Date()} | |
| > | |
| <div className="date-range-button"> | |
| <span className="components-button"> | |
| <span className="date-range-label"> | |
| {dateI18n(getSettings().formats.date, after)} - {dateI18n(getSettings().formats.date, before)} | |
| </span> | |
| </span> | |
| </div> | |
| </DateRangePicker> | |
| ); | |
| }; | |
| // … inside your <DateRangePicker> props … | |
| onUpdate={(update) => { | |
| if (update.after) { | |
| setAfter(update.after); | |
| } | |
| if (update.afterText) { | |
| setAfterText(update.afterText); | |
| } | |
| if (update.before) { | |
| setBefore(update.before); | |
| } | |
| if (update.beforeText) { | |
| setBeforeText(update.beforeText); | |
| } | |
| if (update.focusedInput) { | |
| setFocusedInput(update.focusedInput); | |
| // Only keep the picker open when switching to end-date | |
| if (update.focusedInput === 'endDate' && after) { | |
| // Reset end-date fields when entering end-date selection | |
| setBefore(''); | |
| setBeforeText(''); | |
| setIsOpen(true); | |
| } | |
| } | |
| }} | |
| /> |
🤖 Prompt for AI Agents
In docs/frontend/components.md around lines 627 to 636, the onUpdate handler
closes the DateRangePicker immediately when the focusedInput changes to anything
other than "endDate", causing the picker to close on initial open and preventing
start date selection. To fix this, modify the logic so that setIsOpen(false) is
only called after a date has been selected, not on every focus change. Remove or
guard the setIsOpen(false) call to ensure the picker remains open during start
date selection and only closes appropriately after selection.
mralaminahamed
added
Needs: Testing
mrabbani
added
Needs: Author Reply
and removed
Needs: Dev Review
|
@mrabbani vai, changes made as per coderabbitai feedback |
mralaminahamed
added
Needs: Dev Review
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/date-range-picker/index.tsx(1 hunks)src/components/date-range-picker/style.scss(1 hunks)src/components/index.tsx(1 hunks)src/definitions/woocommerce-components.d.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/components/date-range-picker/style.scss
- src/definitions/woocommerce-components.d.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (3)
src/components/date-range-picker/index.tsx (3)
14-32: Well-structured TypeScript interface.The interface properly defines all component props with appropriate types and optional markers. The callback signatures are comprehensive and type-safe.
84-84: Standard component export.Clean default export follows React conventions.
9-10: Verify WooCommerce component import resolution.The eslint-disable comment suggests there might be module resolution issues with
@woocommerce/components. Ensure this package is properly installed and the import path is correct.#!/bin/bash # Description: Verify @woocommerce/components package installation and DateRange component existence # Check if package is installed if [ -f "package.json" ]; then echo "Checking package.json for @woocommerce/components..." cat package.json | jq '.dependencies["@woocommerce/components"], .devDependencies["@woocommerce/components"]' fi if [ -f "package-lock.json" ]; then echo "Checking package-lock.json for @woocommerce/components..." cat package-lock.json | jq '.dependencies["@woocommerce/components"] // .packages["node_modules/@woocommerce/components"]' fi # Check if there's a TypeScript declaration file for this import fd -t f -e "d.ts" . | xargs grep -l "woocommerce.*components" 2>/dev/null || echo "No TypeScript declarations found"
| */ | ||
| const DateRangePicker = ( props: DateRangePickerProps ): JSX.Element => { | ||
| const ref = useRef( null ); | ||
| useOnClickOutside( ref, () => props.setShow( false ) ); |
There was a problem hiding this comment.
Use consistent prop references throughout the component.
The component inconsistently uses props vs updatedProps. After applying WordPress filters, all prop references should use updatedProps to ensure filter modifications are respected.
Apply this diff to fix the inconsistent prop usage:
- useOnClickOutside( ref, () => props.setShow( false ) );
+ useOnClickOutside( ref, () => updatedProps.setShow( false ) );- { props.children ?? '' }
+ { updatedProps.children ?? '' }Also applies to: 57-57
🤖 Prompt for AI Agents
In src/components/date-range-picker/index.tsx at lines 44 and 57, the code
inconsistently uses `props` instead of `updatedProps` after applying WordPress
filters. To fix this, replace all instances of `props` with `updatedProps` in
these lines to ensure the component respects the filtered prop values
consistently.
| const updatedProps: DateRangePickerProps = applyFilters( | ||
| 'dokan_date_range_picker_props', | ||
| props | ||
| ) as DateRangePickerProps; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety for WordPress filter result.
The type assertion for applyFilters result bypasses TypeScript's type checking and could lead to runtime errors if filters return invalid data.
Consider adding runtime validation or using a type guard:
- const updatedProps: DateRangePickerProps = applyFilters(
- 'dokan_date_range_picker_props',
- props
- ) as DateRangePickerProps;
+ const filteredProps = applyFilters(
+ 'dokan_date_range_picker_props',
+ props
+ );
+
+ // Validate that filtered props maintain required structure
+ const updatedProps: DateRangePickerProps = {
+ ...props,
+ ...(typeof filteredProps === 'object' && filteredProps !== null ? filteredProps : {}),
+ };📝 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 updatedProps: DateRangePickerProps = applyFilters( | |
| 'dokan_date_range_picker_props', | |
| props | |
| ) as DateRangePickerProps; | |
| const filteredProps = applyFilters( | |
| 'dokan_date_range_picker_props', | |
| props | |
| ); | |
| // Validate that filtered props maintain required structure | |
| const updatedProps: DateRangePickerProps = { | |
| ...props, | |
| ...(typeof filteredProps === 'object' && filteredProps !== null ? filteredProps : {}), | |
| }; |
🤖 Prompt for AI Agents
In src/components/date-range-picker/index.tsx around lines 46 to 49, the code
uses a type assertion to cast the result of applyFilters to
DateRangePickerProps, which bypasses TypeScript's type safety and risks runtime
errors if the filter returns invalid data. To fix this, implement a runtime
validation or a type guard function that checks whether the returned object
conforms to DateRangePickerProps before assigning it to updatedProps. This
ensures type safety without relying solely on type assertions.
There was a problem hiding this comment.
Thank you for implementing the DateRangePicker component. The overall implementation looks promising, but there are some important issues that need to be addressed before this can be merged.
Required Changes
- Fix Inconsistent Props Usage
// Current implementation has inconsistent prop references
useOnClickOutside(ref, () => props.setShow(false));
{ props.children ?? '' }
// Should consistently use updatedProps after filter application
useOnClickOutside(ref, () => updatedProps.setShow(false));
{ updatedProps.children ?? '' }- Improve Type Safety
// Replace unsafe type assertion with runtime validation
const filteredProps = applyFilters(
'dokan_date_range_picker_props',
props
);
// Add proper type validation
const updatedProps: DateRangePickerProps = {
...props,
...(typeof filteredProps === 'object' && filteredProps !== null ? filteredProps : {}),
};- Resolve WooCommerce Dependencies
// Current:
// eslint-disable-next-line @typescript/no-unused-vars
import { DateRange } from '@woocommerce/components';
// Need to:
// 1. Verify @woocommerce/components is properly installed
// 2. Add proper TypeScript declarations
// 3. Remove eslint-disable if possible- Add Error Handling & Loading States
// Add error boundary
const DateRangePickerWithErrorBoundary: React.FC<DateRangePickerProps> = (props) => (
<ErrorBoundary fallback={<div>Error loading date picker</div>}>
<DateRangePicker {...props} />
</ErrorBoundary>
);
// Add loading state
const [isLoading, setIsLoading] = useState(false);
// Handle loading in onUpdate
const handleUpdate = async (update: any) => {
setIsLoading(true);
try {
await props.onUpdate(update);
} finally {
setIsLoading(false);
}
};Additional Recommendations
-
Testing
- Add unit tests for component logic
- Add integration tests with WooCommerce DateRange
- Include accessibility tests (keyboard navigation, screen readers)
- Test responsive behavior
-
Documentation
- Add JSDoc comments for all methods
- Document available WordPress filters
- Include usage examples in README
- Add prop-types documentation
-
Performance Optimizations
// Memoize callback props
const handleUpdate = useCallback((update: any) => {
props.onUpdate(update);
}, [props.onUpdate]);
// Add cleanup in effects
useEffect(() => {
// Setup code
return () => {
// Cleanup code
};
}, []);The component shows promise but needs these improvements to ensure robustness and maintainability. Please address these issues and let me know if you need any clarification or help implementing the changes.
mrabbani
removed
the
Needs: Dev Review
All Submissions:
Changes proposed in this Pull Request:
This PR introduces a new DateRangePicker component to the Dokan frontend component library. The component provides a unified, accessible date range selection interface that wraps the WooCommerce DateRange component within a Headless UI Popover for enhanced positioning and user interaction.
Key Features Added:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Basic Functionality Test:
Component Integration Test:
Accessibility Test:
Date Validation Test:
Styling Test:
Changelog entry
DateRangePicker Component Added
Added a new unified DateRangePicker component that provides a consistent date range selection interface across the Dokan admin. The component wraps WooCommerce's DateRange
Summary by CodeRabbit