Skip to content

feat(UniversalFilter): add date-range picker support#1683

Merged
alexquincy merged 2 commits into
masterfrom
feat/universalfilter-date-range
Jul 2, 2026
Merged

feat(UniversalFilter): add date-range picker support#1683
alexquincy merged 2 commits into
masterfrom
feat/universalfilter-date-range

Conversation

@alexquincy

Copy link
Copy Markdown
Contributor

Summary

  • Ports the quick-ranges + custom start/end datetime picker feature from meshery-cloud's homegrown filter component (ui/utility/custom-filter.tsx) into Sistent's UniversalFilter, so meshery-cloud's events/stats page (and any other consumer) can drop its local implementation entirely.
  • New optional props on UniversalFilterProps: datePicker, selectedDateRange, setSelectedDateRange, quickDateRanges. Date-range changes apply immediately (independent of the existing dropdown filters' draft+Apply flow), matching the prior UX.
  • Adds a src/base/DateTimePicker wrapper around @mui/x-date-pickers' DateTimePicker (with LocalizationProvider/AdapterDateFns baked in), following the existing base/ wrapping convention.
  • New optional peerDependencies: @mui/x-date-pickers (^9.0.0) and date-fns (^4.0.0), both marked optional in peerDependenciesMeta since most consumers don't need this feature.
  • New exported types: DateRange, QuickDateRangeOption.

Test plan

  • TDD: wrote failing tests for the new date-range behavior first, watched them fail, then implemented
  • npm run build completes successfully (CJS/ESM/DTS all succeed; DateRange/QuickDateRangeOption types confirmed present in dist/index.d.ts)
  • npx jest — 15 suites / 319 tests pass
  • npm run lint — clean

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>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread package.json
Comment on lines +118 to +123
"@mui/x-date-pickers": {
"optional": true
},
"date-fns": {
"optional": true
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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
  1. 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.

Comment on lines +121 to +135
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
});
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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
});
};

Comment on lines +220 to +247
<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`}
/>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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
  1. Avoid hardcoding UI strings (such as button labels) in shared components. Expose these strings as configurable props to support internationalization (i18n) and localization.
  2. When exposing UI strings as configurable props in shared components to support localization, provide a default value to maintain backward compatibility.

Comment on lines +9 to +10
const DateTimePicker = React.forwardRef<HTMLDivElement, MuiDateTimePickerProps>(
(props, ref) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
const DateTimePicker = React.forwardRef<HTMLDivElement, MuiDateTimePickerProps>(
(props, ref) => {
const DateTimePicker = React.forwardRef<HTMLDivElement, MuiDateTimePickerProps<Date>>(
(props, ref) => {

alexquincy pushed a commit to meshery/meshery that referenced this pull request Jul 2, 2026
…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>
alexquincy pushed a commit to meshery/meshery that referenced this pull request Jul 2, 2026
…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>
@github-actions github-actions Bot added the area/ci Continuous integration | Build and release label Jul 2, 2026
@alexquincy alexquincy merged commit a8bfdb4 into master Jul 2, 2026
5 checks passed
@alexquincy alexquincy deleted the feat/universalfilter-date-range branch July 2, 2026 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ci Continuous integration | Build and release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants