Skip to content

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

Merged
merged 2 commits into from
Feb 17, 2022

Conversation

M-i-k-e-l
Copy link
Collaborator

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

Copy link
Contributor

@lidord-wix lidord-wix 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, 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,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why black?

Copy link
Collaborator Author

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?

Copy link
Contributor

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..

Copy link
Collaborator Author

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?

Copy link
Contributor

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?

Copy link
Collaborator Author

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...

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, ok..

@M-i-k-e-l M-i-k-e-l requested a review from lidord-wix February 17, 2022 09:38
@lidord-wix lidord-wix merged commit 4374cd4 into master Feb 17, 2022
@M-i-k-e-l M-i-k-e-l deleted the feat/move-tab-controller-to-design-tokens branch February 17, 2022 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants