Refactor/to scss blog tag#252
Conversation
and CSS module styling
change log
✅ Deploy Preview for webdevpathstage ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
cherylli
left a comment
cherylli
left a comment
There was a problem hiding this comment.
please disregard my previous review. I missed the hardcoded color values
briangesteban
left a comment
briangesteban
left a comment
There was a problem hiding this comment.
Just as what Satoshi and Cheryl said, just need to change the static color to a CSS variable, so when we need to update the colors, we only need to update one file, which is the variables partials; also helps us implement theming in the future. Aside from that, it looks great!
variable for light background color in themes.scss and applied it to the Tag component. Update changelog to reflect the addition of the BlogTag component.
Satoshi-Sh
left a comment
Satoshi-Sh
left a comment
There was a problem hiding this comment.
Looks good to me. Thanks for your work, Shayla.
briangesteban
left a comment
briangesteban
left a comment
There was a problem hiding this comment.
Looks good to me!
If you have sometime it's also better to update the changelog request of Satoshi, although it's not a big deal, it makes our changelog consistent (as we are talking about component names on those changes).
Thanks @shayla-develops-webs!
cherylli
left a comment
cherylli
left a comment
There was a problem hiding this comment.
hi @shayla-develops-webs please resolve conflicts
cherylli
left a comment
cherylli
left a comment
There was a problem hiding this comment.
thanks @shayla-develops-webs. please merge into main when you have a chance 🙂
4 participants
Have you updated the CHANGELOG.md file? If not, please do it.
yes
What is this change?
Migrated the tag component from styled components to CSS modules
Were there any complications while making this change?
no
How to replicate the issue?
n/a
If necessary, please describe how to test the new feature or fix.
When should this be merged?
after review
Image Reference