Progress ring and Spinner Use cases - #1371
Conversation
…dance-src into progress-ring-use-case
This comment was marked as off-topic.
This comment was marked as off-topic.
bramus
left a comment
There was a problem hiding this comment.
I have no further remarks. Thanks for the updates!
There was a problem hiding this comment.
Implementation
- Why 2 elements? I might be missing something (e.g. some a11y reason), but why are we hiding the actual semantic element here? This seems to confer no benefit over the div + ARIA approach, it just adds boilerplate.
The idea of this guide was is to style the<progress>itself. - I'd use mask as a fallback and
background-clip: border-areaas the primary implementation, as it's more flexible and closer to user intent. - This could also be a good use case for the new
accentcolorsystem color keyword once it ships. Does it have a web-feature id? If not, we could use atmp-(see #1256) - Even generated content still works in that mode, so progress-ring could be a good use case for expanded
attr()too! Make sure to apply a counter rotate. If that's janky, could switch to no rotate and doing it all in the start and end angles.
Visual design
- I'd avoid a gradient for the spinner. It looks dated, the current trend is flat line with either a rotate (e.g. shadcn) or irregular animation (e.g. WA).
- Another thing that applies to both the progress-ring and the spinner is that we often want rounded line caps. I'm not convinced they matter tremendously in such a small size, but they can be done with
radial-gradient()if need be. Might be worth a note.
I quickly Clauded this codepen to illustrate some of these: https://codepen.io/leaverou/pen/EaWaaGm
There isn't an a11y issue. For the progress ring, we need a wrapper for the inner content anyways, and that carried over for the spinner. It was also to avoid the block that hides all the prefixed
Done.
Hmm, the
This was fairly straightforward to do with
Removed the gradient.
I added this as solely a note in the guide, but didn't provide code for this- is that ok? |
I was going to say we don't if it's just a matter of showing a simple unformatted percentage, but I had forgotten generated content on In that case, do we actually need the extra One potential way forwards is: use
I think that's ok, we'll have a similar issue in slider too. We could frame the prefixed ones as fallbacks (in the fallbacks section), and only use the actual standard pseudo-elements (
There are two components of this:
I think even 1 alone makes it a better fallback than the hardcoded blue we currently have, 2 just improves things further. They should probably have separate web-feature ids. I'll file an issue in the web-features repo. Meanwhile, that's the textbook case for temporary web-feature ids!
We do need a MO in the fallback to update it though otherwise the control will fail at this most basic function as a progress bar 😀
NVM, there is no content in the indeterminate case and no rotation in the determinate case so this doesn't apply. Also gencontent doesn't work reliably, so just ignore this, sorry!
See https://github.com/GoogleChrome/modern-web-guidance-src/pull/1371/changes#r3889139078 |
LeaVerou
left a comment
There was a problem hiding this comment.
See comments. Only critical one is using a MO for the attr() fallback, the rest are mostly suggestions.
LeaVerou
left a comment
There was a problem hiding this comment.
LGTM, see comments and suggestions, but none of them is a blocker.
…dance-src into progress-ring-use-case
|
Not sure why it's failing CI, but fine to merge by me! |
rviscomi
left a comment
There was a problem hiding this comment.
Addresses the failing CI check
Co-authored-by: Rick Viscomi <rviscomi@users.noreply.github.com>
Fixes #1247, #1250
Given that these link to each other and have overlap, I wanted to work on them together.
I think there is a case for adding an indeterminate state to the progress ring, but opted to have them separate, as that pattern seemed to be common in design libraries.
Both have alternate markup possibilities, which I mention but opted not to expand on.