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.

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

feat: use design tokens for text-input #443

Merged
merged 6 commits into from
Jan 20, 2019
Merged

feat: use design tokens for text-input #443

merged 6 commits into from
Jan 20, 2019

Conversation

montezume
Copy link
Contributor

@montezume montezume commented Jan 17, 2019

Summary

Talking about tokens is nice. But it's not really enough.

Approach

Use design tokens in the TextInput component.

Since we are using yaml for our design token definition file, we also set prettier to format our yaml files.

@montezume montezume changed the title TOKENIZE TEXT-INPUT feat: use design tokens for text-input Jan 17, 2019
@@ -1,19 +1,13 @@
/* Base */

.container {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this didn't do anything

@montezume montezume requested review from dferber90 and tdeekens and removed request for dferber90 January 17, 2019 14:55
@montezume
Copy link
Contributor Author

I also did PasswordInput and NumberInput because they are 100% copy paste from text-input.mod.css

@montezume
Copy link
Contributor Author

screen shot 2019-01-17 at 5 03 19 pm

😄

Copy link
Member

@emmenko emmenko left a comment

Choose a reason for hiding this comment

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

Looks sharp! 👌

Glad to see that we're using more design tokens now!

label: Font sizes
prefix: font-size
choices:
font-size-m: '13px'
Copy link
Member

Choose a reason for hiding this comment

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

What does -m mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

The size I suppose, so medium.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, exactly

@montezume montezume merged commit 792c505 into master Jan 20, 2019
@montezume montezume deleted the tokenize branch January 20, 2019 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants