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

fix(shadcn): default next styles are not completely cleared #4922

Merged
merged 3 commits into from
Sep 23, 2024

Conversation

sapenlei
Copy link
Contributor

@sapenlei sapenlei commented Sep 21, 2024

after initializing nextjs project, the global css is

CleanShot 2024-09-21 at 08 16 21@2x

after initializing shadcn , the global css is

CleanShot 2024-09-21 at 08 20 12@2x

--foreground, --background of nextjs conflict with shadcn. nextjs will override shadcn.

CleanShot 2024-09-21 at 08 27 39@2x

Fix #4923

Copy link

vercel bot commented Sep 21, 2024

@sapenlei is attempting to deploy a commit to the shadcn-pro Team on Vercel.

A member of the Team first needs to authorize it.

@sapenlei sapenlei marked this pull request as draft September 21, 2024 01:45
@sapenlei sapenlei closed this Sep 21, 2024
@sapenlei sapenlei reopened this Sep 21, 2024
@sapenlei sapenlei marked this pull request as ready for review September 21, 2024 01:49
@shadcn
Copy link
Collaborator

shadcn commented Sep 22, 2024

This is intentional. We use different tokens from default Next.js. Is this an issue?

@shadcn shadcn added the postpone: more info or changes requested maintainers asked a question or needs more info label Sep 22, 2024
@sapenlei
Copy link
Contributor Author

This is intentional. We use different tokens from default Next.js. Is this an issue?

The tokens for previous versions of nextjs were different, but the current ones are the same

@sapenlei
Copy link
Contributor Author

This is intentional. We use different tokens from default Next.js. Is this an issue?

After nextjs version 14.2.7, the background and foreground tokens of global css changed and became the same as those of shadcn

@Jacksonmills
Copy link
Contributor

idk if it helps but I just spun up a new next app with shadcn@latest init to test this as I was hitting some weirdness and had to do a little refactor in a separate project.

specifically noticed this when setting up themes using the class based approach, easy enough to just delete the lines and then the usual set up worked just fine

seems these lines do override the shadcn base variables:

:root {
  --background: #ffffff;
  --foreground: #171717;
}

@media (prefers-color-scheme: dark) {
  :root {
    --background: #0a0a0a;
    --foreground: #ededed;
  }
}

some screenshots:
image
image

@Jacksonmills
Copy link
Contributor

This is intentional. We use different tokens from default Next.js. Is this an issue?

The tokens for previous versions of nextjs were different, but the current ones are the same

vercel/next.js@c0b3f43

@shadcn
Copy link
Collaborator

shadcn commented Sep 23, 2024

@Jacksonmills you're right. @sapenlei I also misread the PR title. I thought you meant we should not override. I'll review and merge. Thank you both.

@shadcn shadcn added bug Something isn't working area: shadcn and removed postpone: more info or changes requested maintainers asked a question or needs more info labels Sep 23, 2024
Copy link
Collaborator

@shadcn shadcn left a comment

Choose a reason for hiding this comment

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

All good. Thank you.

Copy link

vercel bot commented Sep 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Updated (UTC)
ui ⬜️ Ignored (Inspect) Visit Preview Sep 23, 2024 7:38am

@shadcn shadcn merged commit c62167a into shadcn-ui:main Sep 23, 2024
5 checks passed
@sapenlei sapenlei deleted the sapenlei branch November 16, 2024 04:15
niktekusho pushed a commit to niktekusho/shadcnui that referenced this pull request Nov 21, 2024
…i#4922)

* fix(shadcn):  default next styles are not completely cleared

* chore: add changeset

* fix(shadcn): tests

---------

Co-authored-by: shadcn <m@shadcn.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: shadcn bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug]: foreground and background don't change when toggle dark mode
3 participants