feat(fields): let popover fields anchor to an outer element - #1422
Merged
Merged
Conversation
`DialogTrigger` has always taken `targetRef` — "the ref of the element the Dialog should visually attach itself to" — but none of the ten field components that build a popover on the same machinery passed it through, so a consumer could not redirect the anchor. That is what forced the scope carve-out in cubedevinc/cubejs-enterprise#14818: dashboard filter popovers anchor to the whole filter row so adjacent filters line up, which the five `DialogTrigger`-based controls can do and the three built on ui-kit fields cannot. Adds `targetRef` to `Select`, `Picker`, `FilterPicker`, `ColorPicker`, `ColorInput`, `DatePicker`, `DateRangePicker`, `DateRangeSeparatedPicker` and `PeriodPicker`, and `placement` to all of those but `FilterPicker`, which already had it. `MonthPicker`, `QuarterPicker`, `YearPicker` and `WeekPicker` inherit both from `PeriodPicker`. Every default is the current behaviour, so existing callers are untouched. `placement` is not in the ticket for `Select`, but `targetRef` is half a fix without it: `Select` only had `direction`, which resolves to `${direction} end`, so anchored to a wide row it would right-align while the `DialogTrigger` controls beside it start-align — the popovers still would not line up. The two mechanisms differ, and the per-prop docs say so: on `Select` the ref moves the POSITION only, since it goes to `useOverlayPosition` while `triggerRef` stays the focus/dismiss anchor; on the `DialogTrigger` components the anchor also becomes what outside-click dismissal treats as the trigger. `minWidth` stays tied to the trigger everywhere. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tenphi
had a problem deploying
to
Chromatic staging
September 22, 2026 16:18 — with 
GitHub Actions
Error
GitHub Actions
Error
🦋 Changeset detectedLatest commit: 87c650f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📦 NPM canary releaseDeployed canary version 0.0.0-canary-48c41d1. |
Contributor
🏋️ Size limit report
Compared against main at 095f53c — run 35745770032, 2026-09-22T15:13:05Z.To see which modules changed, download the size-limit-statoscope-report artifact from this run and open report.html. |
Contributor
🧪 Storybook is successfully deployed!
|
This branch was successfully deployed
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
1 participant
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.
Closes CUB-4672.
The gap
DialogTriggerhas always takentargetRef— "the ref of the element the Dialog should visually attach itself to" — but none of the ten field components that build a popover on the same machinery passed it through. A consumer had no way to redirect the anchor.That is what forced the scope carve-out in cubedevinc/cubejs-enterprise#14818: dashboard filter popovers anchor to the whole filter row so adjacent filters line up. The five
DialogTrigger-based controls can do that; the three built on ui-kit fields could not, so on one board a boolean filter's list opened offset from the date filter's popover beside it.The change
targetRefonSelect,Picker,FilterPicker,ColorPicker,ColorInput,DatePicker,DateRangePicker,DateRangeSeparatedPickerandPeriodPicker;placementon all of those exceptFilterPicker, which already had it.MonthPicker,QuarterPicker,YearPickerandWeekPickerinherit both fromPeriodPicker(verified in the emitted.d.ts). Every default is the current behaviour, so existing callers are untouched.Two deliberate deviations from the ticket
placementonSelect, which the ticket asks for onPickeronly.targetRefalone is half a fix there:Selecthad onlydirection, which resolves to`${direction} end`, so anchored to a wide row it right-aligns while theDialogTriggercontrols beside it start-align — the popovers still would not line up, which is the ticket's whole goal.Nine components rather than two. No field component exposed
targetRef, so fixing only the two named would have left seven siblings with an identical gap.The two mechanisms are not identical, and the docs say so
Selectpositions throughuseOverlayPosition, sotargetRefmoves the position only —triggerRefstays the focus/outside-click/dismiss anchor. Anchoring to a wide container does not make clicks inside it count as trigger presses.DialogTriggercomponents forward the ref, so the anchor also becomes what outside-click dismissal treats as the trigger (DialogTrigger.tsx:385).minWidthstays tied to the trigger everywhere, so a narrow control anchored to a wide row keeps its own popover width — matching how theDialogTrigger-based controls already behave.Note
ListBoxPopoveralready solved this internally aspositionTargetRefand is not exported, so it is left alone.Verification
pnpm testpnpm test:browserpnpm build(emits.d.ts— the broad typecheck)pnpm test:types/test:types:consumerpnpm audit-docspnpm sizemain(both ~590.5 kB against a 592 kB budget)pnpm chromatic:checkThe new browser test was falsified, not just run. Positioning can only be asserted with real layout — in jsdom every
getBoundingClientRect()is0 × 0, so a build ignoringtargetRefentirely would pass a jsdom spec that looked identical. With the wiring deliberately removed, both anchoring cases fail and the default-behaviour case correctly still passes; restored, all four pass.It also covers the non-obvious risk:
DialogTriggerstops attaching its own ref to the trigger child oncetargetRefis set, andPickerputs its own ref there to hand focus back on close. That path is asserted directly rather than reasoned about.No stories added —
targetRefis a positioning prop that photographs as "a popover, somewhere", so a story would cost a recurring Chromatic snapshot without showing anything the docs do not.Follow-up
The Cloud call site can drop its carve-out and pass
targetRefonce console-ui bumps its pin.🤖 Generated with Claude Code
Correction: the size row originally read
526.92 kB. That was measured against a staledist/—pnpm sizebundles fromdist/, so it must follow a freshpnpm build. Re-measured with a fresh build,mainis 590.48 kB and this branch is the same to the reported precision. The budget still passes, but headroom is ~1.5 kB, not ~65 kB.