fix(tables): canonicalize date cells, render times in effective timezone#5465
fix(tables): canonicalize date cells, render times in effective timezone#5465TheodoreSpeaks wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview UI renders instants with time in the user’s effective timezone ( CSV imports accept an optional IANA New shared module Reviewed by Cursor Bugbot for commit 10060c6. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 10060c6. Configure here.
| const handleTimeChange = (time: string) => { | ||
| setTimeOfDay(time) | ||
| const base = selected ?? today | ||
| emit(base.getFullYear(), base.getMonth(), base.getDate(), time) |
There was a problem hiding this comment.
Time picker drops seconds
Medium Severity
With showTime, the single-mode Calendar wires ChipTimePicker to minute precision only: displayed time is truncated to HH:mm, and handleTimeChange stores that value and emits YYYY-MM-DDTHH:mm without seconds. Editing the time on a cell that already had non-zero seconds re-saves the instant with seconds zeroed.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 10060c6. Configure here.
Greptile SummaryThis PR introduces canonical two-shape date storage (
Confidence Score: 3/5Two defects in new code: the Calendar's midnight guard converts stored instants to bare calendar dates on the next edit, and date-only MM/DD/YYYY inputs skip the day-in-month check that the datetime path applies — both should be fixed before merging. The core canonicalization module (
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Raw date string\n(UI input / CSV cell)"] --> B{Has explicit\noffset?}
B -- Yes --> C["Parse as UTC exactly\n(Date.parse → .toISOString)"]
B -- No --> D{Has time\ncomponent?}
D -- No --> E{ISO reduced\nform?}
E -- Yes --> F["toUtcCalendarDate\nYYYY-MM-DD via UTC"]
E -- No --> G["toLocalCalendarDate\nYYYY-MM-DD via local"]
D -- Yes --> H["wallTimeInZoneToUtc\n(2-pass DST-safe)"]
H --> I["UTC ISO instant\n2026-07-06T20:04:55.000Z"]
C --> I
F --> J["Calendar date\n2026-07-06"]
G --> J
I --> K["formatDateCellDisplay\n→ MM/DD/YYYY h:mm AM/PM\n(viewer effective timezone)"]
J --> L["formatDateCellDisplay\n→ MM/DD/YYYY\n(zone-independent)"]
subgraph ImportPath["CSV Import path"]
M["CSV naive datetime\n(Excel/Sheets export)"] --> N["coerceValue\n+ NormalizeDateCellOptions\n{timezone: user IANA zone}"]
N --> H
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["Raw date string\n(UI input / CSV cell)"] --> B{Has explicit\noffset?}
B -- Yes --> C["Parse as UTC exactly\n(Date.parse → .toISOString)"]
B -- No --> D{Has time\ncomponent?}
D -- No --> E{ISO reduced\nform?}
E -- Yes --> F["toUtcCalendarDate\nYYYY-MM-DD via UTC"]
E -- No --> G["toLocalCalendarDate\nYYYY-MM-DD via local"]
D -- Yes --> H["wallTimeInZoneToUtc\n(2-pass DST-safe)"]
H --> I["UTC ISO instant\n2026-07-06T20:04:55.000Z"]
C --> I
F --> J["Calendar date\n2026-07-06"]
G --> J
I --> K["formatDateCellDisplay\n→ MM/DD/YYYY h:mm AM/PM\n(viewer effective timezone)"]
J --> L["formatDateCellDisplay\n→ MM/DD/YYYY\n(zone-independent)"]
subgraph ImportPath["CSV Import path"]
M["CSV naive datetime\n(Excel/Sheets export)"] --> N["coerceValue\n+ NormalizeDateCellOptions\n{timezone: user IANA zone}"]
N --> H
end
Reviews (1): Last reviewed commit: "fix(tables): canonicalize date cells, re..." | Re-trigger Greptile |
| const parsed = value instanceof Date ? value : new Date(value) | ||
| if (Number.isNaN(parsed.getTime())) return { date: null, time: null } | ||
| const isMidnight = | ||
| parsed.getHours() === 0 && parsed.getMinutes() === 0 && parsed.getSeconds() === 0 | ||
| return { date: parsed, time: isMidnight ? null : timeOfDayFrom(parsed) } |
There was a problem hiding this comment.
When the stored instant's wall time in the user's effective timezone is exactly 00:00:00, pickerValue is built as "2026-07-07T00:00:00" (a naive datetime string). parseDateTimeValue parses this and finds getHours() === 0 && getMinutes() === 0 && getSeconds() === 0 → isMidnight = true → time = null. timeOfDay is then null, so any day-click calls emit(y, m, d, null) → onChange?.("YYYY-MM-DD"). Back in handlePickerChange, isCalendarDateString is true, so doSave('enter', "YYYY-MM-DD") fires immediately, converting the stored instant (e.g. "2026-07-07T07:00:00.000Z" for a PDT user) into a bare calendar date and silently discarding the UTC time offset.
The fix is to skip the midnight guard when the input is an explicit datetime string (contains T), as it was intentionally supplied with a time component.
| const parsed = value instanceof Date ? value : new Date(value) | |
| if (Number.isNaN(parsed.getTime())) return { date: null, time: null } | |
| const isMidnight = | |
| parsed.getHours() === 0 && parsed.getMinutes() === 0 && parsed.getSeconds() === 0 | |
| return { date: parsed, time: isMidnight ? null : timeOfDayFrom(parsed) } | |
| // For explicit datetime strings (containing 'T'), always preserve the time — | |
| // even midnight — because the string was intentionally built from a wall-clock | |
| // reading. The midnight guard exists only for Date objects and bare-date strings | |
| // that coincidentally land at local midnight (e.g. parseDateValue("YYYY-MM-DD") | |
| // → UTC midnight → getHours() = 0 in a UTC runtime). | |
| if (typeof value === 'string' && value.includes('T')) { | |
| const parsed = new Date(value) | |
| if (Number.isNaN(parsed.getTime())) return { date: null, time: null } | |
| return { date: parsed, time: timeOfDayFrom(parsed) } | |
| } | |
| const parsed = value instanceof Date ? value : new Date(value) | |
| if (Number.isNaN(parsed.getTime())) return { date: null, time: null } | |
| const isMidnight = | |
| parsed.getHours() === 0 && parsed.getMinutes() === 0 && parsed.getSeconds() === 0 | |
| return { date: parsed, time: isMidnight ? null : timeOfDayFrom(parsed) } |
| const full = trimmed.match(/^(\d{1,2})\/(\d{1,2})\/(\d{4})$/) | ||
| if (full) { | ||
| const month = Number(full[1]) | ||
| const day = Number(full[2]) | ||
| if (month < 1 || month > 12 || day < 1 || day > 31) return null | ||
| return `${full[3]}-${full[1].padStart(2, '0')}-${full[2].padStart(2, '0')}` | ||
| } | ||
| const partial = display.match(/^(\d{1,2})\/(\d{1,2})$/) | ||
| const partial = trimmed.match(/^(\d{1,2})\/(\d{1,2})$/) | ||
| if (partial) { | ||
| const month = Number(partial[1]) | ||
| const day = Number(partial[2]) | ||
| if (month < 1 || month > 12 || day < 1 || day > 31) return null | ||
| return `${new Date().getFullYear()}-${partial[1].padStart(2, '0')}-${partial[2].padStart(2, '0')}` | ||
| const year = todayLocalCalendarDate(timeZone).slice(0, 4) | ||
| return `${year}-${partial[1].padStart(2, '0')}-${partial[2].padStart(2, '0')}` | ||
| } |
There was a problem hiding this comment.
withTime branch applies
The withTime branch (line 175-176) correctly rejects "02/30/2026 5:00 PM" with an explicit dayCheck. However, the full branch only verifies day < 1 || day > 31, so "02/30/2026" returns the string "2026-02-30" instead of null. In the inline-editor blur path Date.parse("2026-02-30") happens to return NaN in V8 and the toast fires, but in the paste-into-cell path cleanCellValue forwards "2026-02-30" to the server, which then rejects it silently. The same gap exists in the partial branch.
| const full = trimmed.match(/^(\d{1,2})\/(\d{1,2})\/(\d{4})$/) | |
| if (full) { | |
| const month = Number(full[1]) | |
| const day = Number(full[2]) | |
| if (month < 1 || month > 12 || day < 1 || day > 31) return null | |
| return `${full[3]}-${full[1].padStart(2, '0')}-${full[2].padStart(2, '0')}` | |
| } | |
| const partial = display.match(/^(\d{1,2})\/(\d{1,2})$/) | |
| const partial = trimmed.match(/^(\d{1,2})\/(\d{1,2})$/) | |
| if (partial) { | |
| const month = Number(partial[1]) | |
| const day = Number(partial[2]) | |
| if (month < 1 || month > 12 || day < 1 || day > 31) return null | |
| return `${new Date().getFullYear()}-${partial[1].padStart(2, '0')}-${partial[2].padStart(2, '0')}` | |
| const year = todayLocalCalendarDate(timeZone).slice(0, 4) | |
| return `${year}-${partial[1].padStart(2, '0')}-${partial[2].padStart(2, '0')}` | |
| } | |
| const full = trimmed.match(/^(\d{1,2})\/(\d{1,2})\/(\d{4})$/) | |
| if (full) { | |
| const month = Number(full[1]) | |
| const day = Number(full[2]) | |
| if (month < 1 || month > 12 || day < 1 || day > 31) return null | |
| const dayCheck = new Date(Number(full[3]), month - 1, day) | |
| if (dayCheck.getMonth() !== month - 1 || dayCheck.getDate() !== day) return null | |
| return `${full[3]}-${full[1].padStart(2, '0')}-${full[2].padStart(2, '0')}` | |
| } | |
| const partial = trimmed.match(/^(\d{1,2})\/(\d{1,2})$/) | |
| if (partial) { | |
| const month = Number(partial[1]) | |
| const day = Number(partial[2]) | |
| if (month < 1 || month > 12 || day < 1 || day > 31) return null | |
| const year = Number(todayLocalCalendarDate(timeZone).slice(0, 4)) | |
| const dayCheck = new Date(year, month - 1, day) | |
| if (dayCheck.getMonth() !== month - 1 || dayCheck.getDate() !== day) return null | |
| return `${year}-${partial[1].padStart(2, '0')}-${partial[2].padStart(2, '0')}` | |
| } |
| const goToToday = () => { | ||
| setView({ month: today.getMonth(), year: today.getFullYear() }) | ||
| onChange?.(toDateString(today.getFullYear(), today.getMonth(), today.getDate())) | ||
| emit(today.getFullYear(), today.getMonth(), today.getDate(), timeOfDay) | ||
| } | ||
|
|
||
| const handleTimeChange = (time: string) => { | ||
| setTimeOfDay(time) | ||
| const base = selected ?? today | ||
| emit(base.getFullYear(), base.getMonth(), base.getDate(), time) | ||
| } |
There was a problem hiding this comment.
today is new Date() memoized at mount (browser local time). goToToday navigates the view and emits using today.get*() getters, and handleTimeChange falls back to selected ?? today. When the browser's timezone differs from the user's effective settings timezone and they straddle midnight (e.g., UTC browser at 23:30 but America/Los_Angeles effective timezone at 15:30), the "Today" button navigates to tomorrow's date in the effective timezone. Since the Calendar is intentionally timezone-agnostic, the fix would be to accept a todayStr?: string prop (a YYYY-MM-DD precomputed in the effective zone) for the single+showTime mode, falling back to new Date() when absent.
Summary
YYYY-MM-DDcalendar dates (timezone-free) and UTC ISO instants — server coercion and CSV import normalize on write, legacy rows self-heal07/06/2026 4:04 PM, seconds when present) in the user's effective timezone (settings timezone, falling back to browser); switching the setting re-renders liveCalendarsingle mode gainsshowTime(mirrors range mode) — inline cell editor gets a time field, day picks preserve the existing time instead of silently dropping it; row modal usesChipDatePicker+ChipTimePickerCalendarhighlighting the UTC day for ISO instant values, anddisplayToStorageaccepting rolled-over dates like 02/30Type of Change
Testing
Unit tests for the canonical date module (zone math, DST, exporter wire formats) and the display/parse round-trips; full lint, type-check,
check:api-validation:strictpass. Inline editor focus/blur behavior needs a manual browser pass.Checklist