Skip to content

pf 2049 slider migration#4959

Draft
narghev wants to merge 12 commits into
feature/picasso-modernizationfrom
pf-2049-slider-migration
Draft

pf 2049 slider migration#4959
narghev wants to merge 12 commits into
feature/picasso-modernizationfrom
pf-2049-slider-migration

Conversation

@narghev
Copy link
Copy Markdown
Member

@narghev narghev commented May 14, 2026

  • Migrate slider to base-ui
  • Fix Slider vertical alignment after Base UI migration

[FX-NNNN]

Description

Describe the changes and motivations for the pull request.

How to test

  • Temploy
  • FIXME: Add the steps describing how to verify your changes

Screenshots

Before. After.
Insert screenshots or screen recordings Insert screenshots or screen recordings

Development checks

  • Add changeset according to guidelines (if needed)
  • Double check if picasso-tailwind-merge requires major update (check its README.md)
  • Read CONTRIBUTING.md and Component API principles
  • Make sure that additions and changes on the design follow Toptal's BASE design, and it's been already discussed with designers at #-base-core
  • Annotate all props in component with documentation
  • Create examples for component
  • Ensure that deployed demo has expected results and good examples
  • Ensure the changed/created components have not caused accessibility issues. How to use accessibility plugin in storybook.
  • Self reviewed
  • Covered with tests (visual tests included)

Breaking change

  • codemod is created and showcased in the changeset
  • test alpha package of Picasso in StaffPortal

All development checks should be done and set checked to pass the
GitHub Bot: TODOLess action

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

  1. You have no extra requests.
  2. You have optional requests.
    1. Add 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

  1. Something is broken after the changes.
  2. Acceptance criteria is not reached.
  3. Code is dirty.

When to comment (neither ✅ nor ❌)

You want your comments to be addressed before merging this PR in cases like:

  1. There are leftovers like unnecessary logs, comments, etc.
  2. You have an opinionated comment regarding the code that requires a discussion.
  3. You have questions.

How to handle the comments?

  1. An owner of a comment is the only one who can resolve it.
  2. An owner of a comment must resolve it when it's addressed.
  3. A PR owner must reply with ✅ when a comment is addressed.

narghev and others added 2 commits May 13, 2026 10:43
Align rail and thumb with MUI Base baseline:
- Remove top-1/2 -translate-y-1/2 from Slider.Track so it sits at top
  of Slider.Control instead of being centered (matches MUI Base's
  position:absolute top:0 rail)
- Add -mt-[5.5px] to thumb to counter Base UI's hardcoded top:50% +
  translate -50% inline styles, restoring the rail-centered thumb
  position that MUI Base achieved via -mt-[7px]

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@narghev narghev requested a review from a team as a code owner May 14, 2026 12:45
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 14, 2026

⚠️ No Changeset found

Latest commit: 31118cd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Copy Markdown
Contributor

📖 Storybook Preview

🚀 Your Storybook preview is ready: View Storybook

📍 Preview URL: https://toptal.github.io/picasso/prs/4959/

This preview is updated automatically when you push changes to this PR.

github-actions Bot added a commit that referenced this pull request May 14, 2026
Previous fix over-corrected by 5.5px causing the widget to render too
high. MUI Base's rail used top:auto resolving to the static position at
the top of Root's content area (after py-[6px]) — y=0 outer, center
y=0.5 — not the top of the padding-box.

- Slider.Track: top-1/2 (no -translate-y-1/2) places the 1px rail at
  y=0..1 of outer wrapper, center y=0.5
- Slider.Thumb: mt-[0.5px] shifts thumb center from y=0 to y=0.5 to
  align with the rail

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

📖 Storybook Preview

🚀 Your Storybook preview is ready: View Storybook

📍 Preview URL: https://toptal.github.io/picasso/prs/4959/

This preview is updated automatically when you push changes to this PR.

github-actions Bot added a commit that referenced this pull request May 14, 2026
Browsers round mt-[0.5px] to 1px at 1x DPI, dropping the thumb 0.5px
below the rail. Replace the subpixel margin with a 13px-tall
Slider.Control: top:50% of 13 = 6.5px (no rounding), so the thumb's
math-defined center now lands at y=0.5 outer using only integer
pixels.

- Slider.Control: h-[13px] (was inset-0 → 12px). Click area grows by
  1px; Control overflows Root's padding-box by 1px on the bottom.
- Slider.Thumb: remove mt-[0.5px], rely on Base UI's inline
  top:50% + translate(-50%, -50%)
- Slider.Track: top-1/2 -translate-y-1/2 (back from top-1/2 only) so
  the 1px rail centers at the same y=0.5

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
github-actions Bot added a commit that referenced this pull request May 14, 2026
@github-actions
Copy link
Copy Markdown
Contributor

📖 Storybook Preview

🚀 Your Storybook preview is ready: View Storybook

📍 Preview URL: https://toptal.github.io/picasso/prs/4959/

This preview is updated automatically when you push changes to this PR.

