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

Create badge component #4497

Merged
merged 27 commits into from
Jun 24, 2022
Merged

Create badge component #4497

merged 27 commits into from
Jun 24, 2022

Conversation

ClementChaumel
Copy link
Contributor

@ClementChaumel ClementChaumel commented Jun 21, 2022

Done

Create badge component and a few examples

Fixes #1394 #1395 #1396 #1397 #1398

QA

  • Open demo
  • [Add QA steps]
  • Review updated documentation:
    • [List any updated documentation for review]

Check if PR is ready for release

If this PR contains Vanilla SCSS code changes, it should contain the following changes to make sure it's ready for the release:

  • PR should have one of the following labels to automatically categorise it in release notes:
    • Feature 🎁, Breaking Change 💣, Bug 🐛, Documentation 📝, Maintenance 🔨.
  • Vanilla version in package.json should be updated relative to the most recent release, following semver convention:
    • if CSS class names are not changed it can be bugfix relesase (x.x.X)
    • if CSS class names are changed/added/removed it should be minor version (x.X.0)
    • see the wiki for more details
  • Any changes to component class names (new patterns, variants, removed or added features) should be listed on the what's new page.
  • Documentation side navigation should be updated with the relevant labels.

Screenshots

Default:
image

Tabs:
image

Chips:
image

Colors:
image

Side Navigation:
image

@ClementChaumel ClementChaumel added the Feature 🎁 New feature or request label Jun 21, 2022
@webteam-app
Copy link

webteam-app commented Jun 21, 2022

@lyubomir-popov
Copy link
Contributor

@ClementChaumel which design are you using for reference? These seem a bit different from the figma components https://www.figma.com/file/H6MSsN3taoXXOJCPUbIImQ/Design-System-Library?node-id=1310%3A9183

@ClementChaumel
Copy link
Contributor Author

@ClementChaumel which design are you using for reference? These seem a bit different from the figma components https://www.figma.com/file/H6MSsN3taoXXOJCPUbIImQ/Design-System-Library?node-id=1310%3A9183

What are the font sizes, colours and spacing we should use?

@ClementChaumel
Copy link
Contributor Author

Udpated, PTAL @lyubomir-popov

Copy link
Contributor

@lyubomir-popov lyubomir-popov left a comment

Choose a reason for hiding this comment

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

a couple of small comments in the code

scss/_patterns_badge.scss Show resolved Hide resolved
scss/_patterns_chip.scss Outdated Show resolved Hide resolved
@@ -46,7 +46,7 @@
border: none;
color: $color-dark;
display: flex;
gap: $spv--medium;
Copy link
Contributor

@bartaz bartaz Jun 22, 2022

Choose a reason for hiding this comment

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

There is no "medium" in horizontal spacing, use $sph--small or $sph--large in this case (I guess small should be enough?)

https://vanillaframework.io/docs/settings/spacing-settings#horizontal-spacing

Copy link
Contributor

Choose a reason for hiding this comment

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

or maybe just use a n-space in a pseudo element? This shouldn't really be aligned with any spacing as it is a specific scenario between text and a rounded element, which we don't have in other places.

scss/_patterns_chip.scss Outdated Show resolved Hide resolved
scss/_patterns_badge.scss Outdated Show resolved Hide resolved
Copy link
Contributor

@bartaz bartaz left a comment

Choose a reason for hiding this comment

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

Small fix in accessibility docs, but LGTM. Thanks!

templates/docs/patterns/badge/accessibility.md Outdated Show resolved Hide resolved
Co-authored-by: Bartek Szopka <83575+bartaz@users.noreply.github.com>
@bartaz bartaz merged commit 9044d53 into main Jun 24, 2022
@bartaz bartaz deleted the create-badge-component branch April 28, 2023 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set max-width to text elements (p, h1-6, blockquote etc.)
5 participants