-
Notifications
You must be signed in to change notification settings - Fork 151
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
Migrate Tile to Tailwind #4101
Migrate Tile to Tailwind #4101
Conversation
Size Change: -1.04 kB (0%) Total Size: 472 kB
ℹ️ View Unchanged
|
Deploying with Cloudflare Pages
|
fa8173d
to
cf5d184
Compare
ef9b71c
to
8137f4d
Compare
packages/orbit-components/src/Tile/components/TileWrapper/index.tsx
Outdated
Show resolved
Hide resolved
packages/orbit-components/src/Tile/components/TileWrapper/index.tsx
Outdated
Show resolved
Hide resolved
packages/orbit-components/src/Tile/components/TileHeader/index.tsx
Outdated
Show resolved
Hide resolved
Also, unit tests are failing on this components, so please address that 🙏 |
8137f4d
to
c148ff7
Compare
082f459
to
d41e9a6
Compare
packages/orbit-components/src/Tile/components/TileWrapper/index.tsx
Outdated
Show resolved
Hide resolved
packages/orbit-components/src/Tile/components/TileWrapper/index.tsx
Outdated
Show resolved
Hide resolved
e8bbcfa
to
c354b8b
Compare
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.
Just 💅🏻 with cx instead of clsx, for consistency
@@ -1,16 +1,12 @@ | |||
import * as React from "react"; | |||
import styled, { css } from "styled-components"; | |||
import clsx from "clsx"; |
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.
import clsx from "clsx"; | |
import cx from "clsx"; |
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.
adjusted
)} | ||
</Stack> | ||
))} | ||
{!noHeaderIcon && ( | ||
<StyledIconRight external={external} expandable={expandable} expanded={expanded} /> | ||
<IconRight | ||
className={clsx( |
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.
className={clsx( | |
className={cx( |
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.
adjusted
|
||
import mq from "../../../utils/mediaQuery"; | ||
import defaultTheme from "../../../defaultTheme"; | ||
import clsx from "clsx"; |
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.
just 💅🏻 , we use it everywhere as import cx from "clsx" :)
import clsx from "clsx"; | |
import cx from "clsx"; |
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.
adjusted
withPointer={withPointer} | ||
withBorder={withBorder} | ||
useMargins={useMargins} | ||
className={clsx( |
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.
className={clsx( | |
className={cx( |
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.
adjusted
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.
Non-blocking, but I would say we don't need separate commit messages for sub-components, since they are not exported. Ultimately, what's relevant for the user reading the changelog notes is that the Tile component (the only one they have access to) is migrated to Tailwind. But it's a detail, so LGTM 👍
5382d13
to
51d864d
Compare
Did you run chromatic post-migration? |
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.
LGTM, good job 👍🏻
51d864d
to
9bbbec0
Compare
https://kiwicom.atlassian.net/browse/FEPLT-1701
Storybook: https://orbit-mainframev-oreqizer-tw-tile.surge.sh
Chromatic visual test: https://www.chromatic.com/build?appId=6523bafdbc6087c220b3cc48&number=148