Bug #18661
closed
Refresh Button makes the project's listing to flicker while it loads 2 times in a row
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.
Updated by Lucas Di Pentima almost 3 years ago
- Status changed from New to In Progress
Updated by Lucas Di Pentima almost 3 years ago
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.
Updated by Ward Vandewege almost 3 years ago
Lucas Di Pentima wrote:
Updates at arvados-workbench2|bdb36daa - branch
18661-refresh-flicker-fix
Test run: developer-tests-workbench2: #573The "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!
Updated by Lucas Di Pentima almost 3 years ago
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).
Updated by Peter Amstutz almost 3 years ago
- Target version changed from 2022-02-02 sprint to 2022-02-16 sprint
Updated by Lucas Di Pentima almost 3 years ago
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
DataTablecomponent.
Updated by Lucas Di Pentima almost 3 years ago
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.
Updated by Ward Vandewege almost 3 years ago
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!
Updated by Lucas Di Pentima almost 3 years ago
- % Done changed from 0 to 100
- Status changed from In Progress to Resolved
Applied in changeset arvados-workbench2|d861bd54089e9279cd03b2e4561869ee877b9559.
Updated by Peter Amstutz almost 3 years ago
- Related to Bug #18769: Test & confirm no stale data or flickering during data table refreshes added