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

feat(progress-bar): add support for statuses #10843

Merged
merged 2 commits into from
Mar 28, 2022

Conversation

janhassel
Copy link
Member

@janhassel janhassel commented Feb 25, 2022

Ref #10670

Adds support for ProgressBar statuses.

Changelog

New

  • Added prop status to ProgressBar component
    • Possible values: "active", "finished", "error"

Changed

  • Added logic and styling necessary to render different statuses
  • Included tests

Testing / Reviewing

  • Run storybook
  • Compare different statuses with the design guidance in Progress bar docs carbon-website#2688
  • Status "finished" should set its value to max
  • Status "error" should set its value to 0 and emit aria-invalid
  • In the inline variant, the bar should be visually hidden (though accessible) with only the icon and label being shown
    • This is because the inline variant is supposed to be used when space is tight, e.g. in a data table cell and @thyhmdo and I agreed that priorizing the label and icon over the 100%-full bar would be the best option

Notes

  • I opted for the term "finished" instead of "success" (as used in the design guidance) to be consistent with InlineLoading
  • I had to use eslint-disable as it claims aria-invalid is not supported on role="progressbar". Referencing WAI-ARIA 1.1 though, aria-invalid should be supported. Please correct me if I'm wrong.

@netlify
Copy link

netlify bot commented Feb 25, 2022

✅ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: 80a2803

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/62385834e8c8800009a77025

😎 Browse the preview: https://deploy-preview-10843--carbon-react-next.netlify.app

@netlify
Copy link

netlify bot commented Feb 25, 2022

Deploy Preview for carbon-elements failed.

Name Link
🔨 Latest commit d4d9690
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/623c3afc77e5ee0008b68af2

@netlify
Copy link

netlify bot commented Feb 25, 2022

Deploy Preview for carbon-components-react failed.

Built without sensitive environment variables

Name Link
🔨 Latest commit d4d9690
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/623c3afc9415b7000ae89b86

@janhassel janhassel marked this pull request as ready for review March 16, 2022 16:37
@janhassel janhassel requested review from a team as code owners March 16, 2022 16:37
@janhassel janhassel changed the title feat(progressbar): add support for statuses feat(progress-bar): add support for statuses Mar 17, 2022
@joshblack
Copy link
Contributor

Thanks for putting this together @janhassel! Sorry about the delay on my end, let me know if you need anything else from me here!

@janhassel
Copy link
Member Author

@joshblack No worries! I just resolved the last merge conflict that came up from the usePrefix change so I think this PR would be ready to go.

@janhassel janhassel changed the base branch from main to v10 March 24, 2022 09:34
@janhassel
Copy link
Member Author

@joshblack I changed the base branch to v10 as discussed yesterday. Could you check if everything looks up to code now?

@kodiakhq kodiakhq bot merged commit fe600c1 into carbon-design-system:v10 Mar 28, 2022
@janhassel janhassel deleted the 10670-statuses branch March 29, 2022 05:20
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.

3 participants