feat(dashboard): Upgrade to the dateTime filter UI #2864
Conversation
Allow users to specify custom time intervals (e.g., 90 minutes) instead of being limited to preset values. Adds a numeric input field with a unit dropdown (minutes/hours/days) that works with the existing parse-duration backend. Slack thread: https://triggerdotdev.slack.com/archives/C032WA2S43F/p1767877958977499?thread_ts=1767877851.170929&cid=C032WA2S43F
Add useEffect to update customValue and customUnit when the period prop changes, fixing stale values in the custom input fields after selecting a preset time period. Co-authored-by: Eric Allam <[email protected]>
Replace raw HTML input and select with the existing Input and SimpleSelect primitives to match the app's design system aesthetic. Co-authored-by: Eric Allam <[email protected]>
|
WalkthroughAdds a client-side Calendar component built on react-day-picker with a CustomMonthCaption (month/year selects and navigation) and exports Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
…om-time-interval-2Tv4K # Conflicts: # pnpm-lock.yaml
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @apps/webapp/app/components/runs/v3/SharedFilters.tsx:
- Around line 345-353: The useEffect that initializes custom duration and
selectedPeriod uses defaultPeriod when computing setSelectedPeriod but doesn't
include defaultPeriod in its dependency array; update the dependency array of
the useEffect containing getInitialCustomDuration, setCustomValue, setCustomUnit
and setSelectedPeriod to include defaultPeriod so the effect re-runs and
selectedPeriod updates if defaultPeriod changes.
- Around line 728-740: The Cancel button onClick currently only restores from/to
and validation state; also reset duration-related state so changes don't
persist: in the Cancel handler (the Button onClick) call
setSelectedPeriod(period), setCustomValue(customValue),
setCustomUnit(customUnit) to restore the original duration props/values and
setActiveSection(null) (or the component's default section) in addition to
setFromValue(from), setToValue(to), setValidationError(null), and
setOpen(false).
🧹 Nitpick comments (4)
apps/webapp/app/components/primitives/Calendar.tsx (1)
53-60: Consider making the year range configurable or expanding it.The year dropdown shows 100 years centered on the current year (±50 years). This is reasonable for most use cases, but for applications requiring historical data (e.g., birth dates) or future planning, this range might be limiting.
♻️ Optional: Make year range configurable
+type CustomMonthCaptionProps = { + calendarMonth: { date: Date }; + yearRange?: { start: number; end: number }; +}; -function CustomMonthCaption({ calendarMonth }: { calendarMonth: { date: Date } }) { +function CustomMonthCaption({ calendarMonth, yearRange }: CustomMonthCaptionProps) { const { goToMonth, nextMonth, previousMonth } = useDayPicker(); + const currentYear = new Date().getFullYear(); + const startYear = yearRange?.start ?? currentYear - 50; + const endYear = yearRange?.end ?? currentYear + 49; + const years = Array.from({ length: endYear - startYear + 1 }, (_, i) => startYear + i); // ... use years array in the selectapps/webapp/app/components/runs/v3/SharedFilters.tsx (3)
115-128: Consider supporting additional time units.The
parsePeriodStringregex only matchesm,h,dunits, but theparse-durationbackend (per PR description) and the debounce system support additional units likew(weeks) ands(seconds). If users enter values with these units elsewhere in the system, the custom duration display may fall back to showing the raw period string.♻️ Optional: Extend supported units
const timeUnits = [ + { label: "seconds", value: "s", singular: "second", shortLabel: "secs" }, { label: "minutes", value: "m", singular: "minute", shortLabel: "mins" }, { label: "hours", value: "h", singular: "hour", shortLabel: "hours" }, { label: "days", value: "d", singular: "day", shortLabel: "days" }, + { label: "weeks", value: "w", singular: "week", shortLabel: "weeks" }, ]; function parsePeriodString(period: string): { value: number; unit: string } | null { - const match = period.match(/^(\d+)([mhd])$/); + const match = period.match(/^(\d+)([smhdw])$/); if (match) { return { value: parseInt(match[1], 10), unit: match[2] }; } return null; }
659-659: PreferaddDaysoversubDayswith negative value for clarity.Using
subDays(saturday, -1)works but is counter-intuitive. Consider usingaddDaysfrom date-fns for better readability.♻️ Proposed improvement
+import { addDays } from "date-fns"; // ... -const sunday = endOfDay(subDays(saturday, -1)); +const sunday = endOfDay(addDays(saturday, 1));
458-476: Consider removing unconditionalautoFocuson custom duration input.The
autoFocusattribute will focus this input every time the dropdown opens, even if the user intends to select a preset duration. This may interfere with keyboard navigation and preset button clicks. Consider conditionally applying focus only when the custom duration is already selected.♻️ Proposed fix
<input type="number" min="1" step="1" placeholder="Custom" value={customValue} - autoFocus + autoFocus={selectedPeriod === "custom"}
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
apps/webapp/app/components/primitives/Calendar.tsxapps/webapp/app/components/primitives/DateField.tsxapps/webapp/app/components/primitives/DateTimePicker.tsxapps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/package.json
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/package.jsonapps/webapp/app/components/primitives/DateTimePicker.tsxapps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/components/primitives/Calendar.tsxapps/webapp/app/components/primitives/DateField.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/components/primitives/DateTimePicker.tsxapps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/components/primitives/Calendar.tsxapps/webapp/app/components/primitives/DateField.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/primitives/DateTimePicker.tsxapps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/components/primitives/Calendar.tsxapps/webapp/app/components/primitives/DateField.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/components/primitives/DateTimePicker.tsxapps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/components/primitives/Calendar.tsxapps/webapp/app/components/primitives/DateField.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/primitives/DateTimePicker.tsxapps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/components/primitives/Calendar.tsxapps/webapp/app/components/primitives/DateField.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/components/primitives/DateTimePicker.tsxapps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/components/primitives/Calendar.tsxapps/webapp/app/components/primitives/DateField.tsx
🧠 Learnings (2)
📚 Learning: 2025-11-27T16:27:48.109Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T16:27:48.109Z
Learning: pnpm version `10.23.0` and Node.js version `20.11.1` are required for development
Applied to files:
apps/webapp/package.json
📚 Learning: 2025-12-18T14:09:10.154Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 2794
File: internal-packages/run-engine/src/engine/systems/debounceSystem.ts:390-397
Timestamp: 2025-12-18T14:09:10.154Z
Learning: In the debounce system (internal-packages/run-engine/src/engine/systems/debounceSystem.ts), millisecond delays are not supported. The minimum debounce delay is 1 second (1s). The parseNaturalLanguageDuration function supports w/d/hr/h/m/s units only.
Applied to files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
🧬 Code graph analysis (1)
apps/webapp/app/components/primitives/Calendar.tsx (1)
apps/webapp/app/utils/cn.ts (1)
cn(77-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
apps/webapp/app/components/primitives/DateField.tsx (1)
3-8: LGTM!Clean import consolidation. Using
typekeyword for type-only imports (DateFieldState,DateSegment,Granularity) follows TypeScript best practices and enables proper tree-shaking.apps/webapp/app/components/primitives/Calendar.tsx (1)
76-126: LGTM!Well-structured Calendar component with comprehensive styling, proper accessibility (sr-only caption, aria-labels), and clean prop spreading. The
weekStartsOn={1}for Monday start is a good international default.apps/webapp/app/components/primitives/DateTimePicker.tsx (2)
57-67: Consider the UX when time is set before date.When
valueis undefined and the user enters a time, the code createsnew Date()(today). This means time can be set before date selection, defaulting to "today". This might be intentional UX, but consider whether you want to require a date selection first or make this behavior explicit in the UI.
77-144: LGTM!Clean component structure with good accessibility (aria-label on time input), proper popover handling, and well-organized optional action buttons. The hidden calendar picker indicator for the time input is a nice touch for visual consistency.
apps/webapp/app/components/runs/v3/SharedFilters.tsx (3)
3-17: LGTM!Good use of tree-shakeable date-fns imports for the specific date manipulation functions needed by the quick date buttons.
787-807: LGTM!Clean helper component that follows the existing Button patterns and properly handles the active state styling.
423-536: Well-designed duration section UI.The two-mode design (duration vs date range) with visual radio button indication provides clear UX. The preset buttons syncing with the custom input values is a nice touch for user convenience.
apps/webapp/package.json (1)
140-140: Dependency versions are current and stable.Both
date-fns(^4.1.0) andreact-day-picker(^9.13.0) are confirmed as the latest stable versions and are appropriately specified. Dependencies are properly ordered alphabetically and the JSON formatting is correct.
| const handleDateSelect = (date: Date | undefined) => { | ||
| if (date) { | ||
| // Preserve the time from the current value if it exists | ||
| if (value) { | ||
| date.setHours(value.getHours()); | ||
| date.setMinutes(value.getMinutes()); | ||
| date.setSeconds(value.getSeconds()); | ||
| } | ||
| onChange?.(date); | ||
| } else { | ||
| onChange?.(undefined); | ||
| } | ||
| setOpen(false); | ||
| }; |
There was a problem hiding this comment.
Avoid mutating the incoming date parameter.
The handleDateSelect function mutates the date object directly via setHours, setMinutes, and setSeconds. This can cause unexpected side effects if the caller retains a reference to the original date object.
🔧 Proposed fix: Create a new Date object
const handleDateSelect = (date: Date | undefined) => {
if (date) {
// Preserve the time from the current value if it exists
if (value) {
- date.setHours(value.getHours());
- date.setMinutes(value.getMinutes());
- date.setSeconds(value.getSeconds());
+ const newDate = new Date(date);
+ newDate.setHours(value.getHours());
+ newDate.setMinutes(value.getMinutes());
+ newDate.setSeconds(value.getSeconds());
+ onChange?.(newDate);
+ setOpen(false);
+ return;
}
onChange?.(date);
} else {
onChange?.(undefined);
}
setOpen(false);
};
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
ericallam
changed the title
ericallam
changed the title
PR Review: chore(webapp): Upgrade to the dateTime filter UIThis is a well-structured UI/UX improvement that adds flexibility to the date/time filtering system. The implementation adds custom duration input and a new DateTimePicker component. Here is my detailed review: Code Quality and Best PracticesPositives:
Suggestions:
Potential Bugs or Issues
Performance Considerations
Security ConcernsNo significant security concerns identified. The code:
Test CoverageNo test files were added for the new components. Given the complexity of the date/time logic, consider adding tests for:
Additional Observations
SummaryOverall this is a solid PR that improves the user experience for date/time filtering. The main recommendations are:
Nice work on this feature! 🎯 |
PR Review: feat(dashboard): Upgrade to the dateTime filter UIThanks for this well-structured UI/UX improvement to the date/time picker! The new dual-mode approach (duration vs date range) with custom duration support is a nice enhancement. Here's my detailed review: ✅ Code Quality & Best PracticesStrengths:
Suggestions:
🐛 Potential Bugs & Issues
⚡ Performance Considerations
🔒 Security ConcernsNo security concerns identified. The component:
🧪 Test CoverageCurrent state: No tests found for Recommendations: Consider adding tests for:
Even simple unit tests for the helper functions would add confidence for this logic-heavy component. 📝 Minor Nits
SummaryOverall this is a solid PR that improves the UX significantly. The main actionable items are:
Nice work on the clean UI separation! 🎉 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/webapp/app/components/runs/v3/SharedFilters.tsx (3)
355-412: Consider using zod for validation per coding guidelines.The manual validation logic works correctly, but the coding guidelines specify using zod for validation in
apps/webapp. While this is simple UI state validation, consider defining a zod schema for consistency:Example zod validation approach
import { z } from "zod"; const customDurationSchema = z.object({ value: z.string().refine((val) => { const num = parseInt(val, 10); return !isNaN(num) && num > 0; }, "Please enter a valid custom duration"), unit: z.enum(["m", "h", "d"]) }); const dateRangeSchema = z.object({ from: z.date().optional(), to: z.date().optional() }).refine((data) => data.from || data.to, "Please specify at least one date");As per coding guidelines, apps/webapp should use zod for validation.
458-477: Reconsider autoFocus on custom duration input.The
autoFocusattribute on line 464 will focus the custom input field whenever the dropdown opens, even if the user intends to select a preset duration. This could be distracting since most users may use presets rather than custom durations.Consider removing
autoFocusor only applying it when the dropdown opens with an existing custom value:<input type="number" min="1" step="1" placeholder="Custom" value={customValue} - autoFocus + autoFocus={selectedPeriod === "custom"} onChange={(e) => {
423-761: Consider splitting TimeDropdown into smaller components.The
TimeDropdowncomponent is over 400 lines with complex state management and UI rendering. Consider extractingDurationSectionandDateRangeSectionas separate components to improve maintainability and testability.Potential component structure
function DurationSection({ active, onActivate, selectedPeriod, onSelectPeriod, customValue, customUnit, onCustomChange, validationError }) { /* ... */ } function DateRangeSection({ active, onActivate, fromValue, toValue, onFromChange, onToChange, selectedQuickDate, onQuickDateSelect, validationError }) { /* ... */ } // Then in TimeDropdown: <DurationSection {...durationProps} /> <DateRangeSection {...dateRangeProps} />
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/components/runs/v3/SharedFilters.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-18T14:09:10.154Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 2794
File: internal-packages/run-engine/src/engine/systems/debounceSystem.ts:390-397
Timestamp: 2025-12-18T14:09:10.154Z
Learning: In the debounce system (internal-packages/run-engine/src/engine/systems/debounceSystem.ts), millisecond delays are not supported. The minimum debounce delay is 1 second (1s). The parseNaturalLanguageDuration function supports w/d/hr/h/m/s units only.
Applied to files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
🧬 Code graph analysis (1)
apps/webapp/app/components/runs/v3/SharedFilters.tsx (9)
apps/webapp/app/hooks/useSearchParam.ts (1)
useSearchParams(7-64)apps/webapp/app/components/primitives/RadioButton.tsx (1)
RadioButtonCircle(64-91)apps/webapp/app/components/primitives/Label.tsx (1)
Label(25-41)apps/webapp/app/utils/cn.ts (1)
cn(77-79)scripts/analyze_marqs.mjs (1)
input(15-15)apps/webapp/app/components/primitives/Buttons.tsx (1)
Button(296-329)apps/webapp/app/v3/eventRepository/clickhouseEventRepository.server.ts (1)
span(1926-1943)apps/webapp/app/components/primitives/DateTimePicker.tsx (1)
DateTimePicker(24-145)apps/webapp/app/components/primitives/Paragraph.tsx (1)
Paragraph(88-107)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: claude-review
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
apps/webapp/app/components/runs/v3/SharedFilters.tsx (9)
3-30: LGTM: Import additions support new date/time filtering features.The new imports from date-fns and additional UI primitives are appropriate for the enhanced time filtering functionality.
130-134: LGTM: Default period validation.The default period of 7 days is reasonable, and validating it at module load time ensures the constant is always valid.
138-197: LGTM: Default period parameter addition.The
defaultPeriodparameter with a fallback value ofDEFAULT_PERIODprovides good flexibility while maintaining backward compatibility through the default value.
214-236: LGTM: Enhanced period rendering with custom duration support.The logic correctly handles both preset periods and custom periods, with appropriate singular/plural formatting and sensible fallbacks.
296-305: LGTM: Clean helper function.The
getInitialCustomDurationfunction appropriately extracts custom duration state from a period string with sensible defaults.
325-353: LGTM: State management for two-mode filtering.The state variables and synchronization effect appropriately support the duration vs. date-range modes. The
useEffectdependency array correctly includesdefaultPeriod.
787-807: LGTM: Clean reusable component.The
QuickDateButtoncomponent is well-designed for consistent rendering of quick-date options with appropriate visual feedback.
121-128: No changes needed. The regex pattern and time units are intentionally limited to minutes, hours, and days, which aligns with all preset period options defined intimePeriods. This is a consistent and complete implementation for the time filter feature. While week-based selections exist elsewhere in this file (for absolute date ranges like "This week"), they use absolute date functions rather than relative period strings, which is a different feature.
649-665: The last weekend date calculation logic is correct and properly handles all day-of-week cases:
- Monday–Friday:
previousSaturday()returns the most recent past Saturday, yielding the prior weekend- Saturday:
subDays(now, 7)explicitly goes back to the previous Saturday, preventing inclusion of the current day- Sunday:
subDays(now, 8)correctly goes back to the prior Saturday, preventing inclusion of yesterday's Saturday or todayThe custom handling for Saturday and Sunday is intentional to ensure "last weekend" always refers to the most recent complete past weekend (Saturday–Sunday), consistent with other quick-date filters in the component (e.g., "Yesterday" excludes today). The logic is sound.
PR Review: feat(dashboard): Upgrade to the dateTime filter UIThanks for this UI/UX improvement! The new date/time picker with custom duration support and quick date presets is a nice enhancement. Here's my feedback: ✅ What's Good
🔍 Suggestions for Improvement1. Potential Date Mutation Bug (DateTimePicker.tsx:42-55)The if (value) {
date.setHours(value.getHours());
date.setMinutes(value.getMinutes());
date.setSeconds(value.getSeconds());
}
onChange?.(date);This mutates the Date object from const newDate = new Date(date);
if (value) {
newDate.setHours(value.getHours());
// ...
}
onChange?.(newDate);2. Time Input Edge Case (DateTimePicker.tsx:57-67)When
3. Missing
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/webapp/app/components/runs/v3/SharedFilters.tsx (4)
360-417: Consider using zod for validation.The manual validation logic is correct, but the coding guidelines specify using zod for validation in
apps/webapp. Consider refactoring to use zod schemas for custom duration and date range validation.As per coding guidelines, validation in apps/webapp should use zod.
Example zod-based validation approach
import { z } from "zod"; const customDurationSchema = z.object({ value: z.string().refine( (val) => { const num = parseInt(val, 10); return !isNaN(num) && num > 0; }, { message: "Please enter a valid custom duration" } ), unit: z.enum(["m", "h", "d"]), }); const dateRangeSchema = z .object({ from: z.date().optional(), to: z.date().optional(), }) .refine((data) => data.from || data.to, { message: "Please specify at least one date", }) .refine( (data) => { if (data.from && data.to) { return data.from <= data.to; } return true; }, { message: "From date must be before To date" } );
463-482: Consider removing autoFocus from custom duration input.The
autoFocusattribute on the custom duration input (line 469) will automatically focus the field every time the dropdown opens, which may be unexpected when users want to select a preset duration or date range instead.Consider removing the
autoFocusattribute to provide a more neutral initial state when the dropdown opens.
569-600: Remove negative margin layout hack.Lines 569 and 585 use negative left margins (
-ml-8) to align the DateTimePicker components. This is a fragile approach that couples the component to its parent's padding/layout.Consider refactoring the layout structure to avoid negative margins, perhaps by adjusting the parent padding or using flexbox/grid alignment instead.
664-664: Use addDays instead of subDays with negative numbers.Using
subDayswith negative numbers (lines 664 and 678) is semantically confusing. While mathematically correct (subtracting a negative adds), it reduces code clarity.♻️ More readable alternative using addDays
For line 664:
-const sunday = endOfDay(subDays(saturday, -1)); +const sunday = endOfDay(addDays(saturday, 1));For line 678:
-const friday = endOfDay(subDays(monday, -4)); // Monday + 4 days = Friday +const friday = endOfDay(addDays(monday, 4)); // Monday + 4 days = FridayNote: You'll need to import
addDaysfromdate-fns.Also applies to: 678-678
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/components/runs/v3/SharedFilters.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-18T14:09:10.154Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 2794
File: internal-packages/run-engine/src/engine/systems/debounceSystem.ts:390-397
Timestamp: 2025-12-18T14:09:10.154Z
Learning: In the debounce system (internal-packages/run-engine/src/engine/systems/debounceSystem.ts), millisecond delays are not supported. The minimum debounce delay is 1 second (1s). The parseNaturalLanguageDuration function supports w/d/hr/h/m/s units only.
Applied to files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
🧬 Code graph analysis (1)
apps/webapp/app/components/runs/v3/SharedFilters.tsx (7)
apps/webapp/app/hooks/useSearchParam.ts (1)
useSearchParams(7-64)apps/webapp/app/components/primitives/RadioButton.tsx (1)
RadioButtonCircle(64-91)apps/webapp/app/components/primitives/Label.tsx (1)
Label(25-41)apps/webapp/app/utils/cn.ts (1)
cn(77-79)apps/webapp/app/components/primitives/Buttons.tsx (1)
Button(296-329)apps/webapp/app/components/primitives/DateTimePicker.tsx (1)
DateTimePicker(24-145)apps/webapp/app/components/primitives/Paragraph.tsx (1)
Paragraph(88-107)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: claude-review
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/webapp/app/components/runs/v3/SharedFilters.tsx (3)
792-812: LGTM - Clean component design.The
QuickDateButtoncomponent provides a consistent interface for quick date selections with clear active state indication. The implementation is straightforward and reusable.
732-761: Good keyboard shortcut implementation.The Apply button includes a keyboard shortcut (Cmd/Ctrl+Enter) with
enabledOnInputElements: true, which provides good UX by allowing users to apply filters without leaving input fields.
142-147: All signature changes are backward compatible; no breaking changes detected.The functions
timeFilters,timeFilterRenderValues, andTimeDropdownhave been updated to accept an optionaldefaultPeriodparameter with default values. Verification across the codebase shows all 7 usages oftimeFiltersand all 6 usages oftimeFilterRenderValuescontinue to use only the original parameters (period,from,to), and theTimeDropdowncomponent has a single usage within the same file where it's defined. Since the new parameter is optional with a default value, all existing call sites remain fully compatible with these changes.
PR Review: feat(dashboard): Upgrade to the dateTime filter UISummaryThis PR significantly improves the date/time filtering experience in the dashboard by introducing a new dual-mode filter UI (duration vs date range), custom duration input, quick date selection options, and new reusable Calendar and DateTimePicker components. Code Quality and Best PracticesStrengths:
Suggestions:
Potential Bugs or Issues
Performance Considerations
Security ConcernsNo security concerns identified. The component handles user input safely with proper validation before applying to URL search params. Test CoverageCurrent state: No unit tests exist for the SharedFilters component or the new Calendar/DateTimePicker components. Recommendations (per
Example test structure: describe("parsePeriodString", () => {
it("parses valid period strings", () => {
expect(parsePeriodString("90m")).toEqual({ value: 90, unit: "m" });
expect(parsePeriodString("7d")).toEqual({ value: 7, unit: "d" });
});
it("returns null for invalid strings", () => {
expect(parsePeriodString("invalid")).toBeNull();
expect(parsePeriodString("7w")).toBeNull(); // weeks not supported
});
});Additional Notes
Overall AssessmentThis is a well-implemented feature that significantly improves the date/time filtering UX. The code is clean and follows most project conventions. The main areas for improvement are:
Verdict: Approve with minor suggestions above. None of the issues are blocking. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @apps/webapp/app/components/runs/v3/SharedFilters.tsx:
- Around line 463-482: The input currently uses an unconditional autoFocus which
steals focus whenever the dropdown opens; remove the autoFocus prop from the
input and implement conditional focus via a ref and effect: create a ref (e.g.,
customInputRef), attach it to the input that currently uses
customValue/onChange/onFocus, and add a useEffect that calls
customInputRef.current?.focus() only when the dropdown is open and activeSection
=== "duration" and selectedPeriod === "custom" (depend on open, activeSection,
selectedPeriod); keep the existing onChange/onFocus handlers as-is but remove
the static autoFocus attribute so focus only occurs under the intended
conditions.
- Around line 733-745: The Cancel button handler currently only calls
setFromValue, setToValue, setValidationError, and setOpen, leaving other local
state (selectedPeriod, customValue, customUnit, activeSection,
selectedQuickDate) stale; update the Cancel onClick to also reset
selectedPeriod, customValue, customUnit, activeSection, and selectedQuickDate
back to their original/prop-derived values (or add a useEffect that watches the
dropdown open state and resets all those pieces of state when opened) so the
dropdown shows the original values after Cancel/close and reopen.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/components/runs/v3/SharedFilters.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-18T14:09:10.154Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 2794
File: internal-packages/run-engine/src/engine/systems/debounceSystem.ts:390-397
Timestamp: 2025-12-18T14:09:10.154Z
Learning: In the debounce system (internal-packages/run-engine/src/engine/systems/debounceSystem.ts), millisecond delays are not supported. The minimum debounce delay is 1 second (1s). The parseNaturalLanguageDuration function supports w/d/hr/h/m/s units only.
Applied to files:
apps/webapp/app/components/runs/v3/SharedFilters.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: claude-review
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/webapp/app/components/runs/v3/SharedFilters.tsx (4)
199-236: Enhanced period rendering with good UX.The enhanced logic correctly handles both preset periods and custom periods with improved formatting (e.g., "90 mins" instead of "90m"). The fallback chain is well-structured.
792-812: LGTM: Clean implementation of QuickDateButton.The component follows best practices with a function declaration, proper type annotations, and appropriate button attributes. The conditional styling for active state is well-implemented.
360-417: Validation logic is comprehensive and well-structured.The
applySelectioncallback properly validates both sections:
- Duration: Ensures custom values are positive numbers
- Date range: Ensures at least one date is specified and from ≤ to
The error messages are clear and user-friendly.
602-722: Quick date selections use date-fns correctly.The date calculations for quick-select options are implemented correctly:
- "Last weekend" properly handles different starting days
- "Last weekdays" correctly calculates Monday through Friday
- All date boundaries use appropriate start/end functions
Note:
weekStartsOn: 1(Monday) is used consistently, which is appropriate for international contexts.
| <input | ||
| type="number" | ||
| min="1" | ||
| step="1" | ||
| placeholder="Custom" | ||
| value={customValue} | ||
| autoFocus | ||
| onChange={(e) => { | ||
| setCustomValue(e.target.value); | ||
| setSelectedPeriod("custom"); | ||
| setActiveSection("duration"); | ||
| setValidationError(null); | ||
| }} | ||
| onFocus={() => { | ||
| setSelectedPeriod("custom"); | ||
| setActiveSection("duration"); | ||
| setValidationError(null); | ||
| }} | ||
| className="h-full w-full translate-y-px border-none bg-transparent py-0 pl-2 pr-0 text-xs leading-none text-text-bright outline-none placeholder:text-text-dimmed focus:outline-none focus:ring-0 [&::-webkit-inner-spin-button]:appearance-none [&::-webkit-outer-spin-button]:appearance-none" | ||
| /> |
There was a problem hiding this comment.
Reconsider unconditional autoFocus on custom duration input.
The autoFocus attribute always focuses the custom duration input when the dropdown opens, regardless of the active section or selected period. This interrupts the expected interaction flow:
- When a preset duration is selected (e.g., "7d"), focus should not jump to the custom input
- When the date range section should be active, focus shouldn't be in the duration section
- Keyboard users are forced to tab out even if they want to select a preset or use quick dates
♻️ Suggested fix: Remove unconditional autoFocus
<input
type="number"
min="1"
step="1"
placeholder="Custom"
value={customValue}
- autoFocus
onChange={(e) => {If focus management is desired, consider using a ref with conditional focus logic based on activeSection and selectedPeriod:
const customInputRef = useRef<HTMLInputElement>(null);
useEffect(() => {
if (open && activeSection === "duration" && selectedPeriod === "custom") {
customInputRef.current?.focus();
}
}, [open, activeSection, selectedPeriod]);Then apply the ref to the input:
<input
+ ref={customInputRef}
type="number"📝 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.
| <input | |
| type="number" | |
| min="1" | |
| step="1" | |
| placeholder="Custom" | |
| value={customValue} | |
| autoFocus | |
| onChange={(e) => { | |
| setCustomValue(e.target.value); | |
| setSelectedPeriod("custom"); | |
| setActiveSection("duration"); | |
| setValidationError(null); | |
| }} | |
| onFocus={() => { | |
| setSelectedPeriod("custom"); | |
| setActiveSection("duration"); | |
| setValidationError(null); | |
| }} | |
| className="h-full w-full translate-y-px border-none bg-transparent py-0 pl-2 pr-0 text-xs leading-none text-text-bright outline-none placeholder:text-text-dimmed focus:outline-none focus:ring-0 [&::-webkit-inner-spin-button]:appearance-none [&::-webkit-outer-spin-button]:appearance-none" | |
| /> | |
| <input | |
| type="number" | |
| min="1" | |
| step="1" | |
| placeholder="Custom" | |
| value={customValue} | |
| onChange={(e) => { | |
| setCustomValue(e.target.value); | |
| setSelectedPeriod("custom"); | |
| setActiveSection("duration"); | |
| setValidationError(null); | |
| }} | |
| onFocus={() => { | |
| setSelectedPeriod("custom"); | |
| setActiveSection("duration"); | |
| setValidationError(null); | |
| }} | |
| className="h-full w-full translate-y-px border-none bg-transparent py-0 pl-2 pr-0 text-xs leading-none text-text-bright outline-none placeholder:text-text-dimmed focus:outline-none focus:ring-0 [&::-webkit-inner-spin-button]:appearance-none [&::-webkit-outer-spin-button]:appearance-none" | |
| /> |
🤖 Prompt for AI Agents
In @apps/webapp/app/components/runs/v3/SharedFilters.tsx around lines 463 - 482,
The input currently uses an unconditional autoFocus which steals focus whenever
the dropdown opens; remove the autoFocus prop from the input and implement
conditional focus via a ref and effect: create a ref (e.g., customInputRef),
attach it to the input that currently uses customValue/onChange/onFocus, and add
a useEffect that calls customInputRef.current?.focus() only when the dropdown is
open and activeSection === "duration" and selectedPeriod === "custom" (depend on
open, activeSection, selectedPeriod); keep the existing onChange/onFocus
handlers as-is but remove the static autoFocus attribute so focus only occurs
under the intended conditions.
| <Button | ||
| variant="tertiary/small" | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| setFromValue(from); | ||
| setToValue(to); | ||
| setValidationError(null); | ||
| setOpen(false); | ||
| }} | ||
| type="button" | ||
| > | ||
| Cancel | ||
| </Button> |
There was a problem hiding this comment.
Critical: Cancel button doesn't reset all state.
The Cancel button only resets fromValue, toValue, and validationError, but doesn't reset selectedPeriod, customValue, customUnit, activeSection, or selectedQuickDate. Since the useEffect (lines 345-353) only runs when props change, not when the dropdown opens, stale state persists across open/close cycles.
Scenario: User changes custom duration to "90m", clicks Cancel, then reopens the dropdown. The UI still shows "90m" instead of the original value because the state wasn't reset.
🐛 Proposed fix: Reset all state in Cancel handler
<Button
variant="tertiary/small"
onClick={(e) => {
e.preventDefault();
setFromValue(from);
setToValue(to);
+ const parsed = getInitialCustomDuration(period);
+ setCustomValue(parsed.value);
+ setCustomUnit(parsed.unit);
+ const isCustom = period && !timePeriods.some((p) => p.value === period) && parsed.value !== "";
+ setSelectedPeriod(isCustom ? "custom" : period ?? defaultPeriod);
+ setActiveSection(from || to ? "dateRange" : "duration");
+ setSelectedQuickDate(null);
setValidationError(null);
setOpen(false);
}}
type="button"
>
Cancel
</Button>Alternatively, add a useEffect that resets state when the dropdown opens:
+useEffect(() => {
+ if (open) {
+ const parsed = getInitialCustomDuration(period);
+ setCustomValue(parsed.value);
+ setCustomUnit(parsed.unit);
+ const isCustom = period && !timePeriods.some((p) => p.value === period) && parsed.value !== "";
+ setSelectedPeriod(isCustom ? "custom" : period ?? defaultPeriod);
+ setActiveSection(from || to ? "dateRange" : "duration");
+ setFromValue(from);
+ setToValue(to);
+ setValidationError(null);
+ setSelectedQuickDate(null);
+ }
+}, [open, period, from, to, defaultPeriod]);📝 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.
| <Button | |
| variant="tertiary/small" | |
| onClick={(e) => { | |
| e.preventDefault(); | |
| setFromValue(from); | |
| setToValue(to); | |
| setValidationError(null); | |
| setOpen(false); | |
| }} | |
| type="button" | |
| > | |
| Cancel | |
| </Button> | |
| <Button | |
| variant="tertiary/small" | |
| onClick={(e) => { | |
| e.preventDefault(); | |
| setFromValue(from); | |
| setToValue(to); | |
| const parsed = getInitialCustomDuration(period); | |
| setCustomValue(parsed.value); | |
| setCustomUnit(parsed.unit); | |
| const isCustom = period && !timePeriods.some((p) => p.value === period) && parsed.value !== ""; | |
| setSelectedPeriod(isCustom ? "custom" : period ?? defaultPeriod); | |
| setActiveSection(from || to ? "dateRange" : "duration"); | |
| setSelectedQuickDate(null); | |
| setValidationError(null); | |
| setOpen(false); | |
| }} | |
| type="button" | |
| > | |
| Cancel | |
| </Button> |
🤖 Prompt for AI Agents
In @apps/webapp/app/components/runs/v3/SharedFilters.tsx around lines 733 - 745,
The Cancel button handler currently only calls setFromValue, setToValue,
setValidationError, and setOpen, leaving other local state (selectedPeriod,
customValue, customUnit, activeSection, selectedQuickDate) stale; update the
Cancel onClick to also reset selectedPeriod, customValue, customUnit,
activeSection, and selectedQuickDate back to their original/prop-derived values
(or add a useEffect that watches the dropdown open state and resets all those
pieces of state when opened) so the dropdown shows the original values after
Cancel/close and reopen.
UI/UX improvement to the date/time picker:
DatePickerV2.mp4