Adding basic badge component #2005

bryancunningham-okta · 2023-10-23T14:44:33Z

Summary

Testing & Screenshots

I have confirmed this change with my designer and the Odyssey Design Team.

jordankoschei-okta · 2023-10-23T15:19:08Z

packages/odyssey-react-mui/src/theme/components.tsx

+      styleOverrides: {
+        badge: ({ ownerState }) => ({
+          ...(ownerState.color === "primary" && {
+            backgroundColor: odysseyTokens.HueBlue500,


Whenever possible, use the semantic rather than palette token (ie, PalettePrimaryMain rather than HueBlue500. These aren't always available, so fall back to the Hue versions when needed.

jordankoschei-okta · 2023-10-23T15:20:15Z

packages/odyssey-react-mui/src/Badge.tsx

+export type BadgeProps = {
+  children?: ReactNode;
+  badgeContent?: number;
+  max?: MuiBadgeProps["max"];


Let's find a more explicit name for max, it wasn't immediately clear to me that this was the max number before truncation.

@jordankoschei-okta Can do. max is the prop that MUI uses by default so, just brought that over. Wasn't sure if we tried to carry prop names over from MUI or not. Any suggestions?

@jordankoschei-okta Went with badgeContentMax. Seem ok to you? Def open to suggestion still. Not in love with that name

What about maxNumberOfCharacters or maxCharacterCount?

KevinGhadyani-Okta

From eng, this looks good.

@jordankoschei-okta will wanna review the design piece, and @gretchen-okta will wanna review the Storybook text.

KevinGhadyani-Okta · 2023-10-30T18:39:03Z

packages/odyssey-react-mui/src/Badge.tsx

+  variant = "standard",
+  type = "default",
+}: BadgeProps) => {
+  const threeDigitLimitedMax = badgeContentMax > 999 ? 999 : badgeContentMax;


What's with the 999?

@KevinGhadyani-Okta We wanted to ensure that the content never goes past 3 chars. So, if the consumer passed in a max of anything higher than 999, the component would still limit it to 3 chars.

If it goes over 999, would it count as 4 characters to have it show 999+? Just wondering. Support 3 characters because it saves us from comma/period internationalization issues with 1000 etc.

That's a great point!

I hadn't thought about the complications with comma/period. @benlister-okta just wanted the max number to be 3 chars long. We're fine with 3 chars and +.

gretchen-okta · 2023-10-31T13:48:26Z

From eng, this looks good.

@jordankoschei-okta will wanna review the design piece, and @gretchen-okta will wanna review the Storybook text.

Please just send me a staging Storybook link or let me know when it's available. TY
cc @benlister-okta

chrispulsinelli-okta

There's a few changes you need to make and I have some questions about the logic. Let me know if you want to discuss in DM's - happy to clarify any questions or suggestions.

chrispulsinelli-okta · 2023-11-28T17:02:55Z

packages/odyssey-react-mui/src/Badge.tsx

+  const threeDigitLimitedMax =
+    greaterThanZeroContentMax > 999 ? 999 : greaterThanZeroContentMax;
+  const isOverContentMax = Boolean(
+    badgeContent && badgeContent > threeDigitLimitedMax


This won't work if badgeContent is 0

I think that is fine. We'd want to hide the badge if the content is 0.

Maybe a test should be added for this if one doesn't exist yet? If badgeContent = 0, isOverContentMax will be false (which seems incorrect to me as 0 is definitely less than 999), and formattedContent will be 0. I think it will render, no? Apologies if I didn't follow the logic correctly, but this is what it seems like it will do.

packages/odyssey-react-mui/src/Badge.tsx

chrispulsinelli-okta · 2023-11-28T17:07:50Z

packages/odyssey-react-mui/src/Badge.tsx

+      padding: `0 calc(${odysseyDesignTokens.Spacing1} * 1.5)`,
+      backgroundColor: badgeTypeColors(odysseyDesignTokens)[type].background,
+      color: badgeTypeColors(odysseyDesignTokens)[type].font,
+      borderRadius: contentIsLongerThanOneChar ? "10px" : "50%",


Are there any radius variables we can use instead?

We only have 1 radius variable currently, I think. I hate using 10px here but, our standard spacing variables just didn't work quite right. 10px was the magic number

I updated this to use one of our standard radius variables

chrispulsinelli-okta · 2023-11-28T17:08:14Z

packages/odyssey-react-mui/src/Badge.tsx

+    [type, contentIsLongerThanOneChar]
+  );
+
+  const shouldHideBadge = !badgeContent;


I don't believe we need this variable

Yeah, we don't need it per se. My goal was just to make it easier to see at a glance why the badge was hidden. Can remove if we think it's unnecessary

chrispulsinelli-okta · 2023-11-28T17:11:51Z

packages/odyssey-react-mui/src/Badge.tsx

+      fontWeight: `${odysseyDesignTokens.TypographyWeightBodyBold}`,
+      lineHeight: 1,
+    }),
+    [type, contentIsLongerThanOneChar]


You're missing a dependency in this callback (odysseyDesignTokens).

packages/odyssey-react-mui/src/Badge.tsx

chrispulsinelli-okta · 2023-11-30T19:05:21Z

packages/odyssey-react-mui/src/Badge.tsx

+  if (shouldHideBadge) {
+    return null;
+  }


I'd suggest moving this right up to the top. This way, we short circuit all the logic if we don't actually have to do anything.
I'd also make this more clear by stating const shouldHideBadge = badgeContent === 0;

…a standalone component

jordankoschei-okta reviewed Oct 23, 2023

View reviewed changes

KevinGhadyani-Okta reviewed Oct 30, 2023

View reviewed changes

bryancunningham-okta force-pushed the bc/add-badge-component branch from 5e4bb8c to b56da18 Compare November 17, 2023 16:41

bryancunningham-okta marked this pull request as ready for review November 17, 2023 19:32

bryancunningham-okta requested review from benschell-okta and chrispulsinelli-okta as code owners November 17, 2023 19:32

bryancunningham-okta requested a review from a team November 17, 2023 19:32

bryancunningham-okta force-pushed the bc/add-badge-component branch from 4813e05 to 9948fde Compare November 28, 2023 16:27

chrispulsinelli-okta reviewed Nov 28, 2023

View reviewed changes

chrispulsinelli-okta reviewed Nov 30, 2023

View reviewed changes

chrispulsinelli-okta approved these changes Nov 30, 2023

View reviewed changes

bryancunningham-okta added 15 commits December 1, 2023 12:10

feat(odyssey-react-mui): initial commit to add basic badge component

a9d591a

feat(odyssey-react-mui): adds basic badge component

d222198

feat(odyssey-react-mui): use pallete color defs

3149444

feat(odyssey-react-mui): add stories for all Badge types

102ba3f

feat(odyssey-react-mui): rename max prop to be more descriptive

ae592ff

feat(odyssey-react-mui): add button to badge content

050997d

feat(odyssey-react-mui): refactor Badge to not use MUI version

023589a

feat(odyssey-react-mui): remove MuiBadge style overrides

ae6e7ba

feat(odyssey-react-mui): remove obsolete props from Badge story

056f228

feat(odyssey-react-mui): use mon font family and update type names

47b8652

feat(odyssey-react-mui): use correct story names

70922c7

feat(odyssey-react-mui): use smaller border-radius

39c0d2e

feat(odyssey-react-mui): use more bold font

260741f

feat(odyssey-react-mui): remove variable and use standard radius

1d60196

feat(odyssey-react-mui): add back hide variable. Tighten hide logic

a113c16

bryancunningham-okta force-pushed the bc/add-badge-component branch from 0d8704b to a113c16 Compare December 1, 2023 17:40

feat(odyssey-react-mui): remove Badge stories and do not export. not …

dfdd4a3

…a standalone component

bryancunningham-okta merged commit f19896f into main Dec 1, 2023
1 check passed

bryancunningham-okta deleted the bc/add-badge-component branch December 1, 2023 18:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding basic badge component #2005

Adding basic badge component #2005

bryancunningham-okta commented Oct 23, 2023 •

edited

Loading

jordankoschei-okta Oct 23, 2023

jordankoschei-okta Oct 23, 2023

bryancunningham-okta Oct 23, 2023

bryancunningham-okta Oct 23, 2023

KevinGhadyani-Okta Oct 30, 2023 •

edited

Loading

KevinGhadyani-Okta left a comment

KevinGhadyani-Okta Oct 30, 2023

bryancunningham-okta Oct 30, 2023 •

edited

Loading

gretchen-okta Oct 31, 2023

KevinGhadyani-Okta Nov 7, 2023

bryancunningham-okta Nov 28, 2023

gretchen-okta commented Oct 31, 2023 •

edited

Loading

chrispulsinelli-okta left a comment

chrispulsinelli-okta Nov 28, 2023

bryancunningham-okta Nov 28, 2023

chrispulsinelli-okta Nov 30, 2023

chrispulsinelli-okta Nov 28, 2023

bryancunningham-okta Nov 28, 2023

bryancunningham-okta Nov 30, 2023

chrispulsinelli-okta Nov 28, 2023

bryancunningham-okta Nov 28, 2023

chrispulsinelli-okta Nov 28, 2023

chrispulsinelli-okta Nov 30, 2023

Adding basic badge component #2005

Adding basic badge component #2005

Conversation

bryancunningham-okta commented Oct 23, 2023 • edited Loading

Summary

Testing & Screenshots

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KevinGhadyani-Okta Oct 30, 2023 • edited Loading

Choose a reason for hiding this comment

KevinGhadyani-Okta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryancunningham-okta Oct 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gretchen-okta commented Oct 31, 2023 • edited Loading

chrispulsinelli-okta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryancunningham-okta commented Oct 23, 2023 •

edited

Loading

KevinGhadyani-Okta Oct 30, 2023 •

edited

Loading

bryancunningham-okta Oct 30, 2023 •

edited

Loading

gretchen-okta commented Oct 31, 2023 •

edited

Loading