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

[FE] Prep work - Tailwind + shadcn cross integration in LSO + LSE #6870

Open
wants to merge 23 commits into
base: develop
Choose a base branch
from

Conversation

nick-skriabin
Copy link
Member

@nick-skriabin nick-skriabin commented Jan 8, 2025

This PR is the first step for Tailwind and Shadcn adoption. It doesn't bring any visual changes at the moment and acts as a foundations for the future work.

Copy link

netlify bot commented Jan 8, 2025

Deploy Preview for label-studio-docs-new-theme canceled.

Name Link
🔨 Latest commit df8ba6e
🔍 Latest deploy log https://app.netlify.com/sites/label-studio-docs-new-theme/deploys/6786809e288fd900087347c3

Copy link

netlify bot commented Jan 8, 2025

Deploy Preview for heartex-docs canceled.

Name Link
🔨 Latest commit df8ba6e
🔍 Latest deploy log https://app.netlify.com/sites/heartex-docs/deploys/6786809ed650ce0008598f9f

Dockerfile.development Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

This web/dist dir we discussed earlier in FE Guild that it seems like the gitignore is incorrectly handling the dist directories. Just making a note of that.

Copy link
Member Author

Choose a reason for hiding this comment

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

removed and ignored then

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a big proponent of colocating stories files with the component, and leaving storybook to just be the configuration and builder of all the stories across the entire monorepo. I could be convinced this is better, but the one thing I think is a particularly nice aspect of the colocation is that all the NX generators default to this, so scaffolding components can be a very nice way to enforce standards as well as make creating components easier for all. Just a thought.

Copy link
Member Author

Choose a reason for hiding this comment

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

makes sense. though, we need to set up components for that first (comment below)

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it required to have the path exist as web/libs/ui/src/shad/components/ui/select.tsx for all of these components?

Ideally we would want the code to be organized more like: web/libs/ui/src/components/{component}/{component}.tsx and export the exportable pieces from an index.ts, and colocate other files like tests and stories here.

Copy link
Member Author

Choose a reason for hiding this comment

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

this is how shadcn installs components. gotta take a look if we can change that

@@ -195,3 +195,4 @@
}
}
}
<svg width='9'height='5'viewBox='0095'fill='none'xmlns='http://www.w3.org/2000/svg'><path d='M11C10.7238581.223860.51.50.5H7.5C7.776140.580.72385881V3.21922C83.448667.843853.648657.621273.70429L4.621274.4543C4.541654.47424.458354.47424.378734.45429L1.378733.70429L1.257464.18937L1.378733.70429C1.156153.6486513.4486613.21922V1Z'fill='#40A9FF'stroke='#1F1F1F'/></svg>
Copy link
Contributor

Choose a reason for hiding this comment

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

was adding this svg here intended?

Copy link
Member Author

Choose a reason for hiding this comment

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

oops, nope

Copy link
Contributor

Choose a reason for hiding this comment

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

This file seems to have been resurrected at the root of the project

Copy link
Contributor

Choose a reason for hiding this comment

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

this file shouldn't exist at the root of the project

Copy link
Contributor

Choose a reason for hiding this comment

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

There was already a .storybook setup in the UI library, as it is connected to NX generators. Is this a completely separate instance, and where are we canonically going to be configuring and storing our component lib, here? We should consolidate this as I don't believe its intended to have 2 storybook instances (probably just remove the other storybook config and update the commands to run it to point to this one).

Comment on lines +11 to +30
current: "currentColor",
transparent: "transparent",
stroke: "#EEEEEE",
strokedark: "#2D2F40",
hoverdark: "#252A42",
titlebg: "#ADFFF8",
titlebg2: "#FFEAC2",
titlebgdark: "#24598F",
btndark: "#292E45",
white: "#FFFFFF",
black: "#181C31",
blackho: "#2C3149",
blacksection: "#1C2136",
primaryho: "#0063EC",
meta: "#20C5A8",
waterloo: "#757693",
manatee: "#999AA1",
alabaster: "#FBFBFB",
zumthor: "#EDF5FF",
socialicon: "#D1D8E0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Were you able to discuss the token values with @ricardoantoniocm?

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.

3 participants