-
Notifications
You must be signed in to change notification settings - Fork 26.6k
docs(docs-infra): enable using Tailwind CSS in code examples #63583
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
Conversation
Apparently I didn't add the dependencies in a correct way but I am not sure what's the correct way. What I have done is cd adev && pnpm install -D tailwindcss@3 postcss autoprefixer
. Please let me know how to fix the lockfile issue.
Deployed adev-preview for c1bb70a to: https://ng-dev-previews-fw--pr-angular-angular-63583-adev-prev-fdy17wh7.web.app
Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.
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.
Any reason why you choose version 3 instead of 4?
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.
I couldn't get version 4 to work.
The attempt main...ok7sai:_angular:scoped-tailwind and it threw Error: Cannot find module '@tailwindcss/postcss'
. I checked the bazel binary output and couldn't figure out why it didn't work, so I gave up and used version 3 instead.
Also the version 4 CSS-first approach makes it difficult to limit the usages to only be used in code examples, and the utility classes are basically the same, so I think sticking with version 3 is fine.
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.
The problem with the tailwinds v4 setup seems to be a bug in the Angular CLI.
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.
Yea I can only track down to where the issue came from https://github.com/angular/angular-cli/blob/main/packages/angular/build/src/tools/esbuild/stylesheets/stylesheet-plugin-factory.ts#L225 but don't have enough knowledge of fixing it.
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.
angular/angular-cli#31119 should fix it.
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
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
Reviewed-for: dev-infra
Are we sure we want tailwind without actually using it all over the place? I am curious if we see a large diff in the sizes of out generated outputs, I don't know that we have dead code elimination in CSS
Are we sure we want tailwind without actually using it all over the place?
The main purpose is providing code examples for developers to use in their Tailwind CSS based project. Using it across adev is a different story that requires some clean up for Tailwind preflight(base styles) to work, but certainly doable.
I am curious if we see a large diff in the sizes of out generated outputs, I don't know that we have dead code elimination in CSS
For now it added a few lines for css variables. Tailwind only adds new styles when utility classes are used in templates that it parses, so there should be no dead css code in theory.
@ok7sai, this require a rebase and it doesn't apply cleanly on the patch branch.
e4a294e
to
a54eff9
Compare
Rebase done 👍
I see that you just added the action: merge
label, but the following checks are still failing:
failure conflicts with base branch "main"
If you want your PR to be merged, it has to pass all the CI checks.
If you can't get the PR to a green state due to flakes or broken main
, please try rebasing to main
and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.
a54eff9
to
ae2f503
Compare
@ok7sai Looks like this needs a rebase and to resolve conflicts. These two files are likely to have conflicts pretty quickly. So definitely keep an eye on it until it merges.
ae2f503
to
c1bb70a
Compare
c1bb70a
to
8dfcc57
Compare
This PR was merged into the repository. The changes were merged into the following branches:
- main: d9483fa
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Tailwind CSS not supported in code examples.
What is the new behavior?
.docs-example-viewer-preview
).kitchen-sink.md
.Does this PR introduce a breaking change?
Other information