Skip to content

Add support for high contrast themes to ProgressBar #6431

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

Merged
merged 12 commits into from
Aug 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lovely-vans-burn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

Add `border` and `background-color` tokens to `ProgressBar` CSS, which increases contrast for high contrast themes
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 2 additions & 6 deletions e2e/components/ProgressBar.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,8 @@ const stories = [
id: 'components-progressbar-features--inline',
},
{
title: 'Color',
id: 'components-progressbar-features--color',
},
{
title: 'Dev SX Props',
id: 'components-progressbar-dev--default',
title: 'All Colors',
id: 'components-progressbar-features--all-colors',
},
] as const

Expand Down
19 changes: 0 additions & 19 deletions packages/react/src/ProgressBar/ProgressBar.dev.stories.tsx

This file was deleted.

2 changes: 1 addition & 1 deletion packages/react/src/ProgressBar/ProgressBar.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"id": "components-progressbar-features--inline"
},
{
"id": "components-progressbar-features--color"
"id": "components-progressbar-features--all-colors"
},
{
"id": "components-progressbar-features--multiple-items"
Expand Down
16 changes: 13 additions & 3 deletions packages/react/src/ProgressBar/ProgressBar.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,23 @@ export const ProgressDone = () => <ProgressBar progress="100" aria-label="Upload
export const SizeSmall = () => <ProgressBar progress="66" barSize="small" aria-label="Upload test.png" />
export const SizeLarge = () => <ProgressBar progress="66" barSize="large" aria-label="Upload test.png" />

export const Inline = () => <ProgressBar inline progress="66" sx={{width: '100px'}} aria-label="Upload test.png" />
export const Inline = () => <ProgressBar inline progress="66" style={{width: '100px'}} aria-label="Upload test.png" />

export const Color = () => <ProgressBar progress="66" bg="done.emphasis" aria-label="Upload test.png" />
export const AllColors = () => (
<ProgressBar aria-label="Upload test.png">
<ProgressBar.Item progress={20} aria-label="Photo Usage" bg="accent.emphasis" />
<ProgressBar.Item progress={15} aria-label="Application Usage" bg="danger.emphasis" />
<ProgressBar.Item progress={12} aria-label="Music Usage" bg="severe.emphasis" />
<ProgressBar.Item progress={11} aria-label="Music Usage" bg="done.emphasis" />
<ProgressBar.Item progress={8} aria-label="Music Usage" bg="sponsors.emphasis" />
Copy link
Preview

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

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

The bg prop value should be 'sponsor.emphasis' not 'sponsors.emphasis' based on the pattern used in other color variants.

Suggested change
<ProgressBar.Item progress={8} aria-label="Music Usage" bg="sponsors.emphasis" />
<ProgressBar.Item progress={8} aria-label="Music Usage" bg="sponsor.emphasis" />

Copilot uses AI. Check for mistakes.

<ProgressBar.Item progress={7} aria-label="Music Usage" bg="neutral.emphasis" />
<ProgressBar.Item progress={7} aria-label="Music Usage" bg="attention.emphasis" />
</ProgressBar>
)

export const MultipleItems = () => (
<ProgressBar>
<ProgressBar.Item progress={33} aria-label="Photo Usage" sx={{bg: 'accent.emphasis'}} />
<ProgressBar.Item progress={33} aria-label="Photo Usage" bg="accent.emphasis" />
<ProgressBar.Item progress={23} aria-label="Application Usage" bg={'danger.emphasis'} />
<ProgressBar.Item progress={14} aria-label="Music Usage" bg={'severe.emphasis'} />
</ProgressBar>
Expand Down
4 changes: 3 additions & 1 deletion packages/react/src/ProgressBar/ProgressBar.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@
display: flex;
overflow: hidden;
/* stylelint-disable-next-line primer/colors */
background-color: var(--borderColor-default);
background-color: var(--progressBar-track-bgColor);
border-radius: var(--borderRadius-small);
gap: 2px;
outline: solid 1px var(--progressBar-track-borderColor);
outline-offset: -1px;

&:where([data-progress-display='inline']) {
display: inline-flex;
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/ProgressBar/ProgressBar.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const Playground = ({sections, ...args}: ProgressBarProps & {sections: nu
}, [args.bg])

if (sections === 1) {
return <ProgressBar {...args} sx={args.inline ? {width: '100px'} : {}} aria-label="Upload test.png" />
return <ProgressBar {...args} style={{...(args.inline ? {width: '100px'} : {})}} aria-label="Upload test.png" />
} else {
return (
<ProgressBar aria-label="Upload test.png">
Expand Down
76 changes: 72 additions & 4 deletions packages/react/src/ProgressBar/ProgressBar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ describe('ProgressBar', () => {
expect(container.firstChild).toHaveAttribute('data-progress-display', 'inline')
})

it('respects the "progress" prop', () => {
expect(render(<ProgressBar progress={80} aria-label="Upload test.png" />)).toMatchSnapshot()
})

it('passed the `aria-label` down to the progress bar', () => {
const {getByRole, getByLabelText} = render(<ProgressBar progress={80} aria-label="Upload test.png" />)
expect(getByRole('progressbar')).toHaveAttribute('aria-label', 'Upload test.png')
Expand Down Expand Up @@ -76,4 +72,76 @@ describe('ProgressBar', () => {

expect(getByRole('progressbar')).toHaveAttribute('aria-valuenow', '0')
})

describe('bg prop', () => {
it('applies default success bg color when no bg prop is provided', () => {
const {getByRole} = render(<ProgressBar progress={50} aria-label="Upload test.png" />)
const progressBar = getByRole('progressbar') as HTMLElement

expect(progressBar.style.getPropertyValue('--progress-bg')).toBe('var(--bgColor-success-emphasis)')
})

it('applies custom bg color when bg prop is provided', () => {
const {getByRole} = render(<ProgressBar progress={50} bg="danger.emphasis" aria-label="Upload test.png" />)
const progressBar = getByRole('progressbar') as HTMLElement

expect(progressBar.style.getPropertyValue('--progress-bg')).toBe('var(--bgColor-danger-emphasis)')
})

it('handles different color variants correctly', () => {
const colorVariants = [
{input: 'danger.emphasis', expected: 'var(--bgColor-danger-emphasis)'},
{input: 'severe.emphasis', expected: 'var(--bgColor-severe-emphasis)'},
{input: 'sponsor.emphasis', expected: 'var(--bgColor-sponsor-emphasis)'},
{input: 'done.emphasis', expected: 'var(--bgColor-done-emphasis)'},
{input: 'accent.emphasis', expected: 'var(--bgColor-accent-emphasis)'},
{input: 'success.emphasis', expected: 'var(--bgColor-success-emphasis)'},
{input: 'neutral.emphasis', expected: 'var(--bgColor-neutral-emphasis)'},
{input: 'attention.emphasis', expected: 'var(--bgColor-attention-emphasis)'},
]

for (const {input, expected} of colorVariants) {
const {getByRole, unmount} = render(<ProgressBar progress={50} bg={input} aria-label="Upload test.png" />)
const progressBar = getByRole('progressbar') as HTMLElement

expect(progressBar.style.getPropertyValue('--progress-bg')).toBe(expected)

// Clean up after each test to avoid multiple elements
unmount()
}
})

it('applies bg color to ProgressBar.Item when used in multi-item setup', () => {
const {container} = render(
<ProgressBar aria-label="Upload test.png">
<ProgressBar.Item progress={30} bg="danger.emphasis" aria-label="Danger item" />
<ProgressBar.Item progress={20} bg="success.emphasis" aria-label="Success item" />
</ProgressBar>,
)

const progressBars = container.querySelectorAll('[role="progressbar"]')

expect((progressBars[0] as HTMLElement).style.getPropertyValue('--progress-bg')).toBe(
'var(--bgColor-danger-emphasis)',
)
expect((progressBars[1] as HTMLElement).style.getPropertyValue('--progress-bg')).toBe(
'var(--bgColor-success-emphasis)',
)
})

it('handles bg values without emphasis gracefully', () => {
const {getByRole} = render(<ProgressBar progress={50} bg="danger" aria-label="Upload test.png" />)
const progressBar = getByRole('progressbar') as HTMLElement

expect(progressBar.style.getPropertyValue('--progress-bg')).toBe('var(--bgColor-danger-emphasis)')
})

it('preserves progress width regardless of bg color', () => {
const {getByRole} = render(<ProgressBar progress={75} bg="danger.emphasis" aria-label="Upload test.png" />)
const progressBar = getByRole('progressbar') as HTMLElement

expect(progressBar.style.getPropertyValue('--progress-width')).toBe('75%')
expect(progressBar.style.getPropertyValue('--progress-bg')).toBe('var(--bgColor-danger-emphasis)')
})
})
})
6 changes: 4 additions & 2 deletions packages/react/src/ProgressBar/ProgressBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const Item = forwardRef<HTMLSpanElement, ProgressBarItems>(
'aria-valuetext': ariaValueText,
className,
style,
bg,
Copy link
Preview

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

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

The bg prop is being destructured and passed to the Box component, but it's also being used in the logic below via rest.bg. This creates a conflict where rest.bg will be undefined since bg was already extracted from rest. Either use bg consistently throughout or don't destructure it from the props.

Suggested change
bg,
bg, // Note: bg is used only for local style computation and is not passed down

Copilot uses AI. Check for mistakes.

...rest
},
forwardRef,
Expand All @@ -51,9 +52,10 @@ export const Item = forwardRef<HTMLSpanElement, ProgressBarItems>(
const progressBarBg = '--progress-bg'
const styles: {[key: string]: string} = {}

const bgType = rest.bg && rest.bg.split('.')
const bgType = bg && bg.split('.')
styles[progressBarWidth] = progress ? `${progress}%` : '0%'
styles[progressBarBg] = (bgType && `var(--bgColor-${bgType[0]}-${bgType[1]})`) || 'var(--bgColor-success-emphasis)'
styles[progressBarBg] =
(bgType && `var(--bgColor-${bgType[0]}-${bgType[1] || 'emphasis'})`) || 'var(--bgColor-success-emphasis)'

return (
<BoxWithFallback
Expand Down

This file was deleted.

Loading