fix: Icon coloring rules #8323
fix: Icon coloring rules #8323
Conversation
|
Build successful! 🎉 |
|
Build successful! 🎉 |
| @@ -43,7 +43,7 @@ export const focusRing = () => ({ | |||
|
|
|||
| interface IconStyle { | |||
| size?: 'XS' | 'S' | 'M' | 'L' |'XL', | |||
| color?: 'white' | 'black' | 'accent' | 'neutral' | 'negative' | 'informative' | 'positive' | 'notice' | 'gray' | 'red' | 'orange' | 'yellow' | 'chartreuse' | 'celery' | 'green' | 'seafoam' | 'cyan' | 'blue' | 'indigo' | 'purple' | 'fuchsia' | 'magenta' | 'pink' | 'turquoise' | 'cinnamon' | 'brown' | 'silver', | |||
| color?: 'white' | 'black' | 'accent' | 'neutral' | 'negative' | 'informative' | 'positive' | 'notice' | 'gray' | 'red' | 'orange' | 'yellow' | 'chartreuse' | 'celery' | 'green' | 'seafoam' | 'cyan' | 'blue' | 'indigo' | 'purple' | 'fuchsia' | 'magenta' | 'pink' | 'turquoise' | 'cinnamon' | 'brown' | 'silver' | 'currentColor', | |||
There was a problem hiding this comment.
do we care that currentColor will now show up in color's typescript autocomplete for icons?
There was a problem hiding this comment.
perhaps a bit odd since since that means users will be able to have their workflow icons essentially inherit from an arbitrary color that a parent has? Unless I'm misunderstanding how this would get propagated
There was a problem hiding this comment.
yeah, looks like something like this is possible now:
<div
style={{
color: 'blue',
border: '1px dashed currentcolor'
}}>
<Alert {...args} styles={iconStyle({color: 'currentColor', size: 'XL'})} />
<_3DAsset styles={iconStyle({color: 'currentColor', size: 'XL'})} />
</div>
resulting in
Could we export a version of iconStyle that doesn't have currentColor and have an internal one that does support it for our internal usage?
There was a problem hiding this comment.
hmmm it's a good question, but i'd prefer that we don't have something separate, if we can build a component others should be able to build it, and an important and common use case is icons that match text color
There was a problem hiding this comment.
thats fair, just not sure how much we want to lock it down since having a subset of supported colors feels like the original intent was to only support that subset.
# Conflicts: # packages/@react-spectrum/s2/src/TreeView.tsx
|
Build successful! 🎉 |
Closes
Noticed it was possible to change the color of an icon inside a Button. This is not a feature we want to support as the color is specifically meant to meet contrast and align with the text color.
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: