fix(ui): resolve calendar open trigger and align styling to Skyflo de…#150
fix(ui): resolve calendar open trigger and align styling to Skyflo de…#150nitishbhosale7 wants to merge 1 commit intoskyflo-ai:mainfrom
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds explicit click handlers to date popover trigger buttons and tweaks GlassWrapper classes in the date picker; refactors TimeRangeDropdown to replace two controlled datetime-local string inputs with a single Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
@CodeRabbit full scan |
|
The PR description images show:
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/src/components/analytics/TimeRangeDropdown.tsx (1)
48-82:⚠️ Potential issue | 🟠 MajorRemove dead helpers to fix CI lint blockers.
Line 48 (
toUtcInputValue) and Line 62 (parseUtcInputToIso) are unused; CI already fails on these symbols.Proposed fix
-const toUtcInputValue = (date: Date): string => { - return [ - date.getUTCFullYear(), - "-", - pad2(date.getUTCMonth() + 1), - "-", - pad2(date.getUTCDate()), - "T", - pad2(date.getUTCHours()), - ":", - pad2(date.getUTCMinutes()), - ].join(""); -}; - -const parseUtcInputToIso = (value: string): string | null => { - const match = value.match( - /^(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2})$/, - ); - if (!match) { - return null; - } - - const [, year, month, day, hour, minute] = match; - const utcTimestamp = Date.UTC( - Number(year), - Number(month) - 1, - Number(day), - Number(hour), - Number(minute), - 0, - 0, - ); - const date = new Date(utcTimestamp); - return Number.isNaN(date.getTime()) ? null : date.toISOString(); -};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/analytics/TimeRangeDropdown.tsx` around lines 48 - 82, Remove the unused helper functions to satisfy the linter: delete the toUtcInputValue and parseUtcInputToIso functions from TimeRangeDropdown.tsx (the functions named toUtcInputValue and parseUtcInputToIso are dead code according to CI). Ensure any callers or references to these symbols are also removed or replaced; if they were intended to be used, either implement their usage where necessary or replace with an existing utility, otherwise simply remove both function declarations to eliminate the lint errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/src/components/analytics/TimeRangeDropdown.tsx`:
- Around line 143-153: customRange.from/to are local Date objects; calling
start.toISOString() / end.toISOString() shifts the selected day by the user's
timezone. Instead, build UTC-boundary Date objects from the date components:
compute startUtc = new Date(Date.UTC(start.getFullYear(), start.getMonth(),
start.getDate())) and endUtc = new Date(Date.UTC(end.getFullYear(),
end.getMonth(), end.getDate() + 1)) (or end at 23:59:59.999 if inclusive), then
call onCustomRangeApply(startUtc.toISOString(), endUtc.toISOString(), "UTC");
keep the existing validation (start < end), setCustomError, and setIsOpen calls,
replacing only the serialization logic that currently uses toISOString() on
local Dates.
- Around line 123-126: The Date objects created for customRange from
value.startAt and value.endAt (used in TimeRangeDropdown's customRange state and
any subsequent setters like setCustomRange) must be validated to avoid calling
toISOString() on invalid dates; add a helper isValidDate(d) =>
!Number.isNaN(d.getTime()) and use it when initializing customRange (from
value.startAt/value.endAt), when updating customRange elsewhere, and immediately
before any call to toISOString() (and before comparing dates) to either
skip/normalize the value or surface an error/clear the range if invalid.
---
Outside diff comments:
In `@ui/src/components/analytics/TimeRangeDropdown.tsx`:
- Around line 48-82: Remove the unused helper functions to satisfy the linter:
delete the toUtcInputValue and parseUtcInputToIso functions from
TimeRangeDropdown.tsx (the functions named toUtcInputValue and
parseUtcInputToIso are dead code according to CI). Ensure any callers or
references to these symbols are also removed or replaced; if they were intended
to be used, either implement their usage where necessary or replace with an
existing utility, otherwise simply remove both function declarations to
eliminate the lint errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b12cb670-94a6-4047-95a6-3e64471a6cbd
📒 Files selected for processing (2)
ui/src/components/analytics/DatePickerWithRange.tsxui/src/components/analytics/TimeRangeDropdown.tsx
| const [customRange, setCustomRange] = React.useState<DateRange | undefined>({ | ||
| from: new Date(value.startAt), | ||
| to: new Date(value.endAt), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify unguarded Date -> toISOString path in this file
rg -nP 'new Date\(value\.(startAt|endAt)\)' ui/src/components/analytics/TimeRangeDropdown.tsx
rg -nP 'toISOString\(\)' ui/src/components/analytics/TimeRangeDropdown.tsx
rg -nP 'Number\.isNaN\((start|end)\.getTime\(\)\)' ui/src/components/analytics/TimeRangeDropdown.tsxRepository: skyflo-ai/skyflo
Length of output: 450
🏁 Script executed:
cat -n ui/src/components/analytics/TimeRangeDropdown.tsx | head -180 | tail -90Repository: skyflo-ai/skyflo
Length of output: 3771
🏁 Script executed:
cat -n ui/src/components/analytics/TimeRangeDropdown.tsx | head -70Repository: skyflo-ai/skyflo
Length of output: 2485
🏁 Script executed:
cat -n ui/src/components/analytics/TimeRangeDropdown.tsx | sed -n '62,85p'Repository: skyflo-ai/skyflo
Length of output: 786
🏁 Script executed:
rg -n 'onCustomRangeApply' ui/src/components/analytics/TimeRangeDropdown.tsx -A 2 -B 2Repository: skyflo-ai/skyflo
Length of output: 551
🏁 Script executed:
# Verify if Date comparison with NaN behaves as expected
python3 << 'EOF'
import json
# Simulate: const start = new Date("invalid"); const end = new Date(); start >= end
# In JavaScript: new Date("invalid") creates {Invalid Date} where getTime() = NaN
# NaN >= any_number is always false
nan = float('nan')
valid_num = 1609459200000 # milliseconds
print(json.dumps({
"nan >= valid": nan >= valid_num,
"valid >= nan": valid_num >= nan,
"nan == nan": nan == nan,
"isnan check": str(nan) == 'nan'
}))
EOFRepository: skyflo-ai/skyflo
Length of output: 148
🏁 Script executed:
# Verify if toISOString() throws on Invalid Date
node << 'EOF'
try {
const invalidDate = new Date("invalid-string");
console.log("Invalid date created:", invalidDate);
console.log("getTime():", invalidDate.getTime());
console.log("Calling toISOString()...");
invalidDate.toISOString();
} catch (e) {
console.log("ERROR:", e.name, "-", e.message);
}
EOFRepository: skyflo-ai/skyflo
Length of output: 173
Guard Invalid Date before toISOString() to prevent runtime exceptions.
Lines 124–125 and 131–132 construct Date objects from value.startAt and value.endAt without validation. If these strings are malformed, the Date objects will be invalid (with getTime() returning NaN). The existing null check at line 138 does not catch invalid dates. Line 152 then calls toISOString() on these objects, which throws RangeError: Invalid time value for invalid dates. Additionally, comparisons with NaN dates at line 146 silently fail without alerting the user.
Apply validation using a helper like isValidDate() that checks !Number.isNaN(d.getTime()) at all Date construction points (lines 124–125, 131–132) and before calling toISOString() at line 152.
Proposed fix
+const isValidDate = (d: Date | undefined): d is Date =>
+ !!d && !Number.isNaN(d.getTime());
+
export default function TimeRangeDropdown({
value,
onQuickRangeSelect,
onCustomRangeApply,
}: TimeRangeDropdownProps) {
+ const safeStart = new Date(value.startAt);
+ const safeEnd = new Date(value.endAt);
const [isOpen, setIsOpen] = React.useState(false);
const [customRange, setCustomRange] = React.useState<DateRange | undefined>({
- from: new Date(value.startAt),
- to: new Date(value.endAt),
+ from: isValidDate(safeStart) ? safeStart : undefined,
+ to: isValidDate(safeEnd) ? safeEnd : undefined,
});
@@
React.useEffect(() => {
+ const nextStart = new Date(value.startAt);
+ const nextEnd = new Date(value.endAt);
setCustomRange({
- from: new Date(value.startAt),
- to: new Date(value.endAt),
+ from: isValidDate(nextStart) ? nextStart : undefined,
+ to: isValidDate(nextEnd) ? nextEnd : undefined,
});
setCustomError(null);
}, [value.startAt, value.endAt]);
@@
const applyCustomRange = () => {
if (!customRange?.from || !customRange?.to) {
setCustomError("Select both start and end time.");
return;
}
const start = customRange.from;
const end = customRange.to;
+ if (Number.isNaN(start.getTime()) || Number.isNaN(end.getTime())) {
+ setCustomError("Invalid date range.");
+ return;
+ }
if (start >= end) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/analytics/TimeRangeDropdown.tsx` around lines 123 - 126,
The Date objects created for customRange from value.startAt and value.endAt
(used in TimeRangeDropdown's customRange state and any subsequent setters like
setCustomRange) must be validated to avoid calling toISOString() on invalid
dates; add a helper isValidDate(d) => !Number.isNaN(d.getTime()) and use it when
initializing customRange (from value.startAt/value.endAt), when updating
customRange elsewhere, and immediately before any call to toISOString() (and
before comparing dates) to either skip/normalize the value or surface an
error/clear the range if invalid.
There was a problem hiding this comment.
The UI does not allow user to select inappropriate date ranges, So backend does not get any invalid date
There was a problem hiding this comment.
The PR description image 1 shows the "before" state with a native browser datetime-local picker (light-themed, inconsistent with the dark design system). The PR description image 2 shows the "after" state: a custom dark-themed calendar with month navigation, range selection (Apr 06 and Apr 13 highlighted), and an "Apply" button — consistent with the Skyflo design.
@nitishbhosale7, that's a fair point — dates selected through the DatePickerWithRange calendar component are always valid Date objects, so the toISOString() call in applyCustomRange is indeed safe for the user-interaction path.
The residual concern is narrower: the new Date(value.startAt) / new Date(value.endAt) calls at initialization and in the useEffect depend on what the parent passes as props — not on the calendar UI. If value.startAt or value.endAt ever arrives malformed (e.g., stale localStorage, a URL query-param, or an unexpected API response), the component would silently initialize customRange with an Invalid Date, which could produce subtle visual glitches or silent failures later.
If the parent always guarantees valid ISO strings (e.g., they are only ever set through this same picker), the concern is moot in practice and I understand your reasoning. However, for enterprise-grade resilience, a lightweight guard at the prop-initialization boundary (not at the Apply click) would make the component self-contained and defensive:
const isValidDate = (d: Date): boolean => !Number.isNaN(d.getTime());
// In useState initializer and useEffect:
from: isValidDate(new Date(value.startAt)) ? new Date(value.startAt) : undefined,
to: isValidDate(new Date(value.endAt)) ? new Date(value.endAt) : undefined,Whether to add this is ultimately your call — if the prop contract is fully controlled and guaranteed upstream, the current code is functionally safe for the scenarios you've described.
🧠 Learnings used
Learnt from: KaranJagtiani
Repo: skyflo-ai/skyflo PR: 0
File: :0-0
Timestamp: 2026-02-01T04:27:59.630Z
Learning: Skyflo.ai pull requests must be reviewed as top-tier, enterprise-grade software. Enforce best-in-class open-source standards across infrastructure, security, performance, developer ergonomics, and user experience. Assume real production impact and long-term ownership by default. No partial correctness. No cosmetic approvals.
| const start = customRange.from; | ||
| const end = customRange.to; | ||
|
|
||
| if (start >= end) { | ||
| setCustomError("Start time must be before end time."); | ||
| return; | ||
| } | ||
|
|
||
| setCustomError(null); | ||
| onCustomRangeApply(startIso, endIso, "UTC"); | ||
| onCustomRangeApply(start.toISOString(), end.toISOString(), "UTC"); | ||
| setIsOpen(false); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In react-day-picker, does onSelectreturn JavaScriptDate values in the browser’s local timezone (typically local midnight for day picks)?
💡 Result:
Yes, in react-day-picker, the onSelect callback returns JavaScript Date values in the browser's local timezone, normalized to local midnight (00:00:00) for the selected day when no timeZone prop is specified. This is the default behavior as DayPicker uses the browser's local time zone and date-fns functions like startOfDay, which operate in local time to create these dates from the selected day cells.
Citations:
- 1: https://daypicker.dev/localization/setting-time-zone
- 2: https://daypicker.dev/
- 3: https://github.com/gpbl/react-day-picker/blob/main/src/DayPicker.tsx
🏁 Script executed:
# Find the TimeRangeDropdown.tsx file
find . -name "TimeRangeDropdown.tsx" -type fRepository: skyflo-ai/skyflo
Length of output: 111
🏁 Script executed:
# First, check the file size to understand how to read it
wc -l ./ui/src/components/analytics/TimeRangeDropdown.tsxRepository: skyflo-ai/skyflo
Length of output: 115
🏁 Script executed:
# Read the entire file to understand the context
cat -n ./ui/src/components/analytics/TimeRangeDropdown.tsxRepository: skyflo-ai/skyflo
Length of output: 9483
Timezone handling inconsistency shifts selected day boundaries for non-UTC users.
Lines 143–153 serialize local Date objects directly to ISO: start.toISOString() treats the local date (which react-day-picker returns in the browser's local timezone) as if it were already in UTC, causing the actual instant to shift by the user's timezone offset. The "Timezone: UTC" label at line 220 reinforces this incorrect assumption.
For example, a user in PST selecting "January 15" receives 2024-01-15T08:00:00Z instead of the intended UTC midnight on that date.
Normalize to UTC boundaries using Date.UTC() with the year, month, and date components from the selected dates, ensuring proper UTC midnight/end-of-day semantics:
Proposed fix
const applyCustomRange = () => {
@@
- const start = customRange.from;
- const end = customRange.to;
+ const start = new Date(
+ Date.UTC(
+ customRange.from.getFullYear(),
+ customRange.from.getMonth(),
+ customRange.from.getDate(),
+ 0, 0, 0, 0,
+ ),
+ );
+ const end = new Date(
+ Date.UTC(
+ customRange.to.getFullYear(),
+ customRange.to.getMonth(),
+ customRange.to.getDate(),
+ 23, 59, 59, 999,
+ ),
+ );📝 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 start = customRange.from; | |
| const end = customRange.to; | |
| if (start >= end) { | |
| setCustomError("Start time must be before end time."); | |
| return; | |
| } | |
| setCustomError(null); | |
| onCustomRangeApply(startIso, endIso, "UTC"); | |
| onCustomRangeApply(start.toISOString(), end.toISOString(), "UTC"); | |
| setIsOpen(false); | |
| const start = new Date( | |
| Date.UTC( | |
| customRange.from.getFullYear(), | |
| customRange.from.getMonth(), | |
| customRange.from.getDate(), | |
| 0, 0, 0, 0, | |
| ), | |
| ); | |
| const end = new Date( | |
| Date.UTC( | |
| customRange.to.getFullYear(), | |
| customRange.to.getMonth(), | |
| customRange.to.getDate(), | |
| 23, 59, 59, 999, | |
| ), | |
| ); | |
| if (start >= end) { | |
| setCustomError("Start time must be before end time."); | |
| return; | |
| } | |
| setCustomError(null); | |
| onCustomRangeApply(start.toISOString(), end.toISOString(), "UTC"); | |
| setIsOpen(false); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/analytics/TimeRangeDropdown.tsx` around lines 143 - 153,
customRange.from/to are local Date objects; calling start.toISOString() /
end.toISOString() shifts the selected day by the user's timezone. Instead, build
UTC-boundary Date objects from the date components: compute startUtc = new
Date(Date.UTC(start.getFullYear(), start.getMonth(), start.getDate())) and
endUtc = new Date(Date.UTC(end.getFullYear(), end.getMonth(), end.getDate() +
1)) (or end at 23:59:59.999 if inclusive), then call
onCustomRangeApply(startUtc.toISOString(), endUtc.toISOString(), "UTC"); keep
the existing validation (start < end), setCustomError, and setIsOpen calls,
replacing only the serialization logic that currently uses toISOString() on
local Dates.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/src/components/analytics/TimeRangeDropdown.tsx (1)
48-82:⚠️ Potential issue | 🟠 MajorRemove dead UTC input helpers to unblock CI.
Line 48 and Line 62 define helpers that are no longer referenced after the DateRange refactor, and lint is currently failing on them.
Proposed fix
-const toUtcInputValue = (date: Date): string => { - return [ - date.getUTCFullYear(), - "-", - pad2(date.getUTCMonth() + 1), - "-", - pad2(date.getUTCDate()), - "T", - pad2(date.getUTCHours()), - ":", - pad2(date.getUTCMinutes()), - ].join(""); -}; - -const parseUtcInputToIso = (value: string): string | null => { - const match = value.match( - /^(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2})$/, - ); - if (!match) { - return null; - } - - const [, year, month, day, hour, minute] = match; - const utcTimestamp = Date.UTC( - Number(year), - Number(month) - 1, - Number(day), - Number(hour), - Number(minute), - 0, - 0, - ); - const date = new Date(utcTimestamp); - return Number.isNaN(date.getTime()) ? null : date.toISOString(); -};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/analytics/TimeRangeDropdown.tsx` around lines 48 - 82, Remove the unused UTC helper functions to satisfy linting: delete the toUtcInputValue and parseUtcInputToIso function declarations (they are no longer referenced after the DateRange refactor) and ensure there are no remaining references to these symbols; if pad2 becomes unused after removing them, remove that import/definition as well.
♻️ Duplicate comments (1)
ui/src/components/analytics/TimeRangeDropdown.tsx (1)
123-133:⚠️ Potential issue | 🟠 MajorDate validation + UTC boundary serialization are still unresolved.
Line 123/Line 130 construct
Datefrom external strings without validity checks, and Line 152 serializes directly viatoISOString(). This can still throw on invalid dates and can shift day boundaries for non-UTC users with day-picked values. This concern was already raised earlier and still applies in this revision.#!/bin/bash # Verify unguarded Date construction + ISO serialization in this file. rg -nP 'new Date\(value\.(startAt|endAt)\)' ui/src/components/analytics/TimeRangeDropdown.tsx rg -nP 'toISOString\(\)' ui/src/components/analytics/TimeRangeDropdown.tsx rg -nP 'Number\.isNaN\(.+\.getTime\(\)\)' ui/src/components/analytics/TimeRangeDropdown.tsxIn react-day-picker v9, does onSelect for day picks return Date values in the browser local timezone (local midnight), requiring explicit UTC normalization before sending ISO UTC boundaries?Also applies to: 143-153
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/analytics/TimeRangeDropdown.tsx` around lines 123 - 133, The code constructs Dates from external strings in the customRange state and React.useEffect (new Date(value.startAt)/new Date(value.endAt)) and later serializes them with toISOString(), which can throw on invalid inputs and shift day boundaries across timezones; modify the logic around customRange, setCustomRange and the effect to first validate each Date via Number.isNaN(date.getTime()) and set customError when invalid, and when values represent day-only selections normalize to UTC boundaries before serialization (e.g., convert a day pick to a UTC start at 00:00:00.000Z and end at 23:59:59.999Z using Date.UTC or setUTCHours) so the API always receives explicit UTC start/end instants instead of local midnights; ensure any toISOString() calls use these validated/normalized Dates and avoid throwing by guarding against invalid Date instances.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/src/components/analytics/TimeRangeDropdown.tsx`:
- Around line 138-140: The current validation message uses time terminology
which is misleading for the date-picker UI; update the error text set by
setCustomError in the TimeRangeDropdown component (the branch checking
customRange?.from and customRange?.to) to use date terminology, e.g., change
"Select both start and end time." to "Select both start and end dates." so it
matches the date-range UI.
---
Outside diff comments:
In `@ui/src/components/analytics/TimeRangeDropdown.tsx`:
- Around line 48-82: Remove the unused UTC helper functions to satisfy linting:
delete the toUtcInputValue and parseUtcInputToIso function declarations (they
are no longer referenced after the DateRange refactor) and ensure there are no
remaining references to these symbols; if pad2 becomes unused after removing
them, remove that import/definition as well.
---
Duplicate comments:
In `@ui/src/components/analytics/TimeRangeDropdown.tsx`:
- Around line 123-133: The code constructs Dates from external strings in the
customRange state and React.useEffect (new Date(value.startAt)/new
Date(value.endAt)) and later serializes them with toISOString(), which can throw
on invalid inputs and shift day boundaries across timezones; modify the logic
around customRange, setCustomRange and the effect to first validate each Date
via Number.isNaN(date.getTime()) and set customError when invalid, and when
values represent day-only selections normalize to UTC boundaries before
serialization (e.g., convert a day pick to a UTC start at 00:00:00.000Z and end
at 23:59:59.999Z using Date.UTC or setUTCHours) so the API always receives
explicit UTC start/end instants instead of local midnights; ensure any
toISOString() calls use these validated/normalized Dates and avoid throwing by
guarding against invalid Date instances.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fabc3b4d-71f7-4723-a9d9-39621576d436
📒 Files selected for processing (2)
ui/src/components/analytics/DatePickerWithRange.tsxui/src/components/analytics/TimeRangeDropdown.tsx
| if (!customRange?.from || !customRange?.to) { | ||
| setCustomError("Select both start and end time."); | ||
| return; |
There was a problem hiding this comment.
Error message should reflect date-range UI terminology.
Line 139 says “Select both start and end time.” but this flow now uses date pickers. Update wording to avoid user confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/analytics/TimeRangeDropdown.tsx` around lines 138 - 140,
The current validation message uses time terminology which is misleading for the
date-picker UI; update the error text set by setCustomError in the
TimeRangeDropdown component (the branch checking customRange?.from and
customRange?.to) to use date terminology, e.g., change "Select both start and
end time." to "Select both start and end dates." so it matches the date-range
UI.
KaranJagtiani
left a comment
KaranJagtiani
left a comment
There was a problem hiding this comment.
Fix the linter issue and use this commit msg:
fix (ui): resolve calendar open trigger and integrate date picker in analytics
|
|
||
| setCustomError(null); | ||
| onCustomRangeApply(startIso, endIso, "UTC"); | ||
| onCustomRangeApply(start.toISOString(), end.toISOString(), "UTC"); |
There was a problem hiding this comment.
The old code used toUtcInputValue / parseUtcInputToIso to properly handle UTC serialization. The new code calls .toISOString() directly on local Date objects returned by react-day-picker, which are set to local midnight. This shifts the selected day by the user's timezone offset.
Normalize to UTC boundaries before serializing
| <Popover open={isEndOpen} onOpenChange={setIsEndOpen}> | ||
| <PopoverTrigger asChild> | ||
| <button | ||
| onClick={() => date?.from && setIsEndOpen(true)} |
There was a problem hiding this comment.
Check if this true:
Adding onClick={() => setIsStartOpen(true)} on a PopoverTrigger asChild button creates a conflict. Radix already attaches a click handler that toggles the popover. On click-to-close, Radix fires onOpenChange(false), then the explicit onClick immediately sets it back to true. The popover can never be dismissed by clicking the trigger again.
| function GlassWrapper({ children }: { children: React.ReactNode }) { | ||
| return ( | ||
| <div className="lg-wrapper"> | ||
| <div className="lg-wrapper "> |
There was a problem hiding this comment.
Remove trailing whitespace
| setCustomError(null); | ||
| }, [value.startAt, value.endAt]); | ||
|
|
||
| const applyCustomRange = () => { |
There was a problem hiding this comment.
new Date(value.startAt) / new Date(value.endAt) are constructed without validation. If either string is malformed, .toISOString() throws RangeError: Invalid time value at line 152.
Add a guard before serialization in applyCustomRange:
if (Number.isNaN(start.getTime()) || Number.isNaN(end.getTime())) {
setCustomError("Invalid date range.");
return;
}| const [customEndInput, setCustomEndInput] = React.useState<string>( | ||
| toUtcInputValue(new Date(value.endAt)), | ||
| ); | ||
| const [customRange, setCustomRange] = React.useState<DateRange | undefined>({ |
There was a problem hiding this comment.
The ideal approach should be to keep the original datetime-local inputs and fix the actual click/interaction bug. Style them to match the dark theme (the inputs themselves already had dark styling via bg-[#0c1222] — the issue was the browser's calendar overlay).
Build a proper datetime picker (date calendar + time input) that matches the design system and retains full precision.
2 participants
@nitishbhosale7
Description
Please include a summary of the changes and the motivation behind them.
Related Issue(s)
Fixes #149
Type of Change
Testing
Please describe the tests you've added/performed to verify your changes.
Checklist
Before Requesting Review
Code Quality
printstatements orconsole.logcallspackage-lock.json(we useyarnonly for the UI)Screenshots (if applicable)
Before Image
After Image
Additional Notes