-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
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
Docs: improve progress bar labels markup and explanations for accessibility #39364
base: main
Are you sure you want to change the base?
Conversation
Note that by default, the content inside the `.progress-bar` is controlled with `overflow: hidden`, so it doesn't bleed out of the bar. If your progress bar is shorter than its label, the content will be capped and may become unreadable. To change this behavior, you can use `.overflow-visible` from the [overflow utilities]({{< docsref "/utilities/overflow" >}}). | ||
|
||
{{< callout warning >}} | ||
**Accessibility warning:** Long labels may not be fully accessible with this method. As it relies on the text color having the right contrast ratio with both the `.progress` and `.progress-bar` background colors, your color palette could be incompatible with this approach. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the colour problem applies to short labels inside progress bars as well though, so i wouldn't make this callout specific to long labels. move it further down where you talk about setting text colours etc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the colour problem applies to short labels inside progress bars as well though
I'm not sure to understand precisely this point.
My approach was the following:
.text-bg-*
is taking care of the contrast issues (and if overridden, should be done by the users to ensure the right contrast).- When folks don't want to use these utilities, the following sentence is supposed to cover this use case: "... make sure to also set an appropriate text color, so the labels remain readable and have sufficient contrast".
So the remaining case would be the label not fitting in the really small space -> use overflow-visible
but be careful (the yellow accessibility callout) because you probably never find a color contrast working for both the colored progress bar, the gray area behind, and the text over these 2 surfaces.
By moving the yellow callout down and making it non-specific to long labels, I have the impression that a use case wouldn't be explained/understood.
Do you have an idea/a wording to tackle everything in one callout moved further down? Don't hesitate to push a commit on top of mine. Would be great if there was a simplified way to explain that, my wording capacities are limited in English ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure to understand precisely this point.
unless i'm missing something, regardless of length of the label, it will always have to contrast against both the progress bar itself and the progress background. it's just that it's likely that for "long" labels, the problem is likely more obvious?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about the first two use cases in https://codepen.io/julien-deramond/pen/QWYxwXV where the overflow is hidden or when the value is static and the developers know whether the label can always fit within the progress bar and will never overlap the progress background.
Maybe it's a bad practice but I saw that on websites.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, getting back to this after a long break... but my point is even in those examples you mention, look at the first one...it doesn't quite fit into the blue bar either.
so my point is: the callout applies in all situations - you'll likely never find a colour for the text that contrasts sufficiently against both the bar and the bar background, and particularly when the progress bar is actually used to show dynamic progress, you won't generally be able to guarantee that text fits in perfectly under all conditions. that's why i'd say the callout should be generalised, as it's not just a problem with dark mode etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comment about moving the callout
Adding the v5.4.0 project here too @julien-deramond in case we want to save this for a larger release as opposed to a patch. |
@julien-deramond happy to help push this to the end if you need a hand |
I could definitely use a hand to push this to the end. I haven't forgotten, but my TODO list is a bit out of control right now 😅 I agree with what you mentioned in our discussion on this PR. I tried to explain some edge cases where users know the size of the colored part and the specific wording (e.g., in a resume to show skills, on a website for fixed prices or quotas, etc.), where our label-version could be used. However, your explanation is simpler and fits most cases, while my examples are more edge cases. If you think it's better not to mention them at all, I'll follow your judgment. I'd love to see your perspective on this PR! |
@julien-deramond I'm about to disappear for a weak of vacation, but once I'm back, let's get this over the line. The example might still be useful, but as mentioned I think the callout makes sense as a more general callout for the whole component (not just for edge cases). I'll have a think while I'm at the side of a pool :) |
Description
Usage of color and background helper classes
In the current Progress > Backgrounds documentation, we say the following:
In the same spirit as what we've done in #39214, this PR suggests always using
.text-bg-{color}
when a label is used within the progress component. If one changes the colors between the light and the dark mode (contrary to Bootstrap), you'd like the text color to adapt automatically in light and dark mode to have sufficient contrast. And it can't be done with.text-dark
or other utilities.The idea here is to:
.bg-{color}
.text-bg-{color}
.text-bg-{color}
Note: we talked about it internally and @mdo seemed to agree with this option
Long labels
The second part of the current Progress > Labels mentions something to handle the long labels. It's already mentioned that “Be aware though that currently this approach does not take into account color modes.”
However, I don't think there's anything we can provide that will make it work all the time.
Right now, we use .text-dark which obviously doesn’t work in dark mode as mentioned.
Based on my previous proposal, we should rather use
.text-bg-primary
which works well in dark mode and half-works in light mode (only on the blue color):The issue here is that we will never find anything that can work for both colors with the actual color palette. It can only work if the color palette is compatible and that
.progress
and.progress-bar
have the same tint. Said differently, that they are both using light colors or dark colors so that the text color has sufficient contrast.Since we don't know how the color palettes of projects will be built, I suggest that we don't show this example anymore in our documentation but still explain that it's possible, but with a big warning regarding accessibility and contrast ratio, and that we rather recommend most of the time to display the label outside the progress bar.
Note: we talked about it internally and @mdo seemed to agree with this option
Friendly ping for @mdo and @patrickhlauke for wording and double-checking the explanation. We might want to show how to display the label outside of the progress bar and keep it accessible.
Please also note that I haven't changed the Cheatsheet example since the values fit within the progress bar and don't have any contrast ratio issues.
Target version
IMO this is non-breaking for users so it could be embedded directly into the v5.3.x. Thoughts?
Type of changes
Checklist
npm run lint
)Live previews