[Feat] Add new dcc.Loading features in dash 2.17

[AnnMarieW]

Description

Closes #486

This PR is a quick demo of a couple new features in dcc.Loading available in dash 2.17. This is just to make it easier to see how it it might work in Vizro apps.

It adds

• delay_show=500 to reduce the flickering on short loading times
• overlay_style={"visibility": "visible", "filter": "blur(2px)"} as an example of styling the background.

I'm interested to hear what you think!

Notice

• I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

  • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
  • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorized to submit this contribution on behalf of the original creator(s) or their licensees.
  • I certify that the use of this contribution as authorized by the Apache 2.0 license does not violate the intellectual property rights of anyone else.
  • I have not referenced individuals, products or companies in any commits, directly or indirectly.
  • I have not added data or restricted code in any commits, directly or indirectly.

[huong-li-nguyen]

First of all, congratulations @AnnMarieW on getting these features into Dash! 🚀 ⭐ From a visual perspective, I think they can improve the loading spinners significantly.

I left a couple of comments as one big visual advantage of the new loading spinner feature doesn't really come across in Vizro. That's an issue on our side though. I'll double-check with @antonymilne if this might change in the future.

The other visual advantage of reducing the flickering - I would love to understand better as from the current Vizro demo examples it seems to flicker a bit more, but I think it's because we need to fine-tune the ms for delay_show?

[antonymilne]

Hello @AnnMarieW! Thanks very much for making the Github issue and raising this PR. And many congratulations on plotly/dash#2760! It's a really amazing PR ⭐

@huong-li-nguyen Following on from the slack conversation and just to make sure we're all on the same page here, because I had to remind myself of some previous work I'd done to stop graphs flickering (#166) and what our current scheme is... There's currently two times when dcc.Loading is relevant:

1. when on-page-load action replaces the empty graph (or full datatable/AgGrid as @petar-qb explained) with the "real" one containing filtered data
2. when you change some parameter/filter data on the screen

So even though overlay_style might not make any difference to graphs in case 1, it may well do so in case 2 if the filter/parameter takes a long time to apply (we can add some time.sleep in to try it out) so I think is probably valuable already.

delay_show I think should also improve things already (again, for non-graph in case 1 and for graph/table/grid components in case 2). So if it's somehow making flickering worse that would be good to understand what's happening.

Just for the record, some other things that might be useful from plotly/dash#2760 which @AnnMarieW didn't change here would be:

1. the target_components property, if you can remember why that might have been useful to us (I can't) - see our conversation with Ann Marie on dcc.Loading PR on the plotly forums!
2. display="show"/"hide" might help us get rid of on-page-load as per [Bug] Fix bug that prevented custom tables/grids with column referral #439, but this is a longer term thing

[huong-li-nguyen]

@antonymilne, it's good that you mentioned it. The second case was not on my mind anymore 😄 In that case, the overlay_style does work for us!

On delay_show / delay_hide, I would love to get @AnnMarieW's opinion on this. I find it tricky to find the sweet spot of time that works well for us. See PR comments. But maybe some other advantages are not visually visible here?

I've looked into our previous discussion before the PR review and also thought that target_components looks promising! However, I think we always return the entire fig whenever we trigger any dashboard interaction, and we don't know beforehand what users might want to encode as a trigger for custom actions so triggering the dcc.Loading component on any update on the fig is probably the safest. That's why I am not sure if we could leverage this partial triggering of the dcc.Loading component with our current system of actions. @petar-qb—wdyt?

[AnnMarieW]

And regarding the target_components prop - I agree, I don't think it's applicable with Vizro.

[huong-li-nguyen]

Defining delay_show and delay_hide now works nicely for our demos - thanks @AnnMarieW! EDIT: @petar-qb noticed some unwanted behaviour on some other occasions though, so it might be worth removing these two properties for now, but you can definitely go ahead with the changes suggested in the overlay_style :)

Approved from my side :) Could you add the opacity in as suggested? It's not visible in the demos, but it will be whenever a filter/parameter takes a long time!

The other potential remaining things:

• Fix unit tests - I think you just need to bring over your changes to the unit tests as well, as the components are tested for equality.
• Ignore all of the vizro-ai related tests (they currently don't run through, I'll take them out for your PR to merge, whenever it's ready 👍 )

[petar-qb]

First of all, @AnnMarieW congrats for the great Dash contribution, and also a big thank you for the Vizro contribution ❤️.

Regarding this PR, I like the overlay_style={"visibility": "visible", "opacity": 0.3}, as the default Vizro figure configuration and really think it has a place in our codebase.

Also, I've tried a lot of delay_show/hide timings combinations and it's quite difficult to find the right timings given we don't know how long the callback will take to run.

So as I understood, these are the goals we want to achieve by utilising the delay_show/hide:

1. To avoid a figure flickering when the answer comes "immediately" from the server (by utilising the delay_show prop),
2. To avoid a loading-icon flickering when the answer comes "immediately" after the delay_show time passes (by utilising the delay_hide prop).

So, if delay_show=500, delay_hide=500 props are set and server response time is:

• up to 500 ms:

  • The initial loading page "triggers" delay_hide and users wait additionally 500 ms to get fully visible response. It means that the final (fully filtered and parametrised) response is visible to the user for additionally 500 ms but it is in blurred mode. So it's visible, but it seems like some calculations are still happening (but they don't).
  • There is no loading icon when filter/parameter are triggered. This is maybe something we want to achieve.

