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.

Sign up for GitHub

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

Jump to bottom

fix: select placeholder color #2558

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: select placeholder color #2558

wants to merge 1 commit into from

Conversation

bent101
Copy link

@bent101 bent101 commented Jan 24, 2024 •
edited
Loading

The placeholder for the select component was black instead of text-muted-foreground because it was using placeholder: instead of data-[placeholder]:

Closes #574

@vercel Vercel
Copy link

vercel bot commented Jan 24, 2024

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

A member of the Team first needs to authorize it.

@Snailedlt Snailedlt mentioned this pull request Feb 25, 2024
Closed
austinkelleher
austinkelleher approved these changes Mar 21, 2024
View reviewed changes
Copy link

@austinkelleher austinkelleher left a comment

Choose a reason for hiding this comment

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

Great change. We've pulled this into our project.

@abeisleem
Copy link

abeisleem commented May 15, 2024 •
edited
Loading

I agree with this change and was about to make a PR myself. An important addition I would recommend is explicitly styling the ChevronDown as to prevent it from inheriting the text-muted-foreground. Please see my comment on issue #3737

@abeisleem abeisleem mentioned this pull request May 15, 2024
Open
2 tasks
chrism1996
chrism1996 reviewed Nov 7, 2024
View reviewed changes
Copy link

@chrism1996 chrism1996 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@chrism1996
Copy link

Could anyone approve this PR? I've fixed manually in my project, but probably some other folks are facing the same issue with the Select component

@therealtgd
Copy link

Could this be merged? I ran into the same issue just now.

@Marvelousy20
Copy link

This still need to be merged. It's a great fix.

@cristian-sbardelotto-en
Copy link

This should be merged asap, it's a must to the new developers that don't know about this issue

@Neiso
Copy link

Neiso commented Feb 10, 2025

Possible to merge this? :)

IPJT
IPJT approved these changes Feb 11, 2025
View reviewed changes
Copy link

@IPJT IPJT left a comment

Choose a reason for hiding this comment

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

Works like a charm!

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

@chrism1996 chrism1996 chrism1996 left review comments

@austinkelleher austinkelleher austinkelleher approved these changes

@IPJT IPJT IPJT approved these changes

@Snailedlt Snailedlt Snailedlt approved these changes

@cristian-sbardelotto-en cristian-sbardelotto-en cristian-sbardelotto-en approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Placeholder color in Select component not as expected.
10 participants
@bent101 @abeisleem @chrism1996 @therealtgd @Marvelousy20 @cristian-sbardelotto-en @Neiso @austinkelleher @IPJT @Snailedlt