Arvados

21077-background-refresh @ arvados-workbench2|20798b41fd3a5c012141391403d7caf066f46086

<https://ci.arvados.org/... (link to developer test job on jenkins)>

• All agreed upon points are implemented / addressed.
  • yes
• Anything not implemented (discovered or discussed during work) has a follow-up story.
  • n/a
• Code is tested and passing, both automated and manual, what manual testing was done is described
  • passed integration tests locally
• Documentation has been updated.
  • n/a
• Behaves appropriately at the intended scale (describe intended scale).
  • n/a
• Considered backwards and forwards compatibility issues between client and server.
  • n/a
• Follows our coding standards and GUI style guidelines.
  • yes

The motivation for this ticket is that when a websocket event triggers a refresh of the current panel, the panel should be refreshed quietly in the background without displaying the spinner. This is specifically in response to user support requests indicating that they were confused why the spinner was going when they had not done anything and perceived the system as being slow because the page was "loading forever".

Previously, the progressFn callback in src/index.tsx meant that any time there was an active API request, it would spin the progress bar. To allow for background requests (where we don't want it to spin because it is distracting/confusing to the user), we need to change the default behavior.

My approach was to get rid of the default spinner entirely. The alternative would be an additional flag similar to showErrors on the get() and list() methods of CommonService. I didn't go that way because it seemed more disruptive to have to propagate a new background flag to everywhere it might be needed.

As a result, to provide feedback that something is happening, functions that do loading or other operations need to explicitly set START_WORKING and STOP_WORKING. This existed but was implemented inconsistently, because the default processFn behavior tended to cover a lot of cases.

This branch adds START/STOP to all the major panel loading functions in the UI. This provides better user feedback overall, because the spinner now more consistently covers the entire loading process (across multiple API calls and async behavior), instead of just individual API calls.

One question - it sounds like you didn't go the way of adding a background flag but your first pass commit adds a background flag to a few requestItems - not sure if that's intentional?

Besides that it looks good!

Stephen Smith wrote in #note-8:

One question - it sounds like you didn't go the way of adding a background flag but your first pass commit adds a background flag to a few requestItems - not sure if that's intentional?

Besides that it looks good!

Yes, that's different. That's on the REQUEST_ITEMS redux action, because the sender still needs to indicate if it is a background refresh or not. The place I didn't want to add a "background" flag was on the underlying get() / list() methods of CommonServices, because those are called everywhere. So the solution was that API calls no longer start the spinner by default (all API calls are "background") but to add explicit START/STOP WORKING actions around all the higher-level loading behaviors.