Migrate Slider by denieler · Pull Request #4986 · toptal/picasso

denieler · 2026-05-29T14:33:00Z

[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

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

You have no extra requests.
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

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

When to comment (neither ✅ nor ❌)

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

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

How to handle the comments?

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

changeset-bot · 2026-05-29T14:33:07Z

⚠️ No Changeset found

Latest commit: 54fb5a2

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 · 2026-05-29T14:58:29Z

📖 Storybook Preview

🚀 Your Storybook preview is ready: View Storybook

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

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

github-actions · 2026-05-29T15:59:09Z

📖 Storybook Preview

🚀 Your Storybook preview is ready: View Storybook

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

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

github-actions · 2026-05-29T19:57:24Z

📖 Storybook Preview

🚀 Your Storybook preview is ready: View Storybook

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

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

github-actions · 2026-05-29T20:37:13Z

Picasso design patterns check failed

Rule	Status
1 — Optimize defaults for the common case	✅
2 — Reuse prop names across components	✅
3 — Keep prop names short and simple	✅
4 — Mirror native HTML prop names	✅
5 — Style overrides only via `className` or `style`	✅
6 — Prefer `children` over content props	✅
7 — Use `rem` for all sizes	❌ `railStyle` uses `top: '6px'`; `thumbStyle` uses `top: '-7px'` and `marginLeft: '-6px'` — px not rem
10 — Extend `BaseProps`	✅
14 — No `is` prefix on boolean props	❌ `isOverlaped` in exported `SliderValueLabelProps` uses the `is` prefix

github-actions · 2026-05-29T20:52:01Z

📖 Storybook Preview

🚀 Your Storybook preview is ready: View Storybook

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

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

vedrani

There are also good things in this PR: alpha trick, solving position, better BASE UI structure, no owner state.

I plan to update my PR based on it and also address data-state and uncontrolled I commented here.

This PR needs to address design fixes regarding color and thumb and mark size.

vedrani · 2026-06-02T08:21:02Z

+
+  // We mirror the value internally so that marks and value labels can reflect
+  // the live value even when the component is used in an uncontrolled way.
+  const [uncontrolledValue, setUncontrolledValue] = useState<number | number[]>(


It's mirroring state. I understand it's because of uncontrolled way.

Btw did we supported uncontrolled way before migration?

I suggest we use BASE ui state as single point of truth.

@vedrani I don't think we changed how component behaves, we keep the exactly the same Props 🤷‍♂️

vedrani · 2026-06-02T08:24:19Z

  const containerRef = useRef<HTMLDivElement>(null)
-  const sliderRef = useCombinedRefs<HTMLElement>(ref, useRef<HTMLElement>(null))
+
+  const setRootRef = useCallback(


Why not useCombinedRefs, it's not following convention.

@vedrani does it do the same what useCombinedRefs do? I don't think so 🤔

vedrani · 2026-06-02T11:52:43Z

+            {marksData.map(mark => (
+              <SliderMark
+                key={mark.value}
+                data-index={mark.value}


I think it should be mark index, not value.

agree, but to me seems like "nit", we even didn't have that before 😃

Migrate Slider

4bd77fc

denieler self-assigned this May 29, 2026

denieler added no-jira claude labels May 29, 2026

denieler added 2 commits May 29, 2026 16:35

Fix pnpm lock

a4aee2a

Change base ui from rc

330487c

github-actions Bot added a commit that referenced this pull request May 29, 2026

Sync: PR #4986 Preview

5f3b4ff

github-actions Bot added a commit that referenced this pull request May 29, 2026

Sync: PR #4986 Preview

8ba48af

Add inline styles

c7a2fff

denieler force-pushed the bill-migrate-slider branch from efb842a to c7a2fff Compare May 29, 2026 19:39

github-actions Bot added a commit that referenced this pull request May 29, 2026

Sync: PR #4986 Preview

90902b6

Fix half pixel

54fb5a2

github-actions Bot added a commit that referenced this pull request May 29, 2026

Sync: PR #4986 Preview

0c2dce0

vedrani reviewed Jun 2, 2026

View reviewed changes

Conversation

denieler commented May 29, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How to test

Screenshots

Development checks

When to approve? ✅

When to request changes? ❌

When to comment (neither ✅ nor ❌)

How to handle the comments?

Uh oh!

changeset-bot Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

github-actions Bot commented May 29, 2026

Picasso design patterns check failed

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

vedrani left a comment

Choose a reason for hiding this comment

Uh oh!

vedrani Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

denieler Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

vedrani Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

denieler Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

vedrani Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

denieler Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

denieler commented May 29, 2026 •

edited by github-actions Bot

Loading

changeset-bot Bot commented May 29, 2026 •

edited

Loading