[FX-4601] Migrate Slider to TailwindCSS #4272

AdrianContiu · 2024-04-30T10:28:28Z

FX-4601

Description

This PR changes the Slider component to use the Slider from the MUI Base UI and applies styles using TailwindCSS.

Notes:

please note that compact and TooltipComponent props have been removed due to unnecessary extra work that doesn't match the designs. This was agreed between Tomas and Matt Vela.
currently, the props section for the Slider story is no longer displayed. Tomas said it's a known issue but needs debugging as there isn't always a similar pattern for this behavior and he will address it upon his return.

How to test

Temploy
Check the production and compare it with the temploy.

Screenshots

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

Development checks

Change the base branch to feature branch
Add changeset according to guidelines (if needed)
~~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

PR commands

List of available commands:

@toptal-bot run package:alpha-release - Release alpha version
@toptal-anvil ping reviewers - Ping FX team for review

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 · 2024-04-30T10:28:32Z

🦋 Changeset detected

Latest commit: 2677289

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@toptal/picasso-slider	Major
@toptal/picasso	Patch

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

augustobmoura

Left a few comments, but overral nice work!

lerna-debug.log

.changeset/mean-waves-help.md

packages/base/Slider/src/Slider/Slider.tsx

packages/base/Slider/src/Slider/story/ControlledWithLabel.example.tsx

packages/base/Slider/src/Slider/story/Tooltip.example.tsx

dmaklygin

Good work! Left some comments

packages/base/Slider/src/Slider/Slider.tsx

packages/base/Slider/src/Slider/__snapshots__/test.tsx.snap

packages/base/Slider/src/Slider/story/ControlledWithLabel.example.tsx

.changeset/perfect-apes-trade.md

packages/base/Slider/src/Slider/Slider.tsx

.changeset/mean-waves-help.md

packages/base/Slider/src/Slider/Slider.tsx

packages/base/Slider/src/Slider/SliderContext.tsx

TomasSlama · 2024-05-14T09:38:47Z

@toptal-anvil ping reviewers

packages/base/Slider/package.json

packages/base/Slider/src/SliderValueLabel/SliderValueLabel.tsx

packages/base/Slider/src/utils/check-overlap.ts

packages/base/Slider/src/Slider/hooks/use-label-overlap.ts

packages/base/Slider/src/SliderValueLabel/SliderValueLabel.tsx

packages/base/Slider/src/Slider/hooks/use-label-overlap.ts

TomasSlama · 2024-05-14T13:38:49Z

@toptal-anvil ping reviewers

dmaklygin · 2024-05-14T13:48:36Z

packages/base/Slider/src/utils/get-x-placement.ts

