Project

General

Profile

Actions

Bug #21077

closed

Background refresh should not trigger the spinner

Added by Peter Amstutz 5 months ago. Updated 4 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Workbench2
Story points:
0.5
Release relationship:
Auto

Description

When there is a background refresh trigger by websockets, it should not trigger the spinner at the top of the window, it is distracting to users.


Subtasks 1 (0 open1 closed)

Task #21130: Review 21077-background-refreshResolvedStephen Smith10/19/2023Actions

Related issues

Related to Arvados - Bug #20526: Sorting should break ties consistently (stable sort order)ResolvedPeter Amstutz10/19/2023Actions
Actions #1

Updated by Peter Amstutz 5 months ago

  • Story points set to 0.5
  • Description updated (diff)
Actions #2

Updated by Peter Amstutz 5 months ago

  • Related to Bug #20526: Sorting should break ties consistently (stable sort order) added
Actions #4

Updated by Peter Amstutz 5 months ago

  • Release set to 67
Actions #5

Updated by Peter Amstutz 5 months ago

  • Target version changed from Development 2023-11-08 sprint to Development 2023-10-25 sprint
Actions #6

Updated by Peter Amstutz 5 months ago

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.

Actions #7

Updated by Peter Amstutz 5 months ago

  • Assigned To set to Peter Amstutz
  • Status changed from New to In Progress
Actions #8

Updated by Stephen Smith 4 months ago

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!

Actions #9

Updated by Peter Amstutz 4 months ago

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.

Actions #10

Updated by Peter Amstutz 4 months ago

  • Status changed from In Progress to Resolved
Actions

Also available in: Atom PDF