-
Notifications
You must be signed in to change notification settings - Fork 16
[FF-7] Migrate Tabs to Tailwind #4874
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: bebf01b The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
|
🎉 Alpha packages are ready! PR: #4874 Installation commands: These alpha packages were built from the latest commit in this PR. |
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
denys-medynskyi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
packages/base/Tabs/src/Tabs/Tabs.tsx
Outdated
| onChange as ( | ||
| event: React.ChangeEvent<{}> | null, | ||
| value: TabsValueType | ||
| ) => void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this type casting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me see if I can find a workaround I just wanted to fix it so that I can test visually so I didn't look into it further
|
by some reason storybook link is not available 😞 @rocodesign can you please regenerate it? 🙇 |
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
FF-7
Description
Migrate Tabs to Tailwind
How to test
can be tested on staff-portal in this PR
https://github.com/toptal/staff-portal/pull/15126
Screenshots
There's a single slight change from master due to positioning of the icons that was off
https://happo.io/a/431/p/1194/compare/master-2025-12-10/after-changes5
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?