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

#54/create text UI component #85

Merged
merged 11 commits into from
Jul 5, 2022
Merged

#54/create text UI component #85

merged 11 commits into from
Jul 5, 2022

Conversation

gbudau
Copy link
Contributor

@gbudau gbudau commented Jul 1, 2022

closes #54

gbudau added 2 commits July 1, 2022 20:49
Copy link
Contributor

@vdedios vdedios left a comment

Choose a reason for hiding this comment

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

Very nice ❤️

Just requesting changes because I believe we should update the component to use semantic tags as you suggested

webapp/src/shared/components/Text/Text.tsx Outdated Show resolved Hide resolved
webapp/src/shared/components/Text/Text.tsx Outdated Show resolved Hide resolved
webapp/src/shared/components/Text/Text.tsx Outdated Show resolved Hide resolved
webapp/src/shared/components/Text/__tests__/Text.test.tsx Outdated Show resolved Hide resolved
gbudau added 3 commits July 3, 2022 13:29
Add TextColor enum

Update index.css
  Rename --color-game to --gradient-game
  Add --primary-font variable
@gbudau
Copy link
Contributor Author

gbudau commented Jul 3, 2022

@vdedios, thanks for the feedback ❤️. I think this is ready for another review.

@gbudau gbudau requested a review from vdedios July 3, 2022 12:28
Lowercase CSS class names
Copy link
Contributor

@Damrod Damrod left a comment

Choose a reason for hiding this comment

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

LGTM!

Rename TextType to TextVariant and add a default tag and size
Copy link
Contributor

@vdedios vdedios left a comment

Choose a reason for hiding this comment

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

Very nice!!

webapp/src/shared/components/Text/Text.tsx Outdated Show resolved Hide resolved
Use CSS classes instead of variables
Update TextColor enum: extend the Color enums
Update Icon component, pass the color as a className
Update tests
Copy link
Contributor

@fizcamposancos fizcamposancos left a comment

Choose a reason for hiding this comment

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

Very good!!!

@gbudau
Copy link
Contributor Author

gbudau commented Jul 5, 2022

Thank you 🎉

@gbudau gbudau merged commit 92a1297 into main Jul 5, 2022
@gbudau gbudau deleted the #54/Create-text-UI-component branch July 5, 2022 11:26
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.

⚛️ Create text UI component
4 participants