Repository navigation
Fix date range filter inputs committing on every keystroke - #74011
Merged
bbovenzi merged 3 commits intoOct 7, 2026
Merged
Conversation
) Typing in the date range picker committed the partial value on every keystroke, which updated the URL search params and let the sync effect rewrite the inputs from the committed value mid-typing, so the typed text flashed or got replaced. Keep edits local and commit explicitly: pressing Enter, closing the popover, clicking the calendar, or clearing a field. Invalid ranges are never committed.
liwenjie200543
requested review from
bbovenzi,
choo121600,
guan404ming,
pierrejeambrun,
ryanahamilton,
shubhamraj-git and
vatsrahul1001
as code owners
October 1, 2026 06:17
yuseok89
reviewed
Oct 3, 2026
The inputs were filled in the browser timezone while every commit path (typed edits, calendar picks, popover dismissal) interprets them in the selected timezone, so opening and closing the picker re-committed the displayed wall time in the selected timezone and shifted the value (e.g. 01:00 UTC displayed as 10:00 with a Seoul browser came back as 10:00 UTC). Fill the inputs in the selected timezone (initial state, value sync, and the invalid-input blur reset), and commit calendar picks as day boundaries in the selected timezone so a click followed by dismissal re-derives the same value. The dismissal commit now also compares at minute granularity (the inputs are HH:mm), so re-deriving an unchanged range on close — including a calendar-picked 23:59:59.999 end-of-day from a displayed 23:59, or a non-canonical value round-tripped from the URL — no longer fires onChange, and the sync effect re-fills the inputs when the selected timezone changes. Generated-by: ZCode (GLM)
…(eslint/tsc/prettier verified clean locally)
bbovenzi
approved these changes
Oct 7, 2026
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
1 task done
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.
Fix the date range filter inputs calling
onChangeon every valid keystroke, which triggered URL search param updates and caused the editing state to be overwritten by the synced value from the parent. This made the date picker effectively unusable — typed values would flash or get replaced with formatted values, in the DAG list filters and in the Jobs tab (both use the sharedFilterBar).Fix: Keep edits local in
handleInputChange(noonChangecall, and no side effect inside the state updater). The editing state commits explicitly via:commitEditingState()skips the commit when inputs fail validation and performs a cheap comparison so closing the popover without edits does not fireonChangeat all (the concern raised on the previous attempt). There is no Apply button — consistent with the review consensus on #69955 — and no dead props:onCommitis passed throughDateRangeFilter→DateRangeInputs→ all fourDateInputs.closes: #69891
Supersedes the closed-as-stale #69955 (independently re-implemented; the approach follows the review feedback left there).
Test plan
pnpm vitest run src/components/FilterBar/filters/DateRangeFilter.test.tsx→ 12/12 pass, including a new regression test (progressive typing never callsonChangeand the input keeps the typed value) and the updated end-of-day commit test via Enterpnpm vitest run: 1424/1426 pass; the 2 failures (queryClient.test.tsand one load-flaky spec) reproduce identically on unmodifiedmainand are in files unrelated to this changepnpm lint(eslint + tsc) passesWas generative AI tooling used to co-author this PR?
Generated-by: ZCode (GLM) following the guidelines