feat(UniversalFilter): add date-range picker support#1683
Conversation
Ports the quick-ranges + custom start/end datetime picker feature from meshery-cloud's homegrown filter component into Sistent's UniversalFilter, so meshery-cloud's events/stats page (and any other consumer) can drop its local implementation. Adds new optional props (datePicker, selectedDateRange, setSelectedDateRange, quickDateRanges) alongside a base DateTimePicker wrapper around @mui/x-date-pickers (new optional peerDependency, alongside date-fns for the date adapter). Date-range changes apply immediately, independent of the existing dropdown filters' draft+Apply flow, matching prior UX. Signed-off-by: Lee Calcote <lee.calcote@layer5.io>
There was a problem hiding this comment.
Code Review
This pull request introduces date-range filtering capabilities to the UniversalFilter component, adding a new DateTimePicker base component and integrating @mui/x-date-pickers and date-fns. Feedback on these changes focuses on improving type safety and robustness: making the new dependencies required peer dependencies to avoid build-time resolution failures, validating manually entered dates to prevent state corruption, exposing hardcoded UI strings as props to support localization, and explicitly typing the DateTimePicker props with <Date>.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| "@mui/x-date-pickers": { | ||
| "optional": true | ||
| }, | ||
| "date-fns": { | ||
| "optional": true | ||
| }, |
There was a problem hiding this comment.
According to the repository's general rules, if a component is exported from a main entry point, its dependencies should be defined as required peer dependencies rather than optional ones. Since UniversalFilter and DateTimePicker are exported from the main entry point and statically import @mui/x-date-pickers and date-fns, marking them as optional peer dependencies will cause build-time resolution failures for consumers whose bundlers attempt to resolve these static imports. Please remove them from peerDependenciesMeta to make them required peer dependencies.
References
- If a component is exported from a main entry point, its dependencies should be defined as required peer dependencies rather than optional ones. This prevents build-time resolution failures for consumers who may not use the specific component but still have their bundlers attempt to resolve the static imports.
| const handleStartDateChange = (newStartDate: Date | null) => { | ||
| if (!newStartDate || !selectedDateRange) return; | ||
| setSelectedDateRange?.({ | ||
| startDate: newStartDate, | ||
| endDate: newStartDate > selectedDateRange.endDate ? newStartDate : selectedDateRange.endDate | ||
| }); | ||
| }; | ||
|
|
||
| const handleEndDateChange = (newEndDate: Date | null) => { | ||
| if (!newEndDate || !selectedDateRange) return; | ||
| setSelectedDateRange?.({ | ||
| startDate: newEndDate < selectedDateRange.startDate ? newEndDate : selectedDateRange.startDate, | ||
| endDate: newEndDate | ||
| }); | ||
| }; |
There was a problem hiding this comment.
When users manually type into the date-time picker, it can temporarily produce an invalid Date object (where isNaN(date.getTime()) is true). Propagating an invalid date to the state can cause rendering errors or API failures. Please add a check to ensure the date is valid before updating the state.
| const handleStartDateChange = (newStartDate: Date | null) => { | |
| if (!newStartDate || !selectedDateRange) return; | |
| setSelectedDateRange?.({ | |
| startDate: newStartDate, | |
| endDate: newStartDate > selectedDateRange.endDate ? newStartDate : selectedDateRange.endDate | |
| }); | |
| }; | |
| const handleEndDateChange = (newEndDate: Date | null) => { | |
| if (!newEndDate || !selectedDateRange) return; | |
| setSelectedDateRange?.({ | |
| startDate: newEndDate < selectedDateRange.startDate ? newEndDate : selectedDateRange.startDate, | |
| endDate: newEndDate | |
| }); | |
| }; | |
| const handleStartDateChange = (newStartDate: Date | null) => { | |
| if (!newStartDate || isNaN(newStartDate.getTime()) || !selectedDateRange) return; | |
| setSelectedDateRange?.({ | |
| startDate: newStartDate, | |
| endDate: newStartDate > selectedDateRange.endDate ? newStartDate : selectedDateRange.endDate | |
| }); | |
| }; | |
| const handleEndDateChange = (newEndDate: Date | null) => { | |
| if (!newEndDate || isNaN(newEndDate.getTime()) || !selectedDateRange) return; | |
| setSelectedDateRange?.({ | |
| startDate: newEndDate < selectedDateRange.startDate ? newEndDate : selectedDateRange.startDate, | |
| endDate: newEndDate | |
| }); | |
| }; |
| <InputLabel id={`${testId}-quick-range-label`}>Quick Ranges</InputLabel> | ||
| <Select | ||
| data-testid={`${testId}-quick-range-select`} | ||
| value="" | ||
| variant={variant} | ||
| onChange={handleQuickRangeChange} | ||
| style={{ width: '20rem', marginBottom: '1rem' }} | ||
| displayEmpty | ||
| > | ||
| {quickDateRanges.map((option) => ( | ||
| <MenuItem key={option.label} value={option.label}> | ||
| {option.label} | ||
| </MenuItem> | ||
| ))} | ||
| </Select> | ||
|
|
||
| <DateTimePicker | ||
| label="Start" | ||
| value={selectedDateRange.startDate} | ||
| onChange={handleStartDateChange} | ||
| data-testid={`${testId}-start-date`} | ||
| /> | ||
| <DateTimePicker | ||
| label="End" | ||
| value={selectedDateRange.endDate} | ||
| onChange={handleEndDateChange} | ||
| data-testid={`${testId}-end-date`} | ||
| /> |
There was a problem hiding this comment.
To support internationalization (i18n) and localization, avoid hardcoding UI strings like 'Quick Ranges', 'Start', and 'End'. Instead, expose these strings as configurable props on UniversalFilterProps (e.g., quickRangesLabel, startDateLabel, endDateLabel) with default values to maintain backward compatibility.
References
- Avoid hardcoding UI strings (such as button labels) in shared components. Expose these strings as configurable props to support internationalization (i18n) and localization.
- When exposing UI strings as configurable props in shared components to support localization, provide a default value to maintain backward compatibility.
| const DateTimePicker = React.forwardRef<HTMLDivElement, MuiDateTimePickerProps>( | ||
| (props, ref) => { |
There was a problem hiding this comment.
Explicitly type MuiDateTimePickerProps with <Date> to ensure type safety for consumers. Since AdapterDateFns is baked in, the date type is always Date, and typing it explicitly ensures that onChange and value props are correctly typed as Date instead of unknown or any.
| const DateTimePicker = React.forwardRef<HTMLDivElement, MuiDateTimePickerProps>( | |
| (props, ref) => { | |
| const DateTimePicker = React.forwardRef<HTMLDivElement, MuiDateTimePickerProps<Date>>( | |
| (props, ref) => { |
…teFns Sistent's UniversalFilter component now ships an optional date-range picker feature (layer5io/sistent#1683) built on @mui/x-date-pickers' AdapterDateFns. Sistent externalizes its peer dependencies rather than bundling them, so any consumer that resolves @sistent/sistent's dist now needs date-fns installed to satisfy AdapterDateFns's internal imports, even though Meshery UI's own date pickers use AdapterMoment. Without this, `next build` fails with "Module not found: Can't resolve 'date-fns/addDays'". Signed-off-by: Mericio <218162243+simihablo@users.noreply.github.com>
…teFns Sistent's UniversalFilter component now ships an optional date-range picker feature (layer5io/sistent#1683) built on @mui/x-date-pickers' AdapterDateFns. Sistent externalizes its peer dependencies rather than bundling them, so any consumer that resolves @sistent/sistent's dist now needs date-fns installed to satisfy AdapterDateFns's internal imports, even though Meshery UI's own date pickers use AdapterMoment. Without this, `next build` fails with "Module not found: Can't resolve 'date-fns/addDays'". Signed-off-by: Mericio <218162243+simihablo@users.noreply.github.com>
The integration test's pinned commit predates meshery/meshery#20416, which added date-fns as a Meshery UI dependency so it can resolve @mui/x-date-pickers' AdapterDateFns now that Sistent's UniversalFilter ships a date-range picker (this PR). Without the bump, the integration test still builds against a Meshery UI checkout that lacks date-fns and fails with "Module not found: Can't resolve 'date-fns/addDays'". The original reason for pinning (avoiding a broken @mui/x-tree-view v8 upgrade, meshery/meshery#18163) is stale since meshery/meshery#18167 merged months ago. Signed-off-by: Lee Calcote <lee.calcote@layer5.io>
Summary
ui/utility/custom-filter.tsx) into Sistent'sUniversalFilter, so meshery-cloud's events/stats page (and any other consumer) can drop its local implementation entirely.UniversalFilterProps:datePicker,selectedDateRange,setSelectedDateRange,quickDateRanges. Date-range changes apply immediately (independent of the existing dropdown filters' draft+Apply flow), matching the prior UX.src/base/DateTimePickerwrapper around@mui/x-date-pickers'DateTimePicker(withLocalizationProvider/AdapterDateFnsbaked in), following the existingbase/wrapping convention.@mui/x-date-pickers(^9.0.0) anddate-fns(^4.0.0), both marked optional inpeerDependenciesMetasince most consumers don't need this feature.DateRange,QuickDateRangeOption.Test plan
npm run buildcompletes successfully (CJS/ESM/DTS all succeed;DateRange/QuickDateRangeOptiontypes confirmed present indist/index.d.ts)npx jest— 15 suites / 319 tests passnpm run lint— clean