Progress bar documentation #240

ross-pfahler · 2020-03-02T00:34:50Z

Closes RSP-1570

✅ Pull Request Checklist:

Included link to corresponding Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Team:

codecov · 2020-03-02T00:43:27Z

Codecov Report

❗ No coverage uploaded for pull request base (master@1a0e9e6). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #240   +/-   ##
=========================================
  Coverage          ?   70.57%           
=========================================
  Files             ?      206           
  Lines             ?     3973           
  Branches          ?      847           
=========================================
  Hits              ?     2804           
  Misses            ?      587           
  Partials          ?      582

Impacted Files	Coverage Δ
...kages/@react-spectrum/progress/src/ProgressBar.tsx	`100% <ø> (ø)`

adobe-bot · 2020-03-02T00:49:58Z

Build successful! 🎉

adobe-bot · 2020-03-02T16:58:15Z

Build successful! 🎉

adobe-bot · 2020-03-02T16:59:19Z

Build successful! 🎉

adobe-bot · 2020-03-02T17:07:23Z

Build successful! 🎉

ross-pfahler · 2020-03-02T17:07:57Z

formatOptions also missing type information

adobe-bot · 2020-03-02T22:55:45Z

Build successful! 🎉

MidnightCoder06 · 2020-03-03T02:16:43Z

packages/@react-spectrum/progress/docs/ProgressBar.mdx

+### Accessibility
+
+Progress Bars are made accessible as per the ["progressbar" role in WAI-ARIA 1.1](https://www.w3.org/TR/wai-aria-1.1/#progressbar).
+Depending on the visualization being used (ie depending on the `showValueLabel` prop), a `label`, `aria-label`, or `aria-labelledby` attribute are required.


i.e. instead of ie

MidnightCoder06 · 2020-03-03T02:19:22Z

packages/@react-spectrum/progress/docs/ProgressBar.mdx

+### Over background
+[View guidelines](https://spectrum.adobe.com/page/bar-loader/#Over-background)
+
+When a loader needs to be placed on top of a colored background, use the over background loader.


Same as with the progress circle pull, I'm wondering should this be: ... use the overBackground loader variant.

devongovett · 2020-03-03T04:31:49Z

packages/@react-spectrum/progress/docs/ProgressBar.mdx

+```tsx example
+<p><ProgressBar label="Loading…" /></p>
+<p><ProgressBar label="Loading…" labelPosition="side" /></p>
+<p><ProgressBar label="Loading…" showValueLabel={false} /></p>


Would be good to include a value for all examples. They look strange if the value is 0 IMO.

devongovett · 2020-03-03T04:32:24Z

packages/@react-spectrum/progress/docs/ProgressBar.mdx

+When a loader needs to be placed on top of a colored background, use the over background loader.
+
+```tsx example
+<div style={{background: 'rgba(0,0,0,0.4)', padding: '10px'}}>


We might want to use a color other than black. It looks strange in dark mode. Maybe use <View> with a spectrum color variable?

devongovett · 2020-03-03T04:33:19Z

packages/@react-spectrum/progress/docs/ProgressBar.mdx

+[View guidelines](https://spectrum.adobe.com/page/bar-loader/#Size)
+
+```tsx example
+<p><ProgressBar aria-label="Loading…" size="S" /></p>


We could maybe use a <Flex> container for these instead of <p> tags?

devongovett · 2020-03-03T04:40:36Z

packages/@react-spectrum/progress/docs/ProgressBar.mdx

+</div>
+```
+
+### Indeterminate


Should we include this under the value section above similar to Checkbox? @dannify

devongovett · 2020-03-03T04:42:17Z

packages/@react-spectrum/progress/docs/ProgressBar.mdx

+</div>
+```
+
+If using the over background variant, ensure the background offers enough contrast for the loader to be legible and meets color contrast guidelines.


We're a bit inconsistent between "loader" and "progress bar". "Loader" is the old term for it (design may still be using), so I think we should be consistent with "progress bar".

devongovett · 2020-03-03T05:05:50Z

packages/@react-spectrum/progress/docs/ProgressBar.mdx

+
+## Value
+
+By default Progress Bars are controlled with the `value` prop representing current percentage (as an integer).


Might need some commas

devongovett · 2020-03-03T05:06:57Z

packages/@react-spectrum/progress/docs/ProgressBar.mdx

+
+### Accessibility
+
+Progress Bars are made accessible as per the ["progressbar" role in WAI-ARIA 1.1](https://www.w3.org/TR/wai-aria-1.1/#progressbar).


Maybe not needed? We haven't linked to the spec in other docs pages... Definitely will from @react-aria docs.

devongovett · 2020-03-03T05:07:34Z

packages/@react-spectrum/progress/docs/ProgressBar.mdx

+To internationalize a Progress Bar, a localized string should be passed to the `label` prop, `aria-label` prop, or value of associated `aria-labelledby` element.
+
+For RTL (right-to-left) languages, the layout of the bar loader is mirrored for both determinate and indeterminate options.
+The placement of the percent sign differs depending on the locale.


Is this paragraph needed? It's really just an implementation detail, not something the user needs to know about. Not sure. @dannify

Remove this line.

adobe-bot · 2020-03-03T17:27:09Z

Build successful! 🎉

adobe-bot · 2020-03-03T20:01:47Z

Build successful! 🎉

adobe-bot · 2020-03-03T22:18:48Z

Build successful! 🎉

packages/@react-spectrum/progress/docs/ProgressBar.mdx

…nto progress-bar-docs

adobe-bot · 2020-03-03T22:37:56Z

Build successful! 🎉

LFDanLu

LGTM

dannify · 2020-03-04T00:20:40Z

packages/@react-spectrum/progress/docs/ProgressBar.mdx

+To internationalize a Progress Bar, a localized string should be passed to the `label` prop, `aria-label` prop, or value of associated `aria-labelledby` element.
+
+For RTL (right-to-left) languages, the layout of the bar loader is mirrored for both determinate and indeterminate options.
+The placement of the percent sign differs depending on the locale.


Remove this line.

adobe-bot · 2020-03-04T00:23:08Z

Build successful! 🎉

adobe-bot · 2020-03-04T00:29:40Z

Build successful! 🎉

ktabors

I noticed that the TypeScript interfaces aren't get Java doc style comments describing them or setting defaults like top for labelPosition.

ktabors · 2020-03-04T21:34:05Z

packages/@react-spectrum/progress/docs/ProgressBar.mdx

+
+## Value
+
+Progress Bars are controlled with the `value` prop representing current percentage (as an integer).


I thought there was a slack chat where we decided the names in the docs to match the names in the code, so ProgressBar matches <ProgressBar />?

im happy to do which ever, but we should prob be consistent since the existing packages that were merged (eg StatusLight) also do not do this. let me know which i should do and i'll convert!

ktabors · 2020-03-04T21:43:07Z

packages/@react-spectrum/progress/docs/ProgressBar.mdx

+<Flex flexDirection="column" maxWidth="250px">
+  <ProgressBar label="Loading…" marginBottom="25px" value={30} />
+  <ProgressBar label="Loading…" marginBottom="25px" labelPosition="side" value={30} />
+  <ProgressBar label="Loading…" showValueLabel={false} value={30} />


Would this last example be better without the label="Loading…"?

presumably it also sets aria-label, so i can either convert to that or leave it as is. lmk

the label is different from the valuelabel so either label or aria-label will be needed anyway.

ross-pfahler · 2020-03-04T22:44:27Z

I noticed that the TypeScript interfaces aren't get Java doc style comments describing them or setting defaults like top for labelPosition.

Yes, we have a follow up in RSP-1574

MidnightCoder06 · 2020-03-04T23:44:32Z

packages/@react-spectrum/progress/docs/ProgressBar.mdx

+<ProgressBar label="Loading…" value={50} />
+```
+
+Use an indeterminate Progress Bars when progress is happening but the time or effort to completion can’t be determined.


bar should be singular

adobe-bot · 2020-03-05T03:54:07Z

Build successful! 🎉

adobe-bot · 2020-03-05T21:26:29Z

Build successful! 🎉

adobe-bot · 2020-03-05T22:29:13Z

Build successful! 🎉

adobe-bot · 2020-03-05T22:35:55Z

Build successful! 🎉

* * updating TableView documentation at https://react-spectrum.corp.adobe.com/components/TableView. * for issues: adobe/react-spectrum#239 adobe/react-spectrum#240 * Update TableView.mdx added explanation for needing styling * Update TableView.mdx * missing `%` in TableView.styl * changed heading to "Why doesn't my table render" instead of "Example source code" and moved code to bottom * - added style height to div in StoryBook for stories/TableView.js, so deleted stories/TableView.styl - updated TableView.mdx with simpler troubleshooting and links to examples * Update TableView.mdx * Removing link that contains contradictory documentation If we need a simpler example, we can explore that in another PR

Progress bar documentation

7a952ff

ross-pfahler requested review from dannify, devongovett and snowystinger as code owners March 2, 2020 00:34

Fill in accessibility docs a bit more

9ac098a

ross-pfahler added 2 commits March 2, 2020 08:48

Formatting

a66bd5f

More formatting

af6f5ca

Formatting

c2dc256

Remove extra size

3ad4cbc

MidnightCoder06 reviewed Mar 3, 2020

View reviewed changes

devongovett reviewed Mar 3, 2020

View reviewed changes

Review comments

90852e0

Layout improvements

568ef04

Respond to feedback

5272a94

LFDanLu requested changes Mar 3, 2020

View reviewed changes

packages/@react-spectrum/progress/docs/ProgressBar.mdx Outdated Show resolved Hide resolved

packages/@react-spectrum/progress/docs/ProgressBar.mdx Outdated Show resolved Hide resolved

ross-pfahler added 2 commits March 3, 2020 14:27

Merge branch 'master' of github.com:adobe-private/react-spectrum-v3 i…

8be1f11

…nto progress-bar-docs

More review updates

53bb253

LFDanLu previously approved these changes Mar 3, 2020

View reviewed changes

move Indeterminate section

3da8aa0

ross-pfahler dismissed LFDanLu’s stale review via 3da8aa0 March 4, 2020 00:15

dannify requested changes Mar 4, 2020

View reviewed changes

Remove unneeded sentence

dc0e46b

ktabors reviewed Mar 4, 2020

View reviewed changes

MidnightCoder06 reviewed Mar 4, 2020

View reviewed changes

more examples

81cebfb

consistency

6cd6e80

Danni and others added 2 commits March 5, 2020 14:19

more fixes

e309650

Merge branch 'master' into progress-bar-docs

9564946

dannify approved these changes Mar 5, 2020

View reviewed changes

dannify merged commit e003e89 into master Mar 5, 2020

dannify deleted the progress-bar-docs branch March 5, 2020 22:37


		## Value

		By default Progress Bars are controlled with the `value` prop representing current percentage (as an integer).


		### Accessibility

		Progress Bars are made accessible as per the ["progressbar" role in WAI-ARIA 1.1](https://www.w3.org/TR/wai-aria-1.1/#progressbar).


		## Value

		Progress Bars are controlled with the `value` prop representing current percentage (as an integer).

Progress bar documentation #240

Progress bar documentation #240

Uh oh!

Conversation

ross-pfahler commented Mar 2, 2020

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Team:

Uh oh!

codecov bot commented Mar 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

adobe-bot commented Mar 2, 2020

Uh oh!

adobe-bot commented Mar 2, 2020

Uh oh!

adobe-bot commented Mar 2, 2020

Uh oh!

adobe-bot commented Mar 2, 2020

Uh oh!

ross-pfahler commented Mar 2, 2020

Uh oh!

adobe-bot commented Mar 2, 2020

Uh oh!

MidnightCoder06 Mar 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MidnightCoder06 Mar 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devongovett Mar 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adobe-bot commented Mar 3, 2020

Uh oh!

adobe-bot commented Mar 3, 2020

Uh oh!

adobe-bot commented Mar 3, 2020

Uh oh!

Uh oh!

Uh oh!

adobe-bot commented Mar 3, 2020

Uh oh!

LFDanLu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adobe-bot commented Mar 4, 2020

Uh oh!

adobe-bot commented Mar 4, 2020

Uh oh!

ktabors left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

codecov bot commented Mar 2, 2020 •

edited

Loading

MidnightCoder06 Mar 3, 2020 •

edited

Loading

MidnightCoder06 Mar 3, 2020 •

edited

Loading

devongovett Mar 3, 2020 •

edited

Loading