Skip to content
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

Custom loading indicator for long server requests #3763

Open
jouni opened this issue Mar 26, 2018 · 17 comments
Open

Custom loading indicator for long server requests #3763

jouni opened this issue Mar 26, 2018 · 17 comments

Comments

@jouni
Copy link
Member

jouni commented Mar 26, 2018

When a server request takes a long time I want to display a custom loading indicator so that the end user knows that the app is still processing the request.

I also want the designer in my team to be in full control of how that custom loading indicator behaves.

Solution proposal

Apply a state attribute to the <body> element when a server request is active, f.e. <body flow-request-active> (name needs bikeshedding), without any delays.

  • Question: are there different kinds of request?

Extract the current v-loading-indicator implementation into a separate deprecated component that can be added back to the app as an opt-in feature, f.e. myapp.add(new LegacyLoadingIndicator());. This would allow existing custom theme CSS to still work. The LoadingIndicatorConfiguration could be moved inside this component.

Optionally, design and implement a new opt-in loading indicator component specifically for the Lumo theme.

Write a tutorial how to create a custom loading indicator.

@pleku pleku added this to the During 2018 candidates milestone Mar 26, 2018
@Legioth
Copy link
Member

Legioth commented Mar 26, 2018

A CSS based solution can surely be triggered by adding and removing a body attribute, but what if I want to also trigger some JS logic for something related to the progress indicator?

Question: are there different kinds of request?

In Flow, I think there's currently only one type of request. Vaadin 7 and 8 also has the concept of background requests that are completely transparent to the user (e.g. prefetching items to populate the client-side cache in Grid), and would therefore not cause anything to show.

@jouni
Copy link
Member Author

jouni commented Mar 26, 2018

A CSS based solution can surely be triggered by adding and removing a body attribute, but what if I want to also trigger some JS logic for something related to the progress indicator?

Can’t you add a MutationObserver to the body element in your custom loading indicator element?

@manolo
Copy link
Member

manolo commented Mar 28, 2018

I see easier, an annotation for defining what should be the localName of the loading indicator. Hence the logic about when to add/remove the indicator from body is still responsability of the framework. Developer responsibility is to import the appropriate custom element, which might have any logic you need.

My idea is something like

