Feature/picasso modernization temp#4978
Draft
vedrani wants to merge 47 commits into
Draft
Conversation
🦋 Changeset detectedLatest commit: 10f00a2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 86 packages
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 |
vedrani
changed the base branch from
master
to
feature/pf-1992-migration-orchestrator
Contributor
|
vedrani
force-pushed
the
feature/picasso-modernization-temp
branch
from
cf44677 to
bdaa74e
Compare
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
vedrani
force-pushed
the
feature/picasso-modernization-temp
branch
from
9e14fb4 to
4e352de
Compare
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
vedrani
force-pushed
the
feature/picasso-modernization-temp
branch
2 times, most recently
from
ae31ce9 to
a1b0b32
Compare
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
vedrani
force-pushed
the
feature/picasso-modernization-temp
branch
from
04ba6c3 to
fe18ec8
Compare
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
* [PF-1994] Migrate Tooltip to @base-ui/react + Tailwind Rebased onto latest feature/picasso-modernization-temp to pick up the Utils <-> picasso-tailwind-merge dependency wiring (commit 43c817d). Regenerated the Tooltip and FileListItem jest snapshots for the rem-unit arrow, and the pnpm-lock Tooltip importer. Cypress close assertions use should('not.exist') to match @base-ui/react's unmount-on-close. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QTNVwrkXDV5qu3Ntv5u1vR * Seat menu-item tooltip arrow on the option per design A tooltip on a Menu.Item sat with the standard ~7px arrow gap, which in a dense option list points the tip at the strip between rows and reads as the wrong option. Per design (0-4px on menu items), menu-item anchors now use a 0px tip gap so the arrow lands on the option it describes. Scoped to `role="menuitem"` anchors, resolved lazily via base-ui's sideOffset function (no extra state/render); every other tooltip is unchanged. Autocomplete/Select options render in a Menu but as `role="option"`, so they keep the standard gap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Express tooltip anchor gaps in rem for design patterns rule 7 The arrow/compact/follow-cursor gaps were raw px numbers. base-ui's positioner needs a px number, so they now resolve from a rem source (matching the file's existing spacingToPx idiom and the rem-sized arrow in styles.ts) — values are unchanged (15/8/4/10px at the 16px root). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Dismiss followCursor tooltip when the cursor roams far base-ui's cursor tracking only repositions the popup; it never hides when the pointer moves far from where the tooltip opened, so the popup lingered under a distant cursor. Restore the legacy behavior (parity with the removed use-tooltip-follow-cursor.ts): hide once the pointer moves past 50px from where the current move began, and reopen once it settles (250ms). Scoped strictly to followCursor via a dedicated hidden-ref that also keeps base-ui from re-opening mid-move; all other tooltips are untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Reserve tooltip gap as popup margin to fix Happo shadow crop Happo sizes a DOM capture to the union of element BORDER boxes and does not grow it for box-shadows. The migrated popup fitted tightly and was pushed off the anchor with sideOffset, so every non-dropdown snapshot cropped flush at the popup edge — ~13px short of the MUI baseline (the shadow band below the popup was lost). MUI instead reserved the gap as the popup's own `margin: 14px 0`, so its border box covered the gap band and the capture kept the shadow strip. Reproduce that: the popup carries the gap as a margin on the resolved-side axis (my-/mx-[0.875rem], compact m-1), and sideOffset supplies only the remainder (gap - margin) so the popup stays put — 0 for the standard arrow gap, negative for the tighter menu-item seat. Verified via Happo Cypress vs master: the 12 cropped variants now match pixel-perfect; only the intentional inside-dropdown seat and sub-pixel residuals remain. Also pin the Positioner anchor to the trigger's ref'd node (except followCursor): base-ui otherwise anchors whichever element's handler reported the open, so composite children (Radio/Checkbox: ref on the root label, props on the inner control) jumped by the two nodes' rect delta depending on interaction order. MUI always anchored the child's ref'd node. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Add @toptal/picasso-tailwind peer dependency to Tooltip The migrated Tooltip renders className='z-tooltip', a utility that only exists starting with the @toptal/picasso-tailwind minor this PR ships (the tooltip z-index token changeset). Declare that requirement as a peer (^4.1.0, caret style as Switch), and satisfy it locally with a workspace devDependency: autoInstallPeers would otherwise try to fetch the not-yet-published 4.1.0 from the registry and fail the install. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Restore legacy tooltip open/close semantics in useTooltipState Three related regressions of the hand-rolled controlled-open layer racing base-ui, fixed cohesively: - onOpen/onClose fired in handleOpenChange BEFORE the suppress/controlled gates decided whether to honor the request, so callbacks fired while the tooltip stayed closed — repeatedly, given base-ui's close/open churn. They now fire only after a request is honored, and only on a real closed-to-open or open-to-closed transition (legacy fired onOpen only past the ignoreOpening gate; base-ui's own hover timer no longer double-fires an already-open tooltip). The openRef updates eagerly so same-tick duplicate requests are deduped. - handleTriggerMouseLeave never lifted the click-dismiss suppression (only a 600ms timer that any open request cancelled did), so dismiss, leave, re-hover could stay stuck closed. Leaving the trigger now clears the suppression directly, as legacy handleMouseLeave cleared ignoreOpening — guarded by the trigger's bounding box, since a rounded trigger fires mouseleave while the pointer merely crosses its transparent corner pixels (verified by the Cypress boundary test). The timer machinery is gone. - Click-to-open is again a touch-only affordance: the legacy handleClick opened on click solely for touch devices, while desktop opening is owned by hover + openDelay; and followCursor never opens on touch, matching @material-ui@5 (the deleted isTouchDevice guards). Touch detection reuses isPointerDevice from @toptal/picasso-utils, as before the migration. New jest coverage: desktop click does not open, tap opens/closes on touch, followCursor stays closed on touch, and dismiss-leave-rehover reopens. Tests emit pointerdown before click - base-ui syncs closeOnClick from onPointerDown, which jsdom's bare fireEvent.click never dispatches. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Simplify tooltip sideOffset math where gap equals popup margin In getPositionerOffsets the no-arrow branch is only reachable for compact tooltips (followCursor returns earlier), so its gap and popup margin are both 4px and the subtraction was always 0; likewise the standard arrow gap was a pure alias of POPUP_MARGIN_PX, zeroing out against it. State that directly: compact and standard arrow tooltips sit flush (only the user offset remains) and a menu-item anchor shifts by its seat's difference from the margin band. No behavioral change - every sideOffset resolves to the same number as before, and the shadow-margin invariant is untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Document the fractional-rem tooltip arrow as a deliberate exception The 0.71rem arrow box violates the whole-pixel styling rule, but no whole-pixel construction can reproduce the required geometry: the 16px-base, 90-degree-apex triangle clipped from a 45-degree-rotated square needs a side of 8*sqrt(2) (~11.314px, irrational), and the MUI baseline itself rasterized at 0.71em = 11.36px - rounding moves the apex and breaks the pixel match. Border-triangle and clip-path shapes cannot carry the arrow's own box-shadow, and a drop-shadow filter would lose the shadow band in Happo captures (bbox grows for box-shadow only). Comment-only change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Restore MUI Grow open/close animation on the tooltip The migrated popup appeared and vanished instantly; the JSS Tooltip animated via TransitionComponent={Grow} (scale 0.75<->1 + fade, 200ms), growing out of the anchor. base-ui exposes the transition through the data-starting-style/data-ending-style attributes it sets on the popup and the anchor-facing --transform-origin, so the popup class-builder now drives the same Grow. Invisible to Happo: happoScreenshot waits for data-starting-style to clear, so captures are the settled frame — verified the Tooltip Cypress suite shows the same diff set (only the intentional inside-dropdown seat + known sub-pixel residuals), no new diffs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [PF-1994] Migrate Tooltip to @base-ui/react + Tailwind Tier 2 component. See PR description for prop-surface diff, import diff, and Happo summary. Refs: PF-1994 [review-iter 3] address review feedback * Route consumer id to Tooltip popup, not the trigger Legacy MUI put the id on the popper; base-ui spread it to the trigger via ...rest. Destructure id and pass it to Tooltip.Popup so it addresses the popup (role=tooltip) as before. base-ui assigns the trigger its own generated id. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Extract Tooltip positioner geometry into utils.ts Move splitPlacement, spacingToPxNumber, getPositionerOffsets and their gap constants out of Tooltip.tsx into a co-located utils.ts, with a focused unit test. Keeps them component-local for now; spacingToPxNumber and splitPlacement are generic enough to graduate to picasso-shared when a second base-ui positioned component needs them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Regroup Tooltip tests into when-context describe blocks Restructure test.tsx and utils.test.ts into the house nested style (describe subject -> describe 'when <condition>' -> it '<behavior>'). Behavior unchanged; snapshot keys follow the new describe paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Fix tap-to-open on disabled-element tooltips (touch path) A tap on a disabled control dispatches touch events but never a click (the HTML spec bars disabled form controls from click events), so the click-based tap-open path left 'tooltip on a disabled element' unopenable by touch — a regression vs legacy. Add a touchstart-based open path that reaches disabled anchors (mirroring the mouseover bubbling hover path), with a touchOpenedRef dedupe so the trailing synthetic click on normal elements doesn't toggle it straight back to click-to-dismiss. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Fix touch tap-open: scroll discrimination and reopen latch Two touch-interaction gaps in the tap-open path: - A scroll/swipe that starts on the trigger opened a stuck tooltip (touchstart opened immediately; roam-close is mouse-only). Now the tap arms on touchstart and opens on touchend only within tap slop; a touchmove past 10px disarms it as a scroll. - After a tap dismiss, suppressReopenRef was cleared only on mouseleave, which never fires on touch, so a fresh tap could not reopen. A tap that begins with the tooltip closed now lifts the latch (the touch analog of mouseleave). Disabled-element tap-open is preserved (touch events fire on disabled controls; only click is barred). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Describe the tooltip trigger with the popup id while open base-ui's Tooltip has no useRole, so it never sets aria-describedby on the trigger the way legacy MUI did. With id routed to Tooltip.Popup, set aria-describedby on the trigger to that id while open (falling back to any consumer-supplied value when closed or without an id), restoring the trigger-to-popup a11y link. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [PF-1994] Migrate Tooltip to @base-ui/react + Tailwind Tier 2 component. See PR description for prop-surface diff, import diff, and Happo summary. Refs: PF-1994 [cleanup] strip review-aid comments before merge --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
* Fix Modal/Drawer/Popper never rendering when open on first mount
usePicassoRoot() read rootRef.current during render, which is null on the
tree's first render pass and never re-read (a ref mutation triggers no
re-render). Modal, Drawer and Popper passed that null into Base UI /
floating-ui portals, which treat an explicit null container as 'wait for
the container to resolve' — so a mount-open overlay rendered nothing,
forever. The pre-migration MUI Portal fell back to document.body instead.
Two layers:
- Provider: mirror the root node into React state via a callback ref and
expose it through a dedicated PicassoRootNodeContext, so usePicassoRoot
consumers re-render and receive the real node once it mounts — without
re-rendering every RootContext consumer (titleCase etc.). rootRef stays
on RootContext for backward compatibility.
- Modal/Drawer/Popper: coerce an unresolved root to undefined instead of
null so the portal degrades to document.body (MUI-era behavior).
Drawer's disablePortal path keeps its intentional null (state-backed
inline mount node).
The stale PromptModal snapshot had the bug baked in (mount-open modal
captured as an empty Picasso-root div); Modal snapshot ids shifted from
the new test rendering earlier in the file.
* Resolve portal container fallback at the usePicassoRoot call sites
* Return undefined from usePicassoRoot instead of null
* Remove explanatory portal comment from Drawer
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
* Fix twMerge className prop position * Revert some changes * Fix tailwind short classes
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
* [PF-2245] Keep tooltip open when clicked during hover-open delay A click landing within the 200ms hover-open delay left the tooltip permanently suppressed: base-ui opens synchronously on the mousedown- focus, so the trailing click read that transient open and dismissed- then-latched it shut, and the latch only clears on mouseleave (which never fires while the cursor sits on the trigger). Every consumer Cypress spec that clicks a tooltip trigger failed as a result. Track whether the hover-open timer is still pending; a click in that window lets the hover win instead of dismissing. A click after the tooltip has deliberately opened still dismisses as before. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * [PF-2245] Simplify Tooltip open arbitration and drop redundant refs Follow-up cleanup on the same file, no behavior change (23/23 unit specs unchanged). base-ui's hover-open is rest-based, not the enter- based delay MUI/legacy Picasso used, so Picasso runs its own hover timer regardless. Vetoing base-ui's `trigger-hover` opens in handleOpenChange makes Picasso the sole hover-open authority instead of racing two differently-timed machines; base-ui keeps focus-open and all dismissals. Drop three redundant refs (11 -> 8): - hoverOpenPendingRef: identical in lifetime to "openTimerRef has a pending timer"; the callback and every cancel path now null the timer id, so `openTimerRef.current !== undefined` is the pending signal the PF-2245 click guard reads. - targetHoveredRef: mouseleave always clears the settle timer, so the reopen callback only fires while hovering -- the guard was always true. - lastMoveEventRef: the settle timer is re-armed on every mousemove, so the callback's own closed-over event is the last move event. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
* [PF-1994] Tweak cypress transitions and wait logic * [PF-1994] Cover more tests with wait logic * [PF-1994] Tweak rest of cypress tests * [PF-1994] Revert skipped tests * [PF-1994] Tweak datepicker test * [PF-1994] tweak fonts showing correctly test
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
* [PF-2243] Fix Modal duplicate role attribute * [PF-2243] Revert paper role overwrite
Contributor
Picasso design patterns check failed
|
Contributor
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
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
4 participants
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.
[FX-NNNN]
Description
Describe the changes and motivations for the pull request.
How to test
Screenshots
Development checks
picasso-tailwind-mergerequires major update (check itsREADME.md)propsin component with documentationexamplesfor componentBreaking change
Alpha packages
Manually trigger the publish.yml workflow to publish alpha packages. Specify pull request number as a parameter (only digits, e.g.
123).PR Review Guidelines
When to approve? ✅
You are OK with merging this PR and
nit:to your comment. (ex.nit: I'd rename this variable from makeCircle to getCircle)When to request changes? ❌
You are not OK with merging this PR because
When to comment (neither ✅ nor ❌)
You want your comments to be addressed before merging this PR in cases like:
How to handle the comments?