[DateRangePicker] Fix broken active range position underline in single input range fields#23166
Conversation
…ange fields The single input range field's active range underline (`activeBar`) rendered at `width: 0px` (invisible) because its width computation was gated on `sectionElement.content.id`, which was removed from section content props in mui#19523. Guard on `data-range-position` instead, which is still present and already identifies range sections. Adds a browser regression test (offsetWidth is 0 in jsdom, so it is browser-only). Co-Authored-By: Claude Opus 4.8 <[email protected]>
LukasTy
added
type: regression
LukasTy
changed the title
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
LukasTy
added
needs cherry-pick
Adds a browser regression test verifying the active bar's `left` shifts from the start sections to the end sections when the end date is being edited, covering the bar's positioning alongside the existing width check. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…rolled prop Rewrites the active bar movement test to select the start date through the calendar (the real user flow that advances the picker to the end position), instead of flipping the controlled `rangePosition` prop with `setProps`. Co-Authored-By: Claude Opus 4.8 <[email protected]>
The regression origin is documented in the PR description, so the inline comment only needs to state what the guard does. Co-Authored-By: Claude Opus 4.8 <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression where the active range-position underline (activeBar) in single-input DateRangePicker fields renders with width: 0, making it invisible. It updates the width-measurement guard in PickersInputBase to rely on the still-present data-range-position attribute and adds browser-only coverage to prevent future regressions.
Changes:
- Fix
activeBarwidth measurement by guarding oncontent['data-range-position']instead of the removedcontent.id. - Add browser-only
DateRangePickertests to assertactiveBarhas non-zero width and moves fromstarttoendafter selecting the start date.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/x-date-pickers/src/PickersTextField/PickersInputBase/PickersInputBase.tsx | Updates the activeBar measurement guard to correctly detect range sections via data-range-position. |
| packages/x-date-pickers-pro/src/DateRangePicker/DateRangePicker.test.tsx | Adds browser-only tests validating activeBar width and left offset behavior in single-input range fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
flaviendelangle
left a comment
flaviendelangle
left a comment
There was a problem hiding this comment.
PR review
Correct, minimal regression fix: the activeBar width guard keyed off content.id, which PR #19523 silently removed, permanently zeroing the width. Switching the guard to content['data-range-position'] restores it and reads as intent. The values data-range-position can hold ('start' | 'end' | undefined) make the truthiness guard safe, and the whole measurement path is already gated behind isSingleInputRange, so there's no non-range regression. Nothing merge-blocking; one optional consistency note.
Bugs (0)
No findings.
Tests (0)
No findings.
The two new browser tests correctly pin the regression (non-zero offsetWidth) and its positioning (bar shifts right after advancing to the end position). it.skipIf(isJSDOM) is right since offsetWidth/offsetLeft are 0 in jsdom. getPickerDay('3') matches the established exact-name gridcell pattern and resolves uniquely in the January 2018 grid.
Simplifications (1)
1. ℹ️ Guard predicate differs in style from the sibling isSingleInputRange check
Location: packages/x-date-pickers/src/PickersTextField/PickersInputBase/PickersInputBase.tsx:271
// new guard
if (sectionElement.content['data-range-position']) { ... }
// sibling check, same field, line ~443
const isSingleInputRange = elements.some(
(element) => element.content['data-range-position'] !== undefined,
);Both read the same property but one uses truthiness and the other an explicit !== undefined. They're functionally identical here (the value is only ever 'start', 'end', or undefined), so this is purely optional — matching !== undefined would make the two range-detection checks visibly consistent and immune to a future value change. Not worth blocking on.
Failure scenario: No runtime effect today; only a minor readability/consistency cost for a future maintainer comparing the two checks.
Fix: Optionally write if (sectionElement.content['data-range-position'] !== undefined) to mirror isSingleInputRange.
Docs (0)
No findings.
Verdict
Approve — a correct, well-scoped regression fix with adequate browser test coverage; the only note is an optional style-consistency nit.
🤖 Review generated with Claude Code
Use an explicit `!== undefined` in the active bar width guard so it reads the same way as the `isSingleInputRange` check on the same property. Functionally identical, since `data-range-position` is normalized to `undefined` when unset. Co-Authored-By: Claude Opus 4.8 <[email protected]>
|
Cherry-pick PRs will be created targeting branches: v8.x |
|
Cherry-pick PRs will be created targeting branches: v8.x, v8.x |
|
Cherry-pick PRs will be created targeting branches: v8.x |
|
Cherry-pick PRs will be created targeting branches: v8.x, v8.x |
Summary
The active range position underline (
activeBar) that marks the range side currently being selected in a single input range field is visually broken -- it renders but is invisible.Root cause
The bar's width is computed in
resolveSectionElementWidth(PickersInputBase.tsx), which is gated onif (sectionElement.content.id). When theactiveBarwas introduced in #16938 every section's content object carried anid(${id}-${section.type}), so that guard was effectively always true. #19523 later removed thatidfrom the section content props as redundant, which silently turned the guard into a permanent short-circuit:resolveSectionElementWidthnow always returns0, soactiveBarWidthis always0and the bar renders atwidth: 0px.displaystill toggles andleftstill positions correctly -- only the width was zeroed, which is why the bar was invisible rather than mispositioned.Fix
Guard on
content['data-range-position']instead of the removedcontent.id. It is the typed'start' | 'end'signal that is still present on every range section and already drivesisSingleInputRangein the same file, so it is the natural, intent-revealing replacement.Testing
There were previously zero tests referencing the
activeBar, which is why the regression slipped through. This adds two browser-only tests (DateRangePicker.test.tsx): one asserting the bar has a non-zero width once a single-input desktop range picker is open, and one asserting the bar'sleftshifts to the end date sections after the start date is selected (which advances the picker to the end position). They areskipIf(isJSDOM)becauseoffsetWidth/offsetLeftare always0in jsdom. Verified the width test fails on the old guard and passes with the fix.Visual comparison