Arvados

Hi,

This would be an important improvement for us. We are frequently getting strong feedback about the the perceived slowness of the workbench and this would be a big improvement. So please prioritise this if possible.

Moritz Gilsdorf wrote in #note-7:

Hi,

This would be an important improvement for us. We are frequently getting strong feedback about the the perceived slowness of the workbench and this would be a big improvement. So please prioritise this if possible.

Hi Moritz,

We have some pretty significant workbench 2 performance improvements pending in #22127.

The idea with skeletons is that the elements of the control are rendered as either horizontal bars (for text), boxes or circles (for images). These elements may get a subtle throbbing effect to indicate something is happening.

One point that comes to mind -- when rendering skeleton components we don't know exactly how many items there will be, e.g. the data table could have 3 items or 13, or the breadcrumbs can be several levels deep. However, we want to minimize flicker.

Tentatively, I think it should work like this:

• If the component is loading for the first time, use an arbitrary small number (like 3)
• If the component has been loaded before, use the previous number of items -- this way, the items that were already there are replaced by skeleton components, then replaced by new content. The idea is we don't show the old content while loading (which is confusing) but we don't completely blank it out (which creates flicker).

I'm open to other suggestions, though.

Is it possible to avoid resizing the columns in the transition from the previous content to the skeleton content? The column headers still jump around noticeably and that should only happen once, when the new content arrives.

There are a couple less than ideal solutions for that:

• We could try to make the skeleton columns line up better with the expected content for columns that are known width (like the checkbox column), but since most columns adapt to the content there would still be a lot of shifting
• In the scenario where content was already loaded like when refreshing, it would be possible to keep the contents rendered but overlaid with the skeleton, but then the skeleton table cells would be out of the table layout and not line up, and that would complicate the initial loading since it would require extra logic to insert the skeleton into the table and have the headers align based on the skeleton in the special case of the first load.

I might be restating the second bullet, but what about this:

Would it be possible, when the table already has content:

1. In each cell, the original content remains for layout purposes, but isn't rendered (visibility: hidden;)
2. Render the pulser on top of it -- I think the tricky part is getting the width correct
3. Since this would affect all the table rows, it naturally adapts to the number of rows

You mention that it would complicate initial loading, how does the initial load work right now?

That sounds doable - the initial loading case will just need a check for existing content so that's not as bad as creating a duplicate loading screen just for the initial load

Another thing just came to mind: rows in the loading state need to suppress interaction, so make sure nothing happens on right/left mouse click.

Stephen Smith wrote in #note-31:

Here's my attempt to use overlaid skeleton loading using absolutely positioned skeleton loaders and flexbox - this solves column header / layout shifting with the only caveat being that wrapped content doesn't get reflected in the skeleton loader, it just shows a single line skeleton vertically centered in the available space which I think looks good enough.

I like this a lot. I think wrapping multiple lines would look worse than the way you have it with a single line vertically centered, so I think your solution is already the best option. This definitely hits the right balance for me. +1

Updated branch 22360-skeleton-loader with changes, it should be pretty smooth now

• Only thing I think missing is disabling row interaction / context menu and toolbar.
• There is also a slight delay in the skeleton showing when navigating between project panel locations, I'm investigating making that snappier but it could be a lot more work since it may involve setting the loading flag earlier / outside of the DE middleware - it seems to be snappier with other panels besides project panel
• The way it currently works, any DE transition will use the stale content for layout if content was loaded into that specific DE previously, so if you load all processes, away and then back, it will use the old number of all processes rows.
• Navigating to a panel for the first time uses 3 dummy rows.
• Because all project panel locations share DEs, navigating between 2 project folders will use the "from" folder content as the scaffold since it doesn't cache the "destination" content, but since the content would be changing from x to y rows anyway I figure it probably doesn't matter that it uses x rows instead of y for the skeleton.

Stephen Smith wrote in #note-33:

Updated branch 22360-skeleton-loader with changes, it should be pretty smooth now

Some notes:

• Only thing I think missing is disabling row interaction / context menu and toolbar.

FWIW I think I would prefer the toolbar stay put but don't do anything (or maybe goes to half opacity) than to go away entirely only to reappear a second later. Although if something in the list was selected and we're loading new list contents, it should be clearing the selection anyway.

Changes at arvados|2f356a2a3510d8130a9c95810ccd0b70b81af464 branch 22360-skeleton-loader
Tests developer-run-tests-services-workbench2: #1523

• All agreed upon points are implemented / addressed.
  • Adds skeleton loaders into table cells when loading
  • Renders 3 placeholder skeleton loader rows when DE has no content during loading
  • Changed DataTable to use DE Middleware internal working flag instead of linking to nightrider
    • Working state is managed by the DE Middleware state machine, only set when background=false
  • Disabled row interaction / context and clear MS selection when working
  • Sets DE working early in project panel actions to avoid delay in showing skeleton (showing old content when switching to project panel)
  • Also added a test I forgot to commit from 22394-project-tab-preference
• Anything not implemented (discovered or discussed during work) has a follow-up story.
  • none
• Code is tested and passing, both automated and manual, what manual testing was done is described
  • Passes existing tests
  • Tested each panel including subprocess
  • Tested background refresh with all processes - process state updates live without triggering skeleton
• Documentation has been updated.
  • n/a
• Behaves appropriately at the intended scale (describe intended scale).
  • no change
• Considered backwards and forwards compatibility issues between client and server.
  • no change
• Follows our coding standards and GUI style guidelines.
  • done

This LGTM, but you can implement the random number of rows to skeleton when initially loading if you want.