-
Notifications
You must be signed in to change notification settings - Fork 568
feat(chip): semantic variants, pill shape and a status dot #8505
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
base: main
Are you sure you want to change the base?
Changes from all commits
4d9d1bf
0bfc15a
a234bab
8c173df
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -13,13 +13,38 @@ | |||||
| border-color: var(--color-border-action); | ||||||
| } | ||||||
|
|
||||||
| // Status colours carry their meaning in the fill, so an outline only adds | ||||||
| // noise. Transparent rather than removed, so a status chip is the same | ||||||
| // height as a bordered one beside it. | ||||||
| &--success, | ||||||
| &--warning, | ||||||
| &--danger, | ||||||
| &--info, | ||||||
| &--muted, | ||||||
| &--solid { | ||||||
| border-color: transparent; | ||||||
| } | ||||||
|
|
||||||
| // Status dot. currentColor, so it follows the variant with nothing to wire. | ||||||
| &__dot { | ||||||
| width: 6px; | ||||||
| height: 6px; | ||||||
| border-radius: var(--radius-full); | ||||||
| background: currentColor; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Use the configured Stylelint reports Proposed fix- background: currentColor;
+ background: currentcolor;📝 Committable suggestion
Suggested change
🧰 Tools🪛 Stylelint (17.14.0)[error] 31-31: Expected "currentColor" to be "currentcolor" (value-keyword-case) (value-keyword-case) Source: Linters/SAST tools |
||||||
| flex-shrink: 0; | ||||||
| } | ||||||
|
|
||||||
| // Sizes (default is the base above). | ||||||
| &--sm { | ||||||
| padding: 3px 8px; | ||||||
| font-size: 0.75rem; | ||||||
| } | ||||||
|
|
||||||
| // 24px is fixed in the tags frame, so it is stated rather than left to derive | ||||||
| // from the inherited line-height. The frame's 8px vertical padding is an | ||||||
| // artefact of a height override on the auto-layout and is not applied. | ||||||
| &--xs { | ||||||
| height: 24px; | ||||||
| padding: 1px 6px; | ||||||
| font-size: 0.6875rem; | ||||||
| } | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| import React, { FC } from 'react' | ||
|
|
||
| // The leading dot on a status chip. Decorative: the label carries the meaning. | ||
| const ChipDot: FC = () => <span className='ds-chip__dot' aria-hidden='true' /> | ||
|
|
||
| export default ChipDot | ||
|
Comment on lines
+4
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Move The new
📍 Affects 2 files
Source: Coding guidelines |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| export { default } from './Chip' | ||
| export { default as ChipDot } from './ChipDot' | ||
| export type { ChipProps, ChipSize, ChipVariant } from './Chip' |
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,24 @@ | ||
| import { FC } from 'react' | ||
| import Chip, { ChipDot, ChipVariant } from 'components/base/Chip' | ||
| import { ExperimentStatus } from 'common/types/responses' | ||
| import { EXPERIMENT_STATUS_LABELS } from 'components/experiments/constants' | ||
| import './StatusBadge.scss' | ||
|
|
||
| const STATUS_VARIANTS: Record<ExperimentStatus, ChipVariant> = { | ||
| completed: 'muted', | ||
| created: 'info', | ||
| paused: 'warning', | ||
| running: 'success', | ||
| } | ||
|
|
||
| type StatusBadgeProps = { | ||
| status: ExperimentStatus | ||
| } | ||
|
|
||
| const StatusBadge: FC<StatusBadgeProps> = ({ status }) => { | ||
| return ( | ||
| <span className={`status-badge status-badge--${status}`}> | ||
| <span className='status-badge__dot' /> | ||
| {EXPERIMENT_STATUS_LABELS[status]} | ||
| </span> | ||
| ) | ||
| } | ||
| const StatusBadge: FC<StatusBadgeProps> = ({ status }) => ( | ||
| <Chip variant={STATUS_VARIANTS[status]} size='sm'> | ||
| <ChipDot /> | ||
| {EXPERIMENT_STATUS_LABELS[status]} | ||
| </Chip> | ||
| ) | ||
|
|
||
| export default StatusBadge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the inaccurate “pill token” description.
The fixed 6px radius does not produce a pill shape. Change “pill token” to “chip” or “labelled token” so the description matches the component API and rendered shape.