@LoadingIndicatorHTML("<div class='v-loading-indicator'>'")`  // default implementation
@LoadingIndicatorHTML("<vaadin-progress-bar theme='foo' ></vaadin-progress-bar>")`
@LoadingIndicatorHTML("<paper-progress class='bar'><paper-progress>")`
or
@LoadingIndicatorTagName("div")
@LoadingIndicatorTagName("vaadin-progress-bar")
@LoadingIndicatorTagName("paper-progress")

First seems simpler to implement, and probably more customisable.

Lumo might have some css for div.v-loading-indicator that works out-of-the-box if the user does not want to customise anything.

@jouni
Copy link
Member Author

jouni commented Mar 28, 2018

@manolo, I think an annotation is unnecessarily complex. In my opinion, Flow should only toggle the attribute on the body element, and that’s it. All other API would be encapsulated in a “web component” that you place directly under the body element (like I described in the issue description).

By ”web component” I mean a simple custom element (f.e. <my-loading-indicator>) that adds CSS to the global scope and reacts to the attribute on the body. For example:

my-loading-indicator {
  opacity: 0;
  pointer-events: none;
  ...
}

[flow-request-active] my-loading-indicator {
  opacity: 1;
  animation-name: my-loading-indicator;
  animation-duration: 1s;
  animation-delay: 0.5s;
  animation-iteration-count: infinite;
  ...
}

Global CSS is arguably a bad practice. A MutationObserver, which toggles an attribute on the custom element host, might be a better option.

@jouni
Copy link
Member Author

jouni commented Mar 28, 2018

Having the attribute on the body element allows the developer to use that information in more custom ways than just one element that Flow adds/removes to/from the DOM. So it feels like a more low level, generic API in that way, not tied to an element specifically that you can only configure with an annotation.

@manolo
Copy link
Member

manolo commented Mar 29, 2018

Another option might be that the attribute is set to the active UI component in the app instead to the body, in this way developer would have more control in their app.

@manolo
Copy link
Member

manolo commented Mar 29, 2018

I don't have a strong preference about the implementation, whether the attribute is set to body or UI, but I think loading should be shown out-of-the-box like happens in FW8. The user just need to set Lumo theme

@jouni
Copy link
Member Author

jouni commented Mar 29, 2018

I think loading should be shown out-of-the-box like happens in FW8

I don’t completely agree.

I can agree that it’s probably better than nothing.

But, in my view, the default loading indicator (the dummy progress bar at the top of the viewport) is an easy excuse for the designers and developer to not implement better feedback mechanisms to the end user (like instantly updating the UI with a skeleton screen and then progressively loading the content).

That said, it might be better to have the default loading indicator as opt-out instead of opt-in.

My suggestion to have it opt-in was because I wanted to get rid of the current default indicator ASAP without having to design and implement a replacement at the same time. But I suppose we can keep the current default loading indicator as long as we have a new one designed and implemented, and then deprecate the old one somehow.

@jouni
Copy link
Member Author

jouni commented Mar 29, 2018

Another option might be that the attribute is set to the active UI component in the app instead to the body

I thought the body element corresponds to the UI component.

@jouni
Copy link
Member Author

jouni commented Mar 29, 2018

Slightly related: when the view Flow is trying to render is complex enough, the UX degrades somewhat.

As an end user, I click a link to move to another view. The server round-trip takes enough time for the loading indicator to kick in (~300ms) but not enough time for the loading indicator to really animate and show any amount of progress. It either doesn’t show at all or just halts right in the beginning. Then Flow starts to process the response and render the UI update, which blocks rendering. Once the rendering finishes, the loading indicator also flashes from 0 -> 100 immediately.

Not sure how to mitigate that, but it doesn’t feel like great UX.

@mstahv
Copy link
Member

mstahv commented May 23, 2018

I strongly agree with @manolo. The request may take longish for several reasons and in certain cases only occasionally. We certainly don't want to force developers to add custom css for each and every app. In certain cases developers definitely should show a progress indicator with meaningful text what is being done, but if the request is slow due to a temporary network outage, there is no way how Java developers could show a progress indicator.

@jouni
Copy link
Member Author

jouni commented May 23, 2018

We certainly don't want to force developers to add custom css for each and every app.

I can also agree with that, very much (that’s one reason I created this issue).

The implementation we have in Framework 8 and before is very rigid and unnecessarily configured from the server. And the progress bar implementation is ugly, hacky CSS. I would’ve have liked us to use this new major version as an opportunity to improve on this. But I suppose it’ll wait until the next major version then. If that’s the case, then I suppose we can just as well throw in the old messy CSS as well by default. But then we need a tutorial how to get rid of that.

In my view, it would be better to encapsulate the default loading indicator to a component that can be removed from the UI is wanted (opt-out) and have lower level hooks (like the <body> attribute) that allow developers to create their own loading indicator if needed. You kind of described the need, that in some cases there could be a need for a customized progress indicator.

@mstahv
Copy link
Member

mstahv commented May 23, 2018

Better (both from looks and technical PoV) would of course be better, here any in many other places. An ugly one (by default) that can be overridden if needed is much better than nothing.

@jouni
Copy link
Member Author

jouni commented May 24, 2018

Yes, won’t argue against that either. I never intended to block us from having a default loading indicator. Just hoped we would’ve had time for ”better” before V10 😅

@pleku
Copy link
Contributor

pleku commented May 25, 2018

The same indicator as in Vaadin 8 has been brought back in #4174 and is by default on. It can be opt-out if necessary from the LoadingIndicatorConfiguration (which it also fixes to work).

@Braus
Copy link
Contributor

Braus commented May 21, 2019

Is still issue still active?, I see there hasn't been any activity for a long time, so I was just wondering.

@pleku
Copy link
Contributor

pleku commented May 24, 2019

There is plans on the roadmap for changing the current loading indicator behavior
https://vaadin.com/docs/v13/flow/advanced/tutorial-loading-indicator.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

6 participants