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

Various ZUI chips 🥔 #2408

Open
wants to merge 14 commits into
base: undocumented/new-design-system
Choose a base branch
from

Conversation

ziggabyte
Copy link
Contributor

Description

This PR adds various ZUI components all documented under the category "Chips" in Figma:

  • ZUITagChip
  • ZUIProgressChip
  • ZUIDataChip
  • ZUITargetGroupChip

Screenshots

bild

bild

bild

bild

Changes

[Add a list of features added/changed, bugs fixed etc]

  • Adds these four components
  • Adds storybook stories for them

Notes to reviewer

Look at them in the Storybook deployment and see that they look the same as in the Figma

Related issues

None

Copy link
Member

@richardolsson richardolsson left a comment

Choose a reason for hiding this comment

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

Nice work! Some improvements suggested below. 👇

@@ -0,0 +1,85 @@
import { Meta, StoryObj } from '@storybook/react';

Copy link
Member

Choose a reason for hiding this comment

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

This file (and it's siblings) are in a weirdly named directory. The tsx suffix should not be included in the directory name.

Comment on lines +12 to +19
/**
* Values to be displayed in each of the sections of the chip.
* An array of 2, 3 or 4 numbers.
*/
values:
| [number, number]
| [number, number, number]
| [number, number, number, number];
Copy link
Member

Choose a reason for hiding this comment

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

Very nice solution!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Stolen from logic by @river-bbc :D

Comment on lines 7 to 8
Object.keys(funSwatches).forEach((funSwatch) => {
const colorObject = funSwatches[funSwatch as keyof typeof funSwatches];
Copy link
Member

Choose a reason for hiding this comment

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

You have to typecast on line 8, because funSwatch is a string returned from Object.keys() and it's not typed any more specifically than string.

But you don't actually need the funSwatch key as far as I can tell, other than to extract the value. So maybe you could use Object.values() instead and avoid the typecast?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, i'll try this approach!

@thepeno
Copy link
Collaborator

thepeno commented Dec 13, 2024

The badge size for the version without a number inside is 16x16 px in the design it is 10x10.

The badge for the version with numbers has a height of 22px instead of 20px. The font size is correct but the padding I'm assuming is a little bit off. I used 3px y padding and 7px x padding.

That's it! Otherwise looks amazing, good job:)

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.

3 participants