fix(website): stop collection date range fields bouncing between values while typing#6661
Merged
Conversation
…es while typing The "Collection date" From/To inputs (and any other `rangeOverlapSearch` field) could intermittently bounce back and forth between two values while typing. `DateRangeField` kept its own `lowerValue`/`upperValue` state and synced it bidirectionally with the global `fieldValues` via two effects: - a write effect: local state -> setSomeFieldValues -> URL/query state -> re-derived `fieldValues` - a read effect: `fieldValues` -> setLowerValue/setUpperValue `fieldValues` is recomputed from URL state and lags one round-trip behind while typing (each keystroke goes through setSomeFieldValues + setPage(1) + a parent useMemo). Typing a second character before the first round-trip landed let the read effect fire with a stale `fieldValues` and reset the local state backward, which the write effect then pushed back out — so the value ping-ponged between the freshly typed value and the lagging one. Fix: make `fieldValues` the single source of truth. - Remove the `lowerValue`/`upperValue` local state and both mirroring effects. - Derive the displayed values directly from `fieldValues`. - Write only on real user interaction via a `commit(...)` helper called from the date inputs' onChange and the strict-mode toggle — never from an effect, so there is no feedback loop. - Keep a single effect that re-derives `strictMode` from the populated fields (guarded so toggling with no dates doesn't snap back); it is a pure function of which fields exist and cannot oscillate. Tests: - Add a test asserting no write happens on mount. - Rewrite "updates query params" with a stateful wrapper (the old version passed only because the buggy local mirror accumulated edits without feedback) and assert both edits are retained, i.e. neither value bounces back. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
anna-parker
reviewed
Jun 16, 2026
anna-parker
approved these changes
Jun 16, 2026
anna-parker
left a comment
Contributor
There was a problem hiding this comment.
approving as this seems like an overall improvement and testing can be improved without further reviews
…ateRangeField spec Replace the three duplicated inline Wrapper components with a single, documented StatefulDateRangeField helper that mimics the production useSearchPageState.setSomeFieldValues feedback (null/'' deletes the key). This addresses review feedback asking why the wrapper exists: a vi.fn() mock can't catch the bouncing bug because the field derives what it shows from its fieldValues prop, so edits must actually feed back through that prop for a value to bounce. The doc comment now explains this once. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per review: we don't need a dedicated unit-level regression test for the specific bouncing bug, so remove the StatefulDateRangeField harness, the typing-bounce test, and the explanatory comments. Keeps the spec as it was on main, with only the change the fix actually requires: the field no longer writes to query params on mount, so the old on-mount assertion in "derives switches mode correctly" is dropped. Date-picker coverage already exists at the integration level (active-filters.dependent.spec.ts exercises the calendar picker). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Relates to https://loculus.slack.com/archives/C05G172HL6L/p1781275386940369 - hard to reproduce the original issue to test but the new approach seems strictly better, and less code (apart from added tests)
Below written by AI
Problem
The Collection date From/To inputs (and any other field configured with
rangeOverlapSearch) could intermittently bounce back and forth between two values while typing — a race condition reported during normal use of the search page.Root cause
In the default Loculus config,
collectionDateusesrangeOverlapSearch(kubernetes/loculus/values.yaml), so the "Collection date / From / To" box is rendered byDateRangeField.DateRangeFieldkept its ownlowerValue/upperValuestate and synced it bidirectionally with the globalfieldValuesvia twouseEffects:setSomeFieldValues→ URL/query state → re-derivedfieldValuesfieldValues→setLowerValue/setUpperValuefieldValuesis recomputed from URL state on every change and lags one round-trip behind while typing (each keystroke goes throughsetSomeFieldValues+setPage(1)+ a parentuseMemo). When you type a second character before the first round-trip lands, the read effect fires with a stalefieldValuesand resets the local state backward; the write effect then pushes that stale value back out — so the field ping-pongs between the freshly typed value and the lagging one. This is the classic two-effect mirror race.This is a longstanding bug, not a regression from a recent change.
Fix
Make
fieldValuesthe single source of truth:lowerValue/upperValuelocal state and both mirroring effects.fieldValues.commit(...)helper invoked from the date inputs'onChangeand the strict-mode toggle — never from an effect, so there is no feedback loop.strictModefrom the populated fields (guarded so toggling strictness with no dates entered doesn't snap back). It is a pure function of which fields exist and cannot oscillate against the writes.No behavioural change to how dates map onto the four underlying
Lower*/Upper*fields, or to strict/non-strict mode switching.Tests
fieldValues(the old version passed only because the buggy local mirror accumulated edits without any feedback) and assert both edits are retained — i.e. neither value bounces back.DateRangeField(12) andDateField(4) tests pass.🤖 Generated with Claude Code
🚀 Preview: https://fix-date-range-field-boun.loculus.org