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.

Sign up for GitHub

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

Jump to bottom

#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

vdedios
vdedios requested changes Jul 2, 2022
View reviewed changes
Copy link
Contributor

@vdedios vdedios left a comment •
edited
Loading

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
Update Text component
0a1e6ed 
Add TextColor enum

Update index.css
  Rename --color-game to --gradient-game
  Add --primary-font variable
@gbudau
Update Text tests
93568e6
@gbudau
Add type prop to the Text component
29f9007
@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
@gbudau
Update Text component
f774859 
Lowercase CSS class names
Damrod
Damrod approved these changes Jul 3, 2022
View reviewed changes
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!

@gbudau
Update the Text component
8577df9 
Rename TextType to TextVariant and add a default tag and size
vdedios
vdedios approved these changes Jul 4, 2022
View reviewed changes
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
@gbudau
Update Color enum
ed05f5f 
Use CSS classes instead of variables
Update TextColor enum: extend the Color enums
Update Icon component, pass the color as a className
Update tests
fizcamposancos
fizcamposancos approved these changes Jul 5, 2022
View reviewed changes
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
Reviewers

@vdedios vdedios vdedios approved these changes

@Damrod Damrod Damrod approved these changes

@fizcamposancos fizcamposancos fizcamposancos approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

⚛️ Create text UI component
4 participants
@gbudau @Damrod @fizcamposancos @vdedios