Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Closed
ok7sai wants to merge 4 commits into angular:main from ok7sai:scoped-tailwind-v3

Conversation

Copy link
Member

@ok7sai ok7sai commented Sep 3, 2025

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.dev application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Tailwind CSS not supported in code examples.

What is the new behavior?

  • Allow using Tailwind CSS utility classes in code examples(scoped to .docs-example-viewer-preview).
  • Add example using Tailwind CSS to kitchen-sink.md.
9bSjijCRzNCRiZb 3NuosCW4njZ5PhT

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@angular-robot angular-robot bot added area: docs Related to the documentation area: docs-infra Angular.dev application and infrastructure labels Sep 3, 2025
@ngbot ngbot bot added this to the Backlog milestone Sep 3, 2025
Copy link
Member Author

ok7sai commented Sep 3, 2025

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.

Copy link

github-actions bot commented Sep 3, 2025
edited
Loading

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.

"devDependencies": {
"autoprefixer": "10.4.21",
"postcss": "8.5.6",
"tailwindcss": "3.4.17"
Copy link
Contributor

@alan-agius4 alan-agius4 Sep 4, 2025

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?

Copy link
Member Author

@ok7sai ok7sai Sep 4, 2025
edited
Loading

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.

Copy link
Contributor

@alan-agius4 alan-agius4 Sep 4, 2025

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.

Copy link
Member Author

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.

Copy link
Contributor

@alan-agius4 alan-agius4 Sep 4, 2025

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.

ok7sai reacted with heart emoji
Copy link
Contributor

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

LGTM

@alan-agius4 alan-agius4 added target: patch This PR is targeted for the next patch release action: review The PR is still awaiting reviews from at least one requested reviewer labels Sep 4, 2025
Copy link
Member

@josephperrott josephperrott left a 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

Copy link
Member Author

ok7sai commented Sep 4, 2025
edited
Loading

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.

@alan-agius4 alan-agius4 added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews target: major This PR is targeted for the next major release and removed action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Sep 5, 2025
Copy link
Contributor

@ok7sai, this require a rebase and it doesn't apply cleanly on the patch branch.

Copy link
Member Author

ok7sai commented Sep 5, 2025

Rebase done 👍

@ok7sai ok7sai added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Sep 9, 2025
Copy link

ngbot bot commented Sep 9, 2025

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.

@ok7sai ok7sai added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: merge The PR is ready for merge by the caretaker labels Sep 9, 2025
@ok7sai ok7sai added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Sep 9, 2025
Copy link
Contributor

@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.

ok7sai reacted with thumbs up emoji ok7sai reacted with eyes emoji

@thePunderWoman thePunderWoman added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: merge The PR is ready for merge by the caretaker labels Sep 10, 2025
@thePunderWoman thePunderWoman added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Sep 10, 2025
Copy link
Contributor

This PR was merged into the repository. The changes were merged into the following branches:

ok7sai reacted with heart emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@josephperrott josephperrott josephperrott approved these changes

@alan-agius4 alan-agius4 alan-agius4 approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker adev: preview area: docs Related to the documentation area: docs-infra Angular.dev application and infrastructure target: major This PR is targeted for the next major release
Projects
None yet
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /