-
Notifications
You must be signed in to change notification settings - Fork 734
TabController - move to design tokens #1850
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
Conversation
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.
Looks good, two small comments :)
And - I know it's a different component, but we should think about how we'll handle the fader color.
}, | ||
containerShadow: { | ||
...Platform.select({ | ||
ios: { | ||
shadowColor: Colors.grey10, | ||
shadowColor: Colors.black, |
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.
Why black?
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.
Private has this as black
, and it removed the need for a new token, WDYT?
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.
But this way it'll be black
in all modes.
Maybe $backgroundNeutralHeavy
will do the work here since it's dark in light mode, and light in dark mode..
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.
We still don't have a solution for shadows AFAIK, so if we're not handling this in private, why should this be different?
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.
Ohh so you just align the color to the private one?
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.
Not really align everything, but this specifically because otherwise it's a new token for this edge case...
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.
I see, ok..
Description
TabController - move to design tokens
I did change the color slightly in two places so we won't need to create a new token (this is aligned with private, so it seems ok).
Changelog
TabController - move to design tokens