[FX-4620] Migrate Switch component #4302
Conversation
🦋 Changeset detectedLatest commit: c15232e The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
|
@toptal-anvil ping reviewers |
| @@ -60,9 +58,38 @@ export const Switch = forwardRef<HTMLButtonElement, Props>(function Switch( | |||
| style={style} | |||
| disabled={disabled} | |||
| id={id} | |||
| onChange={onChange} | |||
| value={value} | |||
| onChange={onChangeCallback} | |||
There was a problem hiding this comment.
Why this change is needed?
There was a problem hiding this comment.
As I said, MUI/Base Switch doesn't expect value, and expecting value in the Picasso's Swich is also odd.
It should keep only state, nothing more.
value was added unintentionally to support consistency in Field elements, so, better to remove it.
There was a problem hiding this comment.
I am asking about onChange={onChange} why we had to change it
| @@ -118,6 +118,7 @@ module.exports = { | |||
| white: '#FFFFFF', | |||
| black: '#000000', | |||
| transparent: 'transparent', | |||
| current: 'currentColor', | |||
There was a problem hiding this comment.
Could you please explain this change?
There was a problem hiding this comment.
this approach was migrated from the previous implementation:
we applied it to Switch: text-white bg-current: it means, we say that background-color will be the same as the text color -> white.
The default TailwindCSS configuration contains such property:
There was a problem hiding this comment.
yeah, that is what was confusing to me. Default config of tailwind has this bg-current. Why did we have to add our own?
|
The previous version has a bug due to hidden input: This was a reason why happo tests were fallen |
* Migrate Switch component * Update changeset * Update changeset * Update changeset * Update picasso * Change changeset * Remove old dependency
FX-4620
Description
Migrate Switch component to Mui/base Switch and TailwindCSS.
How to test
Video recording
This is a comparison between Temploy and production:
Screen.Recording.2024-05-14.at.10.35.37.mov
Accessibility issues.
No new accessibility issues. Current issues are related to disabled uncontrolled Switch. The same as on production.
Development checks
propsin component with documentationexamplesfor componentBreaking change
PR commands
List of available commands:
@toptal-bot run package:alpha-release- Release alpha version@toptal-anvil ping reviewers- Ping FX team for reviewPR 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?