+  isOverlaped,
+  isFirstLabel,
+  currentPlacement,
+}: GetPositionProps) => {


since, it's a separate function, I would also define a return type.

packages/base/Slider/src/utils/check-overlap.ts

packages/base/Slider/src/Slider/hooks/use-label-overlap.ts

packages/base/Slider/src/utils/check-overlap.test.ts

* Add the Slider from MUIBase * Refactor components to accommodate migration * Adapt stories and remove unused files * Fix a small existing bug on Tooltip Story * Update snapshots * Add changeset * Update rail slot to fix Happo Story failed test * Update snapshots after changes on rail slot * Address part of review comments * Revert back ValueLabel to a span * Test commit * Update yarn * Revert "Revert back ValueLabel to a span" * Change ValueLabel to a span after revert * Update snapshots * Refactor ValueLabel * Move slots components outside the Slider * Add unit tests for getPosition util function * Refactor SliderValueLabel * Refactor hook * Refactor hook and util function * Add unit tests for hooks * Add changeset file * Update changeset * Change position of Label when out of screen * Fix doc generation * Update yarn.lock * Remove logs * Fix spacing * Update snapshots * Update snapshots * Address comments * Rename property to cleaner code * Fix tests * Refactor ValueLabel * Fix tests * Refactor logic to hook * Address comments * Address comments --------- Co-authored-by: TomasSlama <tomas.slama@toptal.com>

AdrianContiu self-assigned this Apr 30, 2024

AdrianContiu changed the title ~~[FX 4601] Migrate Slider to TailwindCSS~~ [FX-4601] Migrate Slider to TailwindCSS Apr 30, 2024

AdrianContiu marked this pull request as ready for review April 30, 2024 11:49

AdrianContiu requested a review from a team as a code owner April 30, 2024 11:49

augustobmoura reviewed May 2, 2024

View reviewed changes

dmaklygin reviewed May 2, 2024

View reviewed changes

AdrianContiu force-pushed the fx-4601-migrate-slider branch from 88d5875 to 1b60b0e Compare May 3, 2024 11:54

TomasSlama force-pushed the fx-4601-migrate-slider branch from 1306bbc to a9b1c47 Compare May 7, 2024 13:56

AdrianContiu force-pushed the fx-4601-migrate-slider branch from 9ee3cff to 18a9600 Compare May 9, 2024 14:15

AdrianContiu commented May 9, 2024

View reviewed changes

TomasSlama force-pushed the fx-4601-migrate-slider branch from 9ed21ad to 7c9a7d9 Compare May 13, 2024 08:59

AdrianContiu and others added 18 commits May 13, 2024 13:19

Add the Slider from MUIBase

791ded1

Refactor components to accommodate migration

a329da6

Adapt stories and remove unused files

0fb0be1

Fix a small existing bug on Tooltip Story

4cf2051

Update snapshots

c6c0eb8

Add changeset

1986d4e

Update rail slot to fix Happo Story failed test

044bea1

Update snapshots after changes on rail slot

2e99b42

Address part of review comments

6a3adb0

Revert back ValueLabel to a span

6a6d907

Test commit

c173a7b

Update yarn

8a46a88

Revert "Revert back ValueLabel to a span"

2a81fe8

Change ValueLabel to a span after revert

8786dd4

Update snapshots

667e511

Refactor ValueLabel

9ae5894

Move slots components outside the Slider

9db6e09

Add unit tests for getPosition util function

446d76b

TomasSlama added 3 commits May 14, 2024 11:04

Refactor ValueLabel

4e6fb6b

Fix tests

9aa2f26

Refactor logic to hook

5ec2217

TomasSlama requested review from dmaklygin, augustobmoura and OleksandrNechai May 14, 2024 09:33

dmaklygin reviewed May 14, 2024

View reviewed changes

packages/base/Slider/package.json Show resolved Hide resolved

dmaklygin reviewed May 14, 2024

View reviewed changes

packages/base/Slider/src/SliderValueLabel/SliderValueLabel.tsx Show resolved Hide resolved

dmaklygin reviewed May 14, 2024

View reviewed changes

packages/base/Slider/src/SliderValueLabel/SliderValueLabel.tsx Show resolved Hide resolved

OleksandrNechai reviewed May 14, 2024

View reviewed changes

packages/base/Slider/src/utils/check-overlap.ts Outdated Show resolved Hide resolved

OleksandrNechai reviewed May 14, 2024

View reviewed changes

packages/base/Slider/src/utils/check-overlap.ts Show resolved Hide resolved

dmaklygin reviewed May 14, 2024

View reviewed changes

packages/base/Slider/src/Slider/hooks/use-label-overlap.ts Outdated Show resolved Hide resolved

OleksandrNechai reviewed May 14, 2024

View reviewed changes

packages/base/Slider/src/SliderValueLabel/SliderValueLabel.tsx Show resolved Hide resolved

OleksandrNechai reviewed May 14, 2024

View reviewed changes

packages/base/Slider/src/Slider/hooks/use-label-overlap.ts Outdated Show resolved Hide resolved

OleksandrNechai approved these changes May 14, 2024

View reviewed changes

Address comments

c30f59d

TomasSlama requested a review from dmaklygin May 14, 2024 13:38

dmaklygin reviewed May 14, 2024

View reviewed changes

packages/base/Slider/src/utils/check-overlap.ts Outdated Show resolved Hide resolved

dmaklygin reviewed May 14, 2024

View reviewed changes

packages/base/Slider/src/Slider/hooks/use-label-overlap.ts Show resolved Hide resolved

dmaklygin reviewed May 14, 2024

View reviewed changes

packages/base/Slider/src/utils/check-overlap.test.ts Outdated Show resolved Hide resolved

dmaklygin approved these changes May 14, 2024

View reviewed changes

TomasSlama changed the base branch from master to feature/tailwind-w20 May 14, 2024 14:17

TomasSlama added 2 commits May 14, 2024 16:34

Address comments

4776888

Merge branch 'feature/tailwind-w20' into fx-4601-migrate-slider

2677289

TomasSlama merged commit a0bdab2 into feature/tailwind-w20 May 14, 2024
13 of 14 checks passed

TomasSlama deleted the fx-4601-migrate-slider branch May 14, 2024 18:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FX-4601] Migrate Slider to TailwindCSS #4272

[FX-4601] Migrate Slider to TailwindCSS #4272

AdrianContiu commented Apr 30, 2024 •

edited by TomasSlama

changeset-bot bot commented Apr 30, 2024 •

edited

augustobmoura left a comment

dmaklygin left a comment

TomasSlama commented May 14, 2024

TomasSlama commented May 14, 2024

dmaklygin May 14, 2024

TomasSlama May 14, 2024

[FX-4601] Migrate Slider to TailwindCSS #4272

[FX-4601] Migrate Slider to TailwindCSS #4272

Conversation

AdrianContiu commented Apr 30, 2024 • edited by TomasSlama

Description

How to test

Screenshots

Development checks

When to approve? ✅

When to request changes? ❌

When to comment (neither ✅ nor ❌)

How to handle the comments?

changeset-bot bot commented Apr 30, 2024 • edited

🦋 Changeset detected

augustobmoura left a comment

Choose a reason for hiding this comment

dmaklygin left a comment

Choose a reason for hiding this comment

TomasSlama commented May 14, 2024

TomasSlama commented May 14, 2024

dmaklygin May 14, 2024

Choose a reason for hiding this comment

TomasSlama May 14, 2024

Choose a reason for hiding this comment

AdrianContiu commented Apr 30, 2024 •

edited by TomasSlama

changeset-bot bot commented Apr 30, 2024 •

edited