Loading indicators - Githubissues

rishson commented 7 years ago

I think we should leave it up to widget authors to provide placeholder content if they wish.

There are times when a spinner may be useful. Should we provide the ability to include a spinner in a generic way, e.g. after time has elapsed: set a spinner on a widget, where we could offer things like:

spinner calculates the widget dimensions in order to position itself in the centre of the widget
the user does not have include the spinner icon explicitly
the user could select from a range of spinner animations/icons
the spinner provides optional fade in/out animations
the spinner provides an optional dimmer (darkened overlay on widget content)

rishson commented 7 years ago

@smhigley @bitpshr @tomdye using our new triage system, the process here should be:

as a group, evaluate if this is needed
if not needed, issue will be closed
if needed, a related issue will be created with a widget spec (or similar) and moved to backlog, and this issue will be linked to the spec issue and then this issue will be closed

bitpshr commented 7 years ago

I think it would be difficult to apply a spinner to components in a generic way while still being aesthetically pleasing. I can see the idea there and agree that it would be useful, but I'm unsure how we'd actually implement something without making overreaching assumptions about our components.

I originally thought a LoadingIndicator component would be useful, but every time I tried to write out a basic properties interface, I struggled to think of anything worthwhile. I'm not sure there's a ton of value add in a component for this that couldn't also be provided by a simple GIF or something similar.

rishson commented 7 years ago

Thanks for the comments. Just to clarify what I had in mind here.

The loader would be 'generic' across widgets/elements that it is attached to, only in that it would take up all of their real estate.

which would give us a properties interface of something like:

loadingText: string,
darken: boolean,
spinner: SpinnerType,
animateFadeIn: boolean,
animateFadeOut: boolean

Not trying to justify this widget's existence - just wanted to clarify what was in my head.

smhigley commented 7 years ago

I'm inclined to leave the actual loading indicator up to widget authors, but I could see adding some sort of standard loading property that triggers a overridable .loading class on certain widgets (inputs, buttons, container-type elements, anything else?)

rishson commented 7 years ago

OK. 2 👎 so far. It's not looking good for loadingIndicator 😄

One last question. If a widget author decided in render that they should display a loader because the props didn't contain enough data for meaningful display, then if they wanted to they could simply add a loading class to the root DNode and remove in subsequent renders. However, if the user wanted an overlay style loader such as the one in the screenshot above, then is it possible to achieve this effect (centered text, icon spinner, dark overlay) without having to create additional elements?

smhigley commented 7 years ago

@rishson ~to have three elements (text, spinner, overlay) you'd need one additional node. Any two of those could be done without additional elements, though~. I sneaky-edit take that back. You could do that with no additional elements.

rishson commented 7 years ago

So following on from that, the value of the loader widget would be that you don't have to add identical extra element, element classes and class toggle code to each widget that you want to have loader behaviour on. It would be a 'convenience widget' at best I guess.

rishson commented 7 years ago

@smhigley So if we kill Loader as a widget, should/could we offer something in our base CSS that is a loader class that you can add to a widget root element if you want a dark overlay, spinner and text?

smhigley commented 7 years ago

@rishson yup, I think we could add something to common/styles for a loader, and then maybe let it be passed in with extraClasses? @tomdye can chime in if he has a better idea for now to make that available to widgets.

sebilasse commented 7 years ago

Just wanted to mention that my Semantic UI dojo2 Integration also contains Dimmer and Loader, posted a WIP preview here : https://github.com/dojo/widget-core/issues/559

tomdye commented 7 years ago

@smhigley late to the party here but if we're going with a simple widget for icon with required aria flags then i imagine we should do the same for loading indicator.

eheasley commented 7 years ago

We discussed this and decided this is no longer an issue. We can reopen it in the future if we get a feature request specific to this functionality.

dojo / widgets

Loading indicators #98