Closed tomivirkki closed 7 years ago
Indeterminate could be implemented as a theme variant?
Yes, I think so.
Shouldn’t the foreground element be inside background element (to make theming easier)?
I don’t see a reason not to have them nested. Can’t remember the reason why I chose otherwise in my prototype.
Further, is the background element needed at all (isn’t host enough)
I would keep the background element as an explicit part as well. It gives us more freedom in the future to adjust the internal DOM, when we add new features (such as custom text content inside the bar or circle).
One more good feature to add (easy to add): set the --vaadin-progress-value
custom property whenever the value changes (using this.updateStyles(...)
to make it work with the shim). The normalized value should be used for this as well.
The theme should use that, instead of the element setting the inline transform explicitly: https://github.com/vaadin/vaadin-progress-bar/blob/master/vaadin-progress-bar.html#L59
@tomivirkki, need clarification of 1) Align with skeleton 2) Accessibility 3) Test coverage ok? 4) IE11 support (visual tests seems to be passing)
Please, give more information of what exactly should I do to resolve these items
Looking nice already. I’m now thinking about the themable parts, and foreground
and background
don’t seem like the best names. Maybe value
for the foreground and bar
for the background, similarly to what Chrome uses for the native <progress>
element?
I've tried the element in different screen readers. For Windows narrator it needs tabindex. Without it I can't select element on the page. Once it has tabindex then the announcement is very good.
--vaadin-progress-value
Other notes: