You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(ProgressBar): introduce a new ProgressBar - #935
Adds a determinate ProgressBar to ui-react and ui-rnative, with semantic and decorative appearances, an optional label and a 200ms ease-out fill animation.
6/10 — The web fill's width transition still runs when the user enables prefers-reduced-motion: reduce. Unlike the native hook's ReduceMotion.System behavior, this component has no reduced-motion override. Add motion-reduce:transition-none so progress updates are immediate for users who request reduced motion.
Forward presentation and direction props to labeled progress wrapper
7/10 — With a visible label, className targets the wrapper, but style, hidden, and dir are spread onto the inner track only. Consequently, style={{ display: 'none' }} or hidden leaves the label visible, a styled width does not constrain the label wrapper, and dir='rtl' does not change the wrapper's start/end alignment on an LTR page. Forward these presentation and direction props to the wrapper when labeled and to the track when unlabeled, while keeping progress-specific ARIA attributes on the track. Add regression tests for these labeled cases.
The reason will be displayed to describe this comment to others. Learn more.
[low]: on RN the label Text is still its own a11y element, so VoiceOver/TalkBack will stop on "Syncing accounts" and then again on the bar ("Sync progress, 72%"). Since the docs say the label is visual only and accessibilityLabel names the bar, I think we can just hide it from the tree, like we do in AmountDisplay / MenuList:
Suggested change
<Texttypography='body4'style={styles.label}>
{label}
</Text>
<Text
typography='body4'
style={styles.label}
accessibilityElementsHidden
importantForAccessibility='no-hide-descendants'
>
{label}
</Text>
Not blocking, but we could also make accessibilityLabel optional when label is a plain string and fall back to it, closer to how web behaves.
The reason will be displayed to describe this comment to others. Learn more.
[low]: small inconsistency here: with a label, className goes to the wrapper but style (and id, data-*, handlers…) stay on the bar through {...props}. So className='mt-8' and style={{ marginTop: 8 }} don't move the same element. RN gets this right since both lx and style go to the wrapper.
Could we route style the same way as className? Or at least mention it next to the className note in the JSDoc and the MDX.
The reason will be displayed to describe this comment to others. Learn more.
[low]: Base sets both aria-label and label with the same text, but the MDX introduces it with "a bar without a visible label needs an aria-label", and says a bit further down that with a label no aria-label is needed. Since people will copy this one, I'd drop aria-label here (keeps label in Controls) and reword the Basic Usage sentence in the MDX.
The reason will be displayed to describe this comment to others. Learn more.
[low]: the example only has fixed values, so whatever variant you pick in Figma, Dev Mode will always show value={25} label='Label' labelPosition='start-bottom'. Most of our other .figma.tsx map a props block (e.g. Pagination). Could we map appearance and label position with figma.enum and the label text with figma.string? Same thing for the RN file.
The reason will be displayed to describe this comment to others. Learn more.
[mid]: Maybe don't overengineer it yet, we might move it to internal components when we start work on the stepper, so we better understand what needs to be abstracted
- Inline the label layout and drop the shared ProgressBase until the
stepper clarifies what to abstract
- Web: aria-label is required and the visible label is visual only, like RN
- Web: route style to the label wrapper alongside className
- RN: hide the label from the accessibility tree
- Map appearance, label and label position in Code Connect
- Align the Base story and MDX with the aria-label requirement
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Adds a determinate
ProgressBartoui-reactandui-rnative, with semantic and decorative appearances, an optional label and a 200ms ease-out fill animation.Closes DLS-1074
Technical details
value(0 to 100),appearance(5 semantic + 8decorative-*),labelandlabelPosition(start-top,start-bottom,end-top,end-bottom).labeloraria-label; React Native requiresaccessibilityLabel.internal/ProgressBase.How to test
Core/ProgressBar.ProgressBarscreen.