Bug #18661
closed
Refresh Button makes the project's listing to flicker while it loads 2 times in a row
Added by Lucas Di Pentima almost 3 years ago.
Updated almost 3 years ago.
Release relationship:
Auto
Description
Once #18207 was merged, changing projects cleared the contents before populating it with the new data, and this a desirable behavior. Now that this listing clearing is in place, it has been made evident that the "Refresh" button is making Workbench2 to do extra work, seemingly loading the project twice because the listing flickers.
IIRC, the Refresh button requested the app to go back&forward the router's history, and this seemed like a good way to implement a generic reload feature instead of having the button signaling all the active parts of the app at any given time, but if it's making the app do extra requests and local work, it doesn't seem ideal.
Check how to get the same effect without simulating a going back in the app's router history.
- Status changed from New to In Progress
Updates at arvados-workbench2|bdb36daa - branch 18661-refresh-flicker-fix
Test run: developer-tests-workbench2: #573 
The "Refresh" button doesn't go back & forth in the history, but just replaces the current location and that is a good solution. The double-loading behavior was recently introduced in #18195 but wasn't evident because the project's contents weren't cleared.
This one-line fix just avoids re-reloading the project because history.replace() already takes care of that.
Lucas Di Pentima wrote:
Updates at arvados-workbench2|bdb36daa - branch 18661-refresh-flicker-fix
Test run: developer-tests-workbench2: #573
The "Refresh" button doesn't go back & forth in the history, but just replaces the current location and that is a good solution. The double-loading behavior was recently introduced in #18195 but wasn't evident because the project's contents weren't cleared.
This one-line fix just avoids re-reloading the project because history.replace() already takes care of that.
LGTM, that was a short patch!
The related test failed, so the fix turned out not to be so simple.
I've been looking at the requests made when reloading projects, and it seems that the duplication indeed comes from the code that I removed on my last update but somehow is tightly coupled with the left side tree panel update code. I'm trying to see how to decouple both behaviors to avoid extra work and correctly refresh the left side panel at the same time.
Also, while reviewing the code for this, I've found some clues on why #18315 is happening. The issue has two parts: the main collection panel refresh issue is super easy to fix, but the collection files panel refresh issue may need more work, as I've seen traces of old code not being used anymore (from the old file browser component era).
- Target version changed from 2022-02-02 sprint to 2022-02-16 sprint
Updates at arvados-workbench2|606119d
Test run: 
- Fixes failing test by reordering action dispatching to make sure the app's store has fresh data before the side panel tree is rendered.
- Improves test reliability by avoiding a potential race condition.
- Fixes some unrelated HTML error on the
DataTable component.
Updates at arvados-workbench2|1313e9c9
Test run: 
Improves project loading further to immediately clear the table's contents upon load.
This is an additional improvement on #18207 and will probably be a good solution for #16951.
Lucas Di Pentima wrote:
Updates at arvados-workbench2|1313e9c9
Test run:
Improves project loading further to immediately clear the table's contents upon load.
This is an additional improvement on #18207 and will probably be a good solution for #16951.
Just one small comment: in src/store/workbench/workbench-actions.ts on line 447, a console.log call is added, is that leftover from debugging?
Otherwise, this LGTM, the difference when hitting "Refresh" is quite noticable, nicely done!
- % Done changed from 0 to 100
- Status changed from In Progress to Resolved
- Related to Bug #18769: Test & confirm no stale data or flickering during data table refreshes added
Also available in: Atom
PDF
Updated by 
Updated by 
Updated by