• from 500 ms and up:

  • The on_page_load behaves same as described above (in the "up to 500 ms" section)
  • When filtering process is triggered, the delay_show=500 shows up. It means that after user's filter value selection, only after 500ms the loading icon will appear. By my opinion this looks buggy, and looks like frontend part of the app isn't optimised properly.
  • Also, the final (fully filtered) response is potentially (if server response comes exactly after delay_show passes) visible for additional 500 ms (because of delay_hide) but in blurred mode.

So, the same effect is keep popping up even if I change delay_show/hide number of milliseconds.
Because of everything listed above, my opinion is that we shouldn't hardcode the delay_show/hide (because we can't foresee server response time for all the apps created using Vizro) and should let our users to do it on themself (by creating the custom component) with the desired number of delay milliseconds.

What do you think? @AnnMarieW @antonymilne @huong-li-nguyen

[AnnMarieW]

Hi @petar-qb

Yes, you are correct with the objectives of the delay_show and the delay_hide props. The main purpose is to prevent the loading spinner from showing for a short time which can appear as a flash on the screen.

Taking the Viro demo site as an example, currently, the loading spinner is displayed on every user interaction. Since the dataset is small, the spinner is displayed for less than one second in most cases.

Adding delay_show of 500ms makes the spinner show up less than half the time (when I run the app locally). However, if the callback takes slightly longer, let's say 700ms, then the spinner is back to having the flashing effect because it will be visible for 200ms.

Adding the delay_hide can prevent the flash, because you can specify the minimum amount of time to display the spinner once it becomes visible. However, the delay_hide prop looks the best when the underlying components are not visible while loading (the default) to handle the case described above where the component might be updated before the delay_hide time has elapsed.

It is tricky to get right, and it also depends a lot on the app and the preferences of the users. I agree that it might be best to let people add it as a custom component. Would you like me to close this PR now?

[huong-li-nguyen]

Hey @AnnMarieW ,

I've talked with both @antonymilne and @petar-qb. You can still go ahead with the PR and add the overlay_style={"visibility": "visible", "opacity": 0.3} :)

As Petar said, let's not define delay_show/delay_hide for now 👍

[huong-li-nguyen]

There is one place where you would have to additionally change the dependency for the integration tests to pass:

https://github.com/mckinsey/vizro/blob/main/vizro-core/hatch.toml#L91

Just change it to dash==2.17.0 on that line :)

[antonymilne]

Thank you very much for this @AnnMarieW and for all the careful reviewing @huong-li-nguyen and @petar-qb 🙏

[AnnMarieW]

Hey everyone,

I think adding the opacity is a nice addition and will make the transition between "loading" and "ready" smoother. However, there is still a flicker when the loading time is very short.

Could you please try one more thing before closing this? I just tried setting the delay_show=200 and I think it looks great! Check out the demo on the Benchmark Analysis page where you click on the grid and it updates the graph. This setting completely eliminates the flicker. And 200ms short enough that I don't think it will have the same problem of shifting the flicker as noted earlier.

And of course if you prefer not to add this, I'm still delighted about the addition of the new loading overlay style 🙂

[petar-qb]

Hey everyone,

I think adding the opacity is a nice addition and will make the transition between "loading" and "ready" smoother. However, there is still a flicker when the loading time is very short.

Could you please try one more thing before closing this? I just tried setting the delay_show=200 and I think it looks great! Check out the demo on the Benchmark Analysis page where you click on the grid and it updates the graph. This setting completely eliminates the flicker. And 200ms short enough that I don't think it will have the same problem of shifting the flicker as noted earlier.

And of course if you prefer not to add this, I'm still delighted about the addition of the new loading overlay style 🙂

Hey @AnnMarieW.

I tried it out and got that the "initial page load" flickers equally as it flickers without setting the delay_show=200. Also, there is no loading icon (and no flickering) while applying filters/parameters. However, it works better only when you run the Vizro demo app locally, and we can't guarantee how all other users examples will work. So, I put time.sleep(0.15) inside the filtering method and found that the app flickered even more.

Since we don't know the performance of users apps built using Vizro (since it depends on many factors), I wouldn't hardcode any delay, and just push the overlay argument.

[petar-qb]

@AnnMarieW thanks again for your contribution! 🥇

I pushed some unit tests to make the CI 🟢 and the PR is now ready to go.