Skip to content
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

misc(Table): migrate to Tailwind #1904

Merged
merged 1 commit into from
Dec 4, 2024
Merged

misc(Table): migrate to Tailwind #1904

merged 1 commit into from
Dec 4, 2024

Conversation

ansmonjol
Copy link
Collaborator

Context

As part of our ongoing journey to migrate from styled components to tailwind, the Table component also needed refactoring.

Description

Few points to note about this migration:

  • All styles components are now JSX components written on top of Table definition, in the same file
  • Styles are defined in 3 separated manner
    • className for "default" and toggled styles
    • styles for "dynamic" styles mapped on variables
    • sx for "complex" selector (specific classes, pseudo elements, ...)
  • Component class names lago-* are not in variables. Even tho they are used multiple times, I had to prevent using variables because we cannot use dynamic classe names in sx selector. Those are not interpreted during run time but build and "transformed" as static css on the flight. Even with unchanged const it don't work.
  • Kept usage of setResponsiveProperty and added a test for it. Also even tho this util is only used in Table today I preferred to keep it in it's separated file. It now return an object and not a component css object anymore.
  • rewrote the Box wrapper with a div & classNames. Note the transform was useless and removed.
  • You can notice a custom captionHl defined in a style of TableInnerCell. I choose not to turn that into a TW className that could help, but rather keep it like this and maybe migrate later if sure reproduce too often.

There should be no change in UI and UX

Fixes LAGO-422

Copy link
Collaborator

@keellyp keellyp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick suggestions but overall LGTM 👏

src/components/designSystem/Table/Table.tsx Outdated Show resolved Hide resolved
src/components/designSystem/Table/Table.tsx Outdated Show resolved Hide resolved
@ansmonjol ansmonjol force-pushed the table-tw branch 2 times, most recently from 2b68b23 to 4332541 Compare December 4, 2024 15:32
@ansmonjol ansmonjol enabled auto-merge (squash) December 4, 2024 15:32
@ansmonjol ansmonjol merged commit a0b2b86 into main Dec 4, 2024
4 checks passed
@ansmonjol ansmonjol deleted the table-tw branch December 4, 2024 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants