chore(Progress): updated placement and verbiage for status examples - #7777
Conversation
|
Preview: https://pf-pr-7777.surge.sh A11y report: https://pf-pr-7777-a11y.surge.sh |
kaylachumley
left a comment
kaylachumley
left a comment
There was a problem hiding this comment.
looks good! didn't see anything too crazy - not sure we want erin to review the copy? up to you!
edonehoo
left a comment
edonehoo
left a comment
There was a problem hiding this comment.
Just some small nits (mostly making each point a sentence)!
| When conveying status, you should ensure: | ||
| - there is visible helper text that explains the status, | ||
| - the helper text includes the status icon, as seen in our [basic helper text example](/components/helper-text#basic), and | ||
| - the helper text is linked to the `.pf-v6-c-progress__bar[role="progressbar]` element via an `aria-describedby` attribute, as seen in the [progress helper text example](#helper-text). | ||
|
|
There was a problem hiding this comment.
| When conveying status, you should ensure: | |
| - there is visible helper text that explains the status, | |
| - the helper text includes the status icon, as seen in our [basic helper text example](/components/helper-text#basic), and | |
| - the helper text is linked to the `.pf-v6-c-progress__bar[role="progressbar]` element via an `aria-describedby` attribute, as seen in the [progress helper text example](#helper-text). | |
| When conveying status, you should ensure the following: | |
| - There is visible helper text that explains the status. | |
| - The helper text includes the status icon, as seen in our [basic helper text example](/components/helper-text#basic). | |
| - The helper text is linked to the `.pf-v6-c-progress__bar[role="progressbar]` element via an `aria-describedby` attribute, as seen in the [progress helper text example](#helper-text). | |
|
|
||
| When conveying status, you should ensure: | ||
| - There is visible helper text that explains the status. | ||
| - The helper text includes the status icon, as seen in our [basic helper text example](/components/helper-text/html#basic). |
There was a problem hiding this comment.
Impressive! Anticipating the change from #static to #basic. But generally if I link to another component in the docs, I link to the default/react tab. That link won't work if you're just looking at core docs (like in this PR), but it works on org. Curious what you and @edonehoo think.
| - The helper text includes the status icon, as seen in our [basic helper text example](/components/helper-text/html#basic). | |
| - The helper text includes the status icon, as seen in our [basic helper text example](/components/helper-text#basic). |
There was a problem hiding this comment.
In this case this should be fine - I think my one caveat would be do we want to link to a React example if someone is browsing the HTML docs? The link not working correctly on staging/local builds is a fair point, but I guess that depends if we should worry as much about that or the final experience on org?
There was a problem hiding this comment.
agree! didn't catch that, but I also link to the react tab unless we're referencing a core-specific example
There was a problem hiding this comment.
oops my page didn't refresh with Eric's comment before my reply. I didn't realize that the progress doc is an html page since I'm usually in react land, but hm that's a fair point. If it's safe to assume that consumers generally stick with html/css implementation consistently (rather than mingling html & react implementations for something like this) then maybe it is better to link to the html helper text here?
mcoker
left a comment
mcoker
left a comment
There was a problem hiding this comment.
Just a couple small comments
| When conveying status, you should ensure: | ||
| - There is visible helper text that explains the status. | ||
| - The helper text includes the status icon, as seen in our [basic helper text example](/components/helper-text/html#basic). | ||
| - The helper text is linked to the `.pf-v6-c-progress__bar[role="progressbar]` element via an `aria-describedby` attribute, as seen in the [progress helper text example](#helper-text). |
There was a problem hiding this comment.
Looks like you a quotation mark
| - The helper text is linked to the `.pf-v6-c-progress__bar[role="progressbar]` element via an `aria-describedby` attribute, as seen in the [progress helper text example](#helper-text). | |
| - The helper text is linked to the `.pf-v6-c-progress__bar[role="progressbar"]` element via an `aria-describedby` attribute, as seen in the [progress helper text example](#helper-text). |
|
🎉 This PR is included in version 6.3.0-prerelease.56 🎉 The release is available on: Your semantic-release bot 📦🚀 |
5 participants
Closes #7288