Improve progress bars

[chlebowa]

Related to #1234.

Update progress bars to improve user experience.

• modules_datasets receives optional progress argument that accepts a Progress object
• ui_nested_tabs and ui_tabs_with_filters receive progress argument that accepts a Progress object
• rewrote progress bar management in srv_teal

Added a large app for testing in __stress_test.R, to be removed before merging.

NOTE

Once the module UI is prepared, there is still a delay before the app starts, so the progress bar can be improved further. So far I was unable to identify the reason for the delay.

[m7pr]

Hey @chlebowa thanks for the input! I think it's gonna be highly useful!

I allowed CI check to be executed on this PR. I see a Warning in R CMD Check - would you be able to check in your free time?

[chlebowa]

Thanks @m7pr, I had forgotten to re-create documentation after removing the faulty defaults.

[chlebowa]

There is one more failure, I think this is something on your end.

[m7pr]

Yeah, this is something @walkowif is investing on our end

[walkowif]

The urlchecker failure should be gone after retriggering the checks.

[m7pr]

Thank you @chlebowa for the time and energy you invested to provide this feature. I just test in on the branched cloned from your fork. Works like a charm! I think we are good to merge this excellent feature, once we figure out the details from the comments.

@kumamiao would you mind providing a NEWS entry for this feature?

video1729290070.mp4

[chlebowa]

I had a thought this morning that may be a slightly more orderly implementation. Rather than creating tro Progress objects, one in a reactive expression and one in an observer, instantiate one Progress in srv_teal and work with it throughout the function by first passing it to modules_datasets and later reset the progress and pass to ui_tabs_with_filters.

The user would see only one progress bar, whereas now two bars can be seen, the filter one is moved up and later disappears as the module one appears. On the code seide, there would be a little less clutter.

What do you think?

[m7pr]

If one waits for the other, then it's probably better for the end user to see just one progress bar at a time :)
Depending on how much time you have. This is already in good condition

[chlebowa]

Have a look.

[m7pr]

Wow @chlebowa nice and clean. Really ready to be merged!

Can you

• delete __stres_test.R file
• extend NEWS.md with an Enhancement * Provided progress bar for modules loading and data filtering during teal app startup.

Then I will merge. Thank you. Really great job!

[m7pr]

really useful stuff!

[pawelru]

Hi all. Apologies for jumping into the old PR - just got back from the break of mine. May I ask for a follow-up PR before the release that will make progress argument optional for all modified functions? The rationale here is simplicity - we would like to keep the requirements to run the funcs to the meaningful minimum which is in these case: id, modules and datasets. If you look at the functions definitions all the other arguments (incl. filter which is also quite important) has some defaults. In case of progress this could be NULL and if it's Progress we will increment it. Another benefits are in testing - no need to create a dummy progress object to execute functions.

(*) I know the is.missing() stuff but IMHO having x = NULL in the function definition exposes this better without reading docs or function body

[m7pr]

@pawelru I created a ticket for that in here #1243