narghev and others added 3 commits May 22, 2026 18:06
Base UI's Slider.Thumb sets translate(-50%, -50%) which creates a
containing block for fixed-positioned descendants. The hidden
<input type="range"> inside the thumb uses position:fixed +
clip-path:inset(50%), so under that containing block its visible
focus outline lands around the thumb itself. Happo's
applyPseudoClasses: true activates :focus, surfacing the diff.

Add [&_input]:outline-none to the thumb class so the nested input
shows no browser focus ring, matching MUI Base's render where the
ring wasn't visible.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Tailwind 4 does not recognize bg-gray-500/[0.24] (slash-bracket-
decimal) and silently drops the class, causing the rail to render at
a different alpha than intended. Picasso is on tailwindcss 4.2.1.

Switch to bg-gray-500/24 (slash-integer-percentage), which is the
supported Tailwind 4 syntax and generates the correct color-mix
expression to render the rail at the same #F1F1F2 as the MUI Base
baseline on a white background.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Pixel analysis of the Happo diff showed the rail rendered across 2
pixels at half-alpha (anti-aliased) vs MUI Base's single 1px rail.
Cause: Base UI's Slider.Thumb hard-sets translate(-50%, -50%) inline,
and the prior fix used h-[13px] Control + top-1/2 + -translate-y-1/2
on Track to compensate, all producing sub-pixel translations.

Restore MUI Base's integer-pixel positioning:
- Slider.Control: back to inset-0 (12px tall, even — 50% lands on
  integer pixel)
- Slider.Track: top-1/2 only (no -translate-y-1/2), rail at y=0..1
  outer, single solid pixel
- Slider.Thumb: -mt-[7px] -ml-[6px] (same offsets MUI Base used),
  plus !translate-none to override Base UI's inline translate so
  the thumb is integer-positioned at center y=0.5 outer

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

📖 Storybook Preview

🚀 Your Storybook preview is ready: View Storybook

📍 Preview URL: https://toptal.github.io/picasso/prs/4959/

This preview is updated automatically when you push changes to this PR.

github-actions Bot added a commit that referenced this pull request May 22, 2026
For stories where the thumb's value coincides with a mark position
(Marks story: value=10, mark at value=10; DisableTrackHighlight:
defaultValue=20, mark at value=20), the mark was rendering on top of
the thumb because marks came after thumbs in the JSX. MUI Base put
the thumb above marks.

Swap the order so marks render first, thumbs second — DOM order
gives the thumb the higher paint order at equal z-index.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

📖 Storybook Preview

🚀 Your Storybook preview is ready: View Storybook

📍 Preview URL: https://toptal.github.io/picasso/prs/4959/

This preview is updated automatically when you push changes to this PR.

github-actions Bot added a commit that referenced this pull request May 22, 2026
Base UI's Slider.Thumb renders a visually-hidden <input type="range">
with position:fixed, width:100%, height:100% — covering the entire
viewport. With Base UI's translate(-50%, -50%) on the thumb that
input would be contained by the thumb (translate creates a CB for
fixed descendants), but my !translate-none override breaks that
containment.

Cypress's `cy.get('body').happoScreenshot()` then captures a much
larger image (viewport-sized vs slider-sized) and the
range/when-tooltip-intersect comparison fails purely on differing
image dimensions, not visual content.

Force [&_input]:!absolute so the input is contained by the thumb
(thumb is position:absolute → CB for absolute descendants). The
input stays scoped to the 15x15 thumb box, body sizing matches MUI
Base baseline.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

📖 Storybook Preview

🚀 Your Storybook preview is ready: View Storybook

📍 Preview URL: https://toptal.github.io/picasso/prs/4959/

This preview is updated automatically when you push changes to this PR.

github-actions Bot added a commit that referenced this pull request May 22, 2026
narghev and others added 3 commits May 22, 2026 20:18
The thumb needs to be a CSS containing block for fixed-positioned
descendants so Base UI's <input type="range"> (position:fixed,
width/height:100%) stays scoped to the thumb's 15×15 box instead of
sizing to the viewport (which extends Cypress's body-screenshot
dimensions to 1280×1023 vs the baseline's 1280×60).

Previous !translate-none set translate:none (initial value, no CB
established). Replace with !translate-x-0 !translate-y-0 which sets
translate:0 0 — a non-initial value that establishes a CB without
visually shifting the thumb (positioning still comes from -mt-[7px]
-ml-[6px]).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
translate: 0 0 (previous fix) creates a containing block for the
hidden range input but triggers GPU compositing, which produces a
4-pixel sub-pixel-rendering diff at the thumb edge in the
Disable-track-highlight story.

Switch to contain: layout (creates the CB without compositor
optimizations) + !translate-none (clears Base UI's translate -50%
-50% so the thumb stays integer-positioned via -mt/-ml margins).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

📖 Storybook Preview

🚀 Your Storybook preview is ready: View Storybook

📍 Preview URL: https://toptal.github.io/picasso/prs/4959/

This preview is updated automatically when you push changes to this PR.

github-actions Bot added a commit that referenced this pull request May 22, 2026
@narghev narghev marked this pull request as draft May 22, 2026 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant