Skip to content

ProgressBar: Fix inconsistent aria-valuenow rounding for decimal strings - #8449

Open
UnderHear wants to merge 5 commits into
primer:mainfrom
UnderHear:main
Open

UnderHear wants to merge 5 commits into
primer:mainfrom
UnderHear:main

Conversation

@UnderHear

@UnderHear UnderHear commented Sep 26, 2026 •

Copy link
Copy Markdown

Fix inconsistent aria-valuenow rounding between numeric and string decimal progress values.

Previously:

<ProgressBar progress={33.8} />   // aria-valuenow="34"
<ProgressBar progress="33.8" />   // aria-valuenow="33"

Both render a width of 33.8%, but expose different accessibility values because parseInt truncates the decimal part of string inputs before Math.round can round the value.

Replacing parseInt with parseFloat preserves the decimal part, allowing the existing Math.round to round both numeric and string inputs to the nearest integer consistently. Now, both 33.8 and "33.8" produce aria-valuenow="34", while the visual width remains 33.8%.

Changelog

New

  • No new APIs.

Changed

  • Use parseFloat instead of parseInt when parsing string progress values, preserving decimals for the existing rounding logic.
  • Add regression tests covering numeric and string decimal progress values.

Removed

  • None.

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Testing & Reviewing

  • All 20 ProgressBar tests passed using the Playwright provider with local Microsoft Edge.
  • ESLint passed for both modified files.
  • Regression tests verify that 33.8 and "33.8" both produce aria-valuenow="34" while preserving the 33.8% visual width.

@UnderHear
UnderHear requested a review from a team as a code owner September 26, 2026 17:51
@changeset-bot

changeset-bot Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 848a301

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@UnderHear UnderHear left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Replacing parseInt with parseFloat preserves decimal precision before the existing Math.round, making numeric and string decimal inputs consistent. However, it also changes how some special strings are handled:

Input Previous aria-valuenow New aria-valuenow
"33.8" 33 34
"Infinity" 0 Infinity
"1e2" 1 100
"1e-2" 1 0
"1e309" 1 Infinity

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The published behavior change is missing the required patch changeset.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Fixes ProgressBar accessibility values so numeric and string decimal inputs round consistently without changing visual width.

Changes:

  • Replaces parseInt with parseFloat for string progress values.
  • Adds regression tests covering decimal rounding and preserved width.
File Description
packages/​react/​src/​ProgressBar/​ProgressBar.tsx Preserves string decimals before rounding accessibility values.
packages/​react/​src/​ProgressBar/​ProgressBar.test.tsx Checks both input types produce value 34 and width 33.8%.

Comment thread packages/react/src/ProgressBar/ProgressBar.tsx

@siddharthkp siddharthkp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Very nice, thank you!

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@UnderHear

UnderHear commented Oct 8, 2026 •

Copy link
Copy Markdown
Author

Hi, siddharthk! thk u approve my PR.
and I have a question, how to get Finishing Touch achievements?

Finishing Touch achievements image

i'm really like it

@siddharthkp

siddharthkp commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

FYI: I've enabled auto-merge, it will merge when main is unlocked.

and I have a question, how to get Finishing Touch achievements?

Sorry, that one is only available for staff for now!

@primer
primer Bot disabled auto-merge October 14, 2026 15:59
@primer
primer Bot enabled auto-merge October 14, 2026 15:59
@primer
primer Bot disabled auto-merge October 14, 2026 16:01
@primer
primer Bot enabled auto-merge October 14, 2026 16:01
@primer
primer Bot disabled auto-merge October 16, 2026 17:09
@primer
primer Bot enabled auto-merge October 16, 2026 17:09
@primer
primer Bot disabled auto-merge October 16, 2026 17:11
@primer
primer Bot disabled auto-merge October 16, 2026 17:11
@primer
primer Bot enabled auto-merge October 16, 2026 17:11

This branch was successfully deployed

1 active deployment
github-pages — 848a301a Deployed Oct 17, 2026 by UnderHear via deploy-preview #30042
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