[scheduler] Fix the event dialog moving untouched dates across timezones - #23462
Open
rita-codes wants to merge 18 commits into
Open
[scheduler] Fix the event dialog moving untouched dates across timezones#23462rita-codes wants to merge 18 commits into
rita-codes wants to merge 18 commits into
Conversation
The dialog built all-day bounds with the display timezone, so saving an all-day event viewed from another timezone — even a plain rename — shifted its stored day boundaries and stretched it across two data-zone days. computeRange now takes the all-day anchor timezone and the submit path passes the event's data timezone for edits; validation and the placeholder keep the display timezone, where ordering and rendering happen. Creations still anchor to the display timezone (the event has no timezone of its own yet). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDEgka4iDRFrenWeWFhunq
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDEgka4iDRFrenWeWFhunq
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDEgka4iDRFrenWeWFhunq
The submit read the creation placeholder twice, and the data-timezone lookup inlined the processed-event walk; a `dataTimezone` selector now carries the lookup and the 'default' fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDEgka4iDRFrenWeWFhunq
The form seeds its date fields from the display-timezone bounds, so resending them untouched re-anchors the event to the display zone: viewed from another timezone, an all-day event moved to a different day on a mere rename. Anchoring the submit in the data timezone (previous approach) could not fix this — it faithfully persisted the display day the form was seeded with. Instead, leave start/end out of the update when none of the range fields is dirty. This also keeps an untouched range out of the pattern-based recurring update, where a display-zone start could shift a weekly series' BYDAY. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDEgka4iDRFrenWeWFhunq
…e2e pin A time typed while all-day was toggled off stayed dirty after toggling it back on, re-arming the range resend and reintroducing the cross-timezone day shift. Only the keys the submitted range actually reads count now. Also: RANGE_FORM_KEYS moves next to computeRange as the single copy of the list (the placeholder keys derive from it), getDirtyValues drops its now-unused excludeKeys parameter, the recurring rename test completes the scope flow on a cross-timezone weekly series, the touch-resize test pins the seeded end time, and a test pins the whole-displayed-range semantics of a partial edit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDEgka4iDRFrenWeWFhunq
The recurring scope paths (update, delete, drag) identified the occurrence by its display-timezone start. For an all-day occurrence viewed from another timezone those bounds are day-normalized to a different instant, so the exception date and the detached event landed on the wrong data-zone day. The occurrence's own data-timezone start is the identity the expansion keys on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDEgka4iDRFrenWeWFhunq
The data-timezone bounds are now the occurrence identity for recurring scope operations, so every holder must maintain them: - setEditingOccurrenceTimes and repointEditingOccurrence sync dataTimezone alongside the display bounds, so a follow-up edit or delete of the armed occurrence targets the post-resize/post-move occurrence. - A rename-only scope change (no start/end in the changes) still re-points the armed occurrence onto the moved-to event, keeping its current bounds. - The grid event primitives take dataStart/dataEnd so drag data carries the real occurrence identity instead of one fabricated from the display segment bounds. - The 'dataTimezone' in occurrence checks share an isEventOccurrence guard, the RangeFormKey type derives from RANGE_FORM_KEYS, and the occurrenceStart parameters document the data-timezone contract. New coverage: store-level identity-sync and rename-repoint tests, cross-timezone delete pins for the dialog, context menu, and toolbar, a scope-'all' edited-day pin, the all-day-to-timed conversion, and generateOccurrenceFromEvent units. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDEgka4iDRFrenWeWFhunq
The rename fallback fed the display bounds into the data-timezone sync, so a rename-only scope change on a cross-timezone all-day armed occurrence relabeled its identity onto the display day — the repoint keyed off the wrong day and a follow-up delete excluded it. The fallback now threads the occurrence's own data bounds through setEditingOccurrenceTimes and repointEditingOccurrence, which accept explicit data bounds and only fall back to the instant-preserving relabel for genuine time changes. Also: an end-to-end cross-timezone drag test discriminates the dataStart threading through the grid primitives, the scope-'all' pin asserts the end day and the BYDAY, the exDate asserts parse through the adapter, the wrappers compute the data bounds once, and the scope-sync comments say 'change' instead of 'resize' now that renames flow through too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDEgka4iDRFrenWeWFhunq
Contributor
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
…-data-timezone # Conflicts: # packages/x-scheduler/src/internals/components/event-dialog/EventDialog.test.tsx
…he pattern math Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CC6LfHbZXD549R8qtmQojJ
…-'all' update Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CC6LfHbZXD549R8qtmQojJ
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CC6LfHbZXD549R8qtmQojJ
…elete Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CC6LfHbZXD549R8qtmQojJ
…ilder Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CC6LfHbZXD549R8qtmQojJ
…e drag, and the pending-validation untouched range Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CC6LfHbZXD549R8qtmQojJ
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CC6LfHbZXD549R8qtmQojJ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Saving an all-day event from a display timezone different from its data timezone corrupted its stored bounds — even a plain rename. The dialog seeds its date fields from the display-timezone bounds (the days the user actually sees) and resent
start/endon every save, so the persisted event was re-anchored to the display zone: a UTC "July 4th" renamed from New York moved to a different day and stretched across two data-zone days.The fix: the submit now leaves
start/endout of the update when none of the range fields was edited, reusing the form store's dirty tracking that custom fields already rely on. Only the keys the submitted range actually reads count:startDate/endDate/allDayfor an all-day save (a time typed while all-day was momentarily toggled off must not re-arm the resend), all five for a timed one. An untouched range can no longer move an event in any timezone combination, and dates the user does edit keep their existing display-timezone interpretation — consistent with how the grid renders them. The recurring path benefits for free: an untouched display-zonestartno longer enters the pattern-based recurring update, where it could shift a weekly series'BYDAY.An earlier iteration of this PR anchored the all-day
computeRangebounds in the event's data timezone instead; that could not work, because the form is seeded with display-zone day strings — the anchor faithfully persisted the wrong day. That approach is fully reverted.RANGE_FORM_KEYSnow lives next tocomputeRangeas the single copy of the key list (the placeholder keys derive from it), andgetDirtyValuesdrops its now-unusedexcludeKeysparameter.Covered by dialog-level tests: rename-only saves keep the stored dates byte-identical for all-day (both timezone directions, single- and multi-day) and timed events, the orphaned-time toggle case stays untouched, a genuine day edit still applies in the display timezone, a partial range edit submits the whole displayed range, and a rename of a cross-timezone weekly series completes the scope flow end-to-end without moving its dates or
BYDAY. Found while testing the auto-scheduling engine on #23439, which documents the pre-fix behavior — its mirrored tests can be simplified once this lands.A second review pass surfaced a sibling bug on the recurring scope paths: the occurrence handed to
updateRecurringEvent/deleteRecurringEventwas identified by its display-timezone start, which for a cross-timezone all-day occurrence is day-normalized to a different instant — so "Only this event" excluded the wrong day and detached the occurrence onto it. All call sites (dialog, toolbar, context menu, drag) now pass the occurrence's data-timezone start, the identity the recurrence expansion keys on. Covered by a scope-flow test that pins the exception date and the detached event on the event's own day.A third review pass hardened the identity change itself: the armed-occurrence snapshot (
setEditingOccurrenceTimes,repointEditingOccurrence) now keeps the data-timezone bounds in sync (a follow-up edit or delete of a resized/moved armed occurrence no longer targets the pre-change occurrence), a rename-only scope change still re-points the armed occurrence onto the moved-to event, and the grid event primitives threaddataStart/dataEndinto the drag data so the drag path carries the real occurrence identity instead of one fabricated from the display segment bounds. TheoccurrenceStartparameters now document the data-timezone contract, and the four'dataTimezone' in occurrencechecks share anisEventOccurrenceguard. All of it is pinned red/green: store-level identity-sync and rename-repoint tests, cross-timezone delete tests for the dialog, context menu, and toolbar, a scope-alledited-day pin, andgenerateOccurrenceFromEventunits.A fourth pass closed the last corner of the armed-state sync: the rename fallback fed display bounds into the identity sync, relabeling a cross-timezone all-day armed occurrence onto its display day.
setEditingOccurrenceTimes/repointEditingOccurrencenow accept explicit data bounds — a rename keeps the occurrence's own identity, a genuine time change relabels the new instants. Pinned red/green at the store level (cross-timezone rename repoint) and end-to-end with a cross-timezone timeline drag test that discriminates thedataStartthreading through the grid primitives.