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(badge): adding leading-icon to badge #2868

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

feat(badge): adding leading-icon to badge #2868

wants to merge 18 commits into from

Conversation

LinKCoding
Copy link
Contributor

@LinKCoding LinKCoding commented Apr 28, 2024

Overview

Adding an icon prop to the badge component.
Also adding a tertiary-fill variant for badges.

PR Checklist

Testing Instructions

For Mono:

  1. For the tertiaryFill and icon, you can go to the homepage
  2. Then check that badges aren't broken, go to the pricing page, there's a "New" badge above "AI learning assistance" -- check that it renders just fine

For the monolith:

  1. Log in with an admin account
  2. Go to the admin page
  3. check that both badges ("For Kicks" + "and giggles") render

PR Links and Envs

Repository PR Link PR Env
Monolith Monolith PR Monolith Env
Mono Mono Link Portal Env

@LinKCoding LinKCoding marked this pull request as ready for review May 10, 2024 13:42
@LinKCoding LinKCoding requested a review from a team as a code owner May 10, 2024 13:42
Copy link
Contributor

@dreamwasp dreamwasp left a comment

Choose a reason for hiding this comment

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

code looks great! one small tweak on some centering then it's good to go 🔥

<Icon height={size} width={size} />
</FlexBox>
)}
{children}
Copy link
Contributor

@dreamwasp dreamwasp May 10, 2024

Choose a reason for hiding this comment

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

all of the SB examples look great but noticed the portal example is very slightly wonky -
image

i think wrapping children in 100% width centered Flexbox should fix that! if it persists you may need to tweak lineHeight too 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahhhh that's on me... probably a bad example to just tweak, there was some margin stuff in there that I didn't consider. Sorry about that :(
image

oddly enough, I don't see the text being shifted - admittedly, I removed the margin and put it back just as a sanity check and it looks fine to me
image
the badge seems to have a flex display and align-items: center.
image

I temporarily changed it in storybook and it looks good (just in case it was an issue with the icon itself)
image

maybe we can look at this together on Monday.
Thanks Cass :)

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah that looks great now!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but now I'm paranoid about why you saw what you saw lol

Comment on lines +1 to +7
import { GamutIconProps } from '@codecademy/gamut-icons';

export interface WithChildrenProp {
children?: React.ReactNode | React.ReactNode[];
}

export type IconComponentType = { icon: React.ComponentType<GamutIconProps> };
Copy link
Contributor

Choose a reason for hiding this comment

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

love ittt

@LinKCoding
Copy link
Contributor Author

In the end, I opted to give 2px margins for both sized badge variants.
It looks good to me, but I'm happy to make changes if need be.

I also added a bunch more examples in SB -- more so just for reviewing.
Once I get the ok, I'll remove the ones appended with "FOR TESTING" at the end.

@LinKCoding LinKCoding requested a review from dreamwasp May 14, 2024 20:39
Copy link
Contributor

@dreamwasp dreamwasp left a comment

Choose a reason for hiding this comment

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

check it out in light mode and dark mode and both looks great! 🚢

@codecademydev
Copy link
Collaborator

📬Published Alpha Packages:

@codecademy/gamut@55.19.2-alpha.6e976e.0
@codecademy/gamut-kit@0.6.408-alpha.6e976e.0
@codecademy/styleguide@66.19.4-alpha.6e976e.0

@codecademydev
Copy link
Collaborator

🚀 Styleguide deploy preview ready!

https://6644b6f137a043033edb52e8--gamut-preview.netlify.app

Deploy Logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants