-
Notifications
You must be signed in to change notification settings - Fork 7
Update placeholder colors and get new Figma variables #485
Conversation
🦋 Changeset detectedLatest commit: cd70a2e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
40fcdff
to
cd70a2e
Compare
'--glide-core-text-placeholder' has dark mode issues. Aligns | ||
with Input and Textarea as suggested by design. | ||
*/ | ||
color: rgb(117 117 117); |
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.
Nice to see this getting cleaned up!
&.disabled { | ||
color: var(--glide-core-text-tertiary-disabled); | ||
} |
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.
From what I could tell, this isn't a valid visual state anymore due to the javascript in dropdown.ts
. We only show the placeholder, when there is no value and in the single-select case. We want the placeholder color to remain the same, no matter if it's disabled or not. This happens on line 236 above.
With this line, we would override that color value, making the placeholder washed out.
return html`<span | ||
class=${classMap({ | ||
placeholder: true, | ||
disabled: this.disabled, |
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 above comment address this deletion.
} | ||
&::placeholder { | ||
color: var(--glide-core-text-placeholder); |
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.
This is for the filterable
/ multi-select case. Since dropdown renders an input element here, we can use the pseudo element like in Input and Textarea 🎉
🚀 Description
📋 Checklist
🔬 How to Test
Placeholder colors were updated throughout, so double-check these situations:
There should be more contrast!
📸 Images/Videos of Functionality