Page MenuHomePhabricator

Newcomer tasks: show skeleton screen while loading
Closed, ResolvedPublic

Description

This task is about adding a skeleton card to the Suggested edits module while tasks are being loaded. The expected basic behaviour will be:

Desktop

  1. Suggested edits module loads at the height that it will be when all elements are loaded.
  2. Pager text will show as 1 of ... suggestions whilst the total number of tasks is being fetched
  3. The filter bar and skeleton of the article card will load.
  4. The task type, info icon, difficulty level tag, and task type short description text will be shown after the initial skeleton card is loaded
  5. The tasks number will be updated with the correct number of tasks

Mobile

  1. Suggested edits module screen loads with dialog header in place, and the footer in its final position.
  2. Pager text will show as 1 of ... suggestions whilst the total number of tasks is being fetched
  3. The filter bar and skeleton of the article card will load in their final position (there should not be any shift in position when the information loads).
  4. The task type, info icon, difficulty level tag, and task type short description text will be shown after the initial skeleton card is loaded
  5. The tasks number will be updated with the correct number of tasks

Moved to leftovers task T263040

Our team will work to make the suggested edits module and cards load as quickly as possible. Because there may still be some delay, we should also have a skeleton screen to show during the millisecond of loading.

Proposed design:

image.png (840×1 px, 67 KB)

https://app.zeplin.io/project/5bd0b069495b5d0a002e7eb6/screen/5dceca39ec737944ea2120d6
zpl://screen?pid=5bd0b069495b5d0a002e7eb6&sid=5dceca39ec737944ea2120d6

... with CSS animation:
https://codepen.io/reets/pen/wvBwGKE

And as animated gif

expected-skeleton-screen.mov.gif (1×848 px, 677 KB)
open full screen to see animation

Notable aspects:
  1. Pager text will show as 1 of ... suggestions whilst the total number of tasks is being fetched
  2. All elements of the article card will load as a 'skeleton' placeholder grey boxes
  3. The task type, info icon, difficulty level tag, and task type short description text will be shown (fade in) after the initial skeleton article card is loaded (along with filter bar and pager text)
  4. The suggested edits white footer section animates in (moving upwards into position) after all other content is loaded - this removes the joltiness with how this section shifts down currently when it is shown first.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

The first step here is for @RHo to design the skeleton screen.

@MMiller_WMF - I noticed that in V1.0 at the moment when refreshing on mobile there is about a 500ms delay where the screen looks like this:

image.png (1×818 px, 87 KB)

@RHo -- after discussing this with @Catrope and @Tgr, we have decided that we want to prioritize adding loading placeholders over trying to speed up the loading. We think we need an additional placeholder for card navigation. When the user clicks the navigation arrows quickly, what we have is the article titles, task types, and page views -- not the image or the text extract. Can you design something for that? Should it just be what you already posted to the description, but with the things filled in that we know (the article titles and task types)? Or should it be something different? Should the loading state for an image be the gray box (even if it ends up having no image)? Or should it be the placeholder "mountain" image (which may then load into a real image)?

We don't have the page views either (the fix to T238178: Newcomer tasks: pageview count not appearing or inconsistent was to load it per card). We might have images but it's probably better to only rely on the title + task type, we are guaranteed to always have those while other things might come and go as we try to optimize the API.

Per team meeting today, @RHo will specify how this skeleton screen will animate.

@RHo has now included all the assets in the description, and so this is fully ready for development.

@RHo @MMiller_WMF this design works for displaying while the user clicks previous/next (side note: it seems instantaneous to me in production, and even when clicking quickly it doesn't seem bothersome. Is the time between next/previous a problem that we need to solve?)

But as noted in T238231#5666995 the biggest lag is when the page loads, but before the JavaScript has finished loading. For that, I think we need to take a different approach to what the design proposes.

The other modules have server-side rendered content so they appear well before the Suggested Edits module which is all client-side. On initial page load, we can't use the design here because it assumes we know the initial task title and its task type. It also requires us to render the task filters, previous next buttons, and task explanation on the server-side.

If we modified the design such that the placeholder has no title nor task type it's a little simpler because we just render a totally generic placeholder on the server-side. But we would still then need to render the filters + arrows + explanation widget on the server-side. If we updated the design to remove those from the initial page load placeholder, then that simplifies things more.

If we do want to keep task title + task type + filters + arrows + explanation widget as part of the initial page load experience, then we are 90% of the way towards {T236738: Newcomer tasks: server-side rendered version of suggested edits module} which will be a better user experience anyway.

IMO whether rendering happens on the server side or not does not make that much difference. The fundamental problem is that task search is slow; moving it to the server side will make the whole page load slower, which is probably worse. If we cache it or speed it up in some other way, a fast API will be almost as good from a performance POV as server-side rendering. (The task module rendering would have to wait for the ResourceLoader modules to load, and then for the API request, so there would still be a perceivable delay compared to server-side rendering, but nowhere near what we have now.)

IMO whether rendering happens on the server side or not does not make that much difference. The fundamental problem is that task search is slow; moving it to the server side will make the whole page load slower, which is probably worse. If we cache it or speed it up in some other way, a fast API will be almost as good from a performance POV as server-side rendering.

I'm not sure I agree with that, because the API can't be used until we have the JS loaded and executing, at which point it's already too late to make a difference to user perception.

When I load cs.wikipedia.org/wiki/Special:Homepage, the response HTML comes back in 407 ms. The CSS is loaded by the time we reach one second.

The request to load the homepage module JS starts at 1.72 seconds, and it takes 471 ms to come back. Then the API query for fetching tasks starts at 2.78 seconds and takes 691 ms. Let's say we somehow got that API query down to 50ms or 100ms, we would still be looking at ~1.5-2 seconds where the user is looking at the styled and interactive homepage but does not see a card in the suggested edits module.

Looking at the API query on testwiki (modified to use ggtlimit: 1), where we use local config + search, the response time is ~350-500ms. From the profile of the API request a big chunk of that time is API module overhead. The relevant search calls seem to take about ~100ms. So, if in SuggestedEdits.php we execute a query in search to load a single task and render that server-side, I think we'd be adding 100-200ms to the page load time. While that's not nothing, IMHO it will be a better user experience to have an actual task when the page is loaded and styled than to look at a placeholder for ~2 seconds.

All this said... if we accept that it's OK to cache one of the 200 items for some period of time (1 week?), which means that there might be cases where a task has already been completed (maintenance tags removed) but still shows up in the task feed, then we could do the server-side rendered version without adding any noticeable cost to the current server-side rendering time.

@RHo @MMiller_WMF this design works for displaying while the user clicks previous/next (side note: it seems instantaneous to me in production, and even when clicking quickly it doesn't seem bothersome. Is the time between next/previous a problem that we need to solve?)

I thought this was the priority based on the subsequent comment T238231#5691846....

But as noted in T238231#5666995 the biggest lag is when the page loads, but before the JavaScript has finished loading. For that, I think we need to take a different approach to what the design proposes.

> The other modules have server-side rendered content so they appear well before the Suggested Edits module which is all client-side. On initial page load, we can't use the design here because it assumes we know the initial task title and its task type. It also requires us to render the task filters, previous next buttons, and task explanation on the server-side.

If we modified the design such that the placeholder has no title nor task type it's a little simpler because we just render a totally generic placeholder on the server-side. But we would still then need to render the filters + arrows + explanation widget on the server-side. If we updated the design to remove those from the initial page load placeholder, then that simplifies things more.

We can also do this for the initial load if we can get the skeleton to look like the following:

image.png (840×1 px, 73 KB)

Skeleton with no server-side rendered elements (except background color)
CSS animated version: https://codepen.io/reets/pen/bGNeMdL

If we do want to keep task title + task type + filters + arrows + explanation widget as part of the initial page load experience, then we are 90% of the way towards {T236738: Newcomer tasks: server-side rendered version of suggested edits module} which will be a better user experience anyway.

Agree this would be my pref.

kostajh removed RHo as the assignee of this task.Dec 16 2019, 6:35 PM

Change 558550 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] (wip) Suggested Edits: Show skeleton animation while loading

https://gerrit.wikimedia.org/r/558550

Change 558550 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Show skeleton animation while loading

https://gerrit.wikimedia.org/r/558550

Tested on cs.wikipedia.beta.wmflabs.org/ (the issues that I noticed might be specific only to betalabs environment).

(1) My testing is in agreement with what @kostajh wrote in his comments

  • on production the navigation between the articles via arrows feels instantaneous
  • the biggest delay is on the initial load of the Suggested edits module when an empty card is display - the animated gif below shows that when Homepage is reload it takes about 5 sec to see the image on the article card. There is no visual indications that the loading process is still in progress.

suggested_edits_initial_load.gif (696×510 px, 107 KB)

(2) It seems that the specs have been implemented as per CSS animated skeleton screen. There are two twists (they might be betalabs-specific twists, pinging @kostajh )

  • the page views are visible for a moment in the skeleton
  • if the page text extracts are impossible to fetch, the skeleton lines are changed to a blank card. In betalabs, there are two types of cards - the cards that instantaneously will display the text extracts and those who unable to fetch the text extracts and thus they become blank after displaying the skeleton. Below is an animated gif (click to view):

navigation_suggested_edits_betalabs.gif (696×510 px, 91 KB)

The screenshot of barely visible pageview info

Screen Shot 2019-12-23 at 5.31.35 PM.png (569×457 px, 86 KB)

I have added this to be discussed in a team meeting on Jan 6.

the page views are visible for a moment in the skeleton

This is unintentional (and I thought I had checked for it to not occur), I can look at it.

if the page text extracts are impossible to fetch, the skeleton lines are changed to a blank card. In betalabs, there are two types of cards - the cards that instantaneously will display the text extracts and those who unable to fetch the text extracts and thus they become blank after displaying the skeleton. Below is an animated gif (click to view):

That is expected behavior, as far as I understood the requirements. What would you prefer to have happen if the text extracts can't be fetched? Also, in production this scenario should be encountered rarely if at all.

Change 561877 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Adjust task impression log invocation

https://gerrit.wikimedia.org/r/561877

Change 561877 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Adjust task impression log invocation

https://gerrit.wikimedia.org/r/561877

the page views are visible for a moment in the skeleton

This is unintentional (and I thought I had checked for it to not occur), I can look at it.

if the page text extracts are impossible to fetch, the skeleton lines are changed to a blank card. In betalabs, there are two types of cards - the cards that instantaneously will display the text extracts and those who unable to fetch the text extracts and thus they become blank after displaying the skeleton. Below is an animated gif (click to view):

That is expected behavior, as far as I understood the requirements. What would you prefer to have happen if the text extracts can't be fetched? Also, in production this scenario should be encountered rarely if at all.

Checked in wmf.14 testwiki - the issue with briefly displaying "undefined visits" message is still present - to reproduce it switch to Network Slow 3G":

Screen Shot 2020-01-07 at 1.29.36 PM.png (374×473 px, 43 KB)

To answer the second question - the issue of not fetching article extracts - it does not seem to me as a real issue; I didn't find any way to see it in production.

Relevant stills of the current loading behaviour:

image.png (829×3 px, 525 KB)

Video from which the above frames were captured: https://youtu.be/-te4Qxj2uwk

There are still a fair few unexpected issues with how items are shown during and after loading.

  1. There should not be a grey box shown at the bottom of the loading screen at any point
  2. There should not be white background on the loading screen before the pale blue.
  3. The Suggested edits white footer section should not start higher in the screen before shifting into position at the bottom of the screen
  4. Suggested edits header text shifts slightly to the right when the the article card loads. It’s expected to be in place as soon as the text loads.
  5. Pale grey background ‘skeleton’ box for pageviews text should not be longer than the article extract text ‘skeleton’.
  6. It is not expected that the image icon that appears when there is no article image to be shown at all if there is an actual article image.

Based on the current way these issues are appearing, here is a suggestion of how to amend the order and way items load to reduce the jumpiness.

image.png (922×2 px, 343 KB)

@kostajh - can we take 5-10min to review together how much of this could be adjusted?

Based on the current way these issues are appearing, here is a suggestion of how to amend the order and way items load to reduce the jumpiness.

Could I suggest that item 1 be changed to look like item 2, but with loading placeholders for the pager (1 of 345), the task name (egg tart) and and the task type (copyedit)? Then there would be much less visual distraction as the content is loaded into place, because based on current speeds, the transition from the blank blue (in frame 1) to frame 2 is always going to take the longest. See also T238171#5871512

@kostajh - can we take 5-10min to review together how much of this could be adjusted?

Yes, let's find time on Monday!

Changes to this task and skeleton behaviour after discussion with @kostajh :

  1. Pager text will show as 1 of ... suggestions whilst the total number of tasks is being fetched
  2. The article title is also loaded as a 'skeleton' placeholder box along with the rest of the article card content
  3. The task type, info icon, difficulty level tag, and task type short description text will be shown after the initial skeleton card is loaded
  4. The suggested edits white footer section animates in (moving upwards into position) after all other content is loaded - this removes the joltiness with how this section shifts down currently when it is shown first.
This comment was removed by RHo.

Untagging myself for now.

Change 615300 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] WIP: Suggested edits: Show controls and skeleton card while tasks are fetched

https://gerrit.wikimedia.org/r/615300

Moving into code review as there's a relatively small patch that is an improvement on what we currently have, IMHO.

Here's a video to show the difference, simulating a slow API response to load the tasks.

With patch:

Without patch:

Change 615300 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested edits: Show controls and skeleton card while tasks are fetched

https://gerrit.wikimedia.org/r/615300

Change 624039 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Remove setting current card to no results widget

https://gerrit.wikimedia.org/r/624039

Change 624040 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Remove skeleton class from pager after tasks fetched

https://gerrit.wikimedia.org/r/624040

That's great! I love it!

Happy to hear it :) I'm not sure where to put this task now (QA? Leftovers? Ready for Dev?); the patch I polished up and which was merged doesn't conform exactly to the task description which has some additions (animation effects mainly).

That's great! I love it!

Happy to hear it :) I'm not sure where to put this task now (QA? Leftovers? Ready for Dev?); the patch I polished up and which was merged doesn't conform exactly to the task description which has some additions (animation effects mainly).

Thanks @kostajh ! Maybe we could refine this to be about providing the spacing for the filter bar and card to animate in a little more gracefully for Growth-Team-Leftovers? Ie.
a. Automatically set the final height of the SE module to remove the minor 'jump' change in the size of the SE module that is currently occurring when the filter bar appears
b. Add a fade in transition or provide the container for the card already on the module, rather than having the currently filter bar and article card abruptly appear.

Change 624039 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Remove setting current card to no results widget

https://gerrit.wikimedia.org/r/624039

Okay, so I think what @RHo is saying is: we should keep using this ticket to do a couple refinements on the small patch that has been merged (the two that she listed in her comment), and then break other unmet specifications into a separate leftovers task? Is that right?

Or @kostajh, perhaps we need you to specify which parts of this task your patch does not do.

I don't want us to spend more time on this now, beyond a couple tweaks, because we want to be focusing on Variants C and D.

My proposal would be that @RHo updates the task description with what additional changes are desired, and that probably goes into Growth-Team-Leftovers for us to triage later?

Change 624040 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Remove skeleton class from pager after tasks fetched

https://gerrit.wikimedia.org/r/624040

Okay, so I think what @RHo is saying is: we should keep using this ticket to do a couple refinements on the small patch that has been merged (the two that she listed in her comment), and then break other unmet specifications into a separate leftovers task? Is that right?

Or @kostajh, perhaps we need you to specify which parts of this task your patch does not do.

I don't want us to spend more time on this now, beyond a couple tweaks, because we want to be focusing on Variants C and D.

hi @kostajh - @MMiller_WMF and I spoke about this earlier in the week, and I think the main thing is to set the height of the module on mobile and desktop to be as if the filters and number of tasks are loaded, so as not to cause the jarring height change and shift in position of the other elements. Is that a reasonably small tweak to be doable on this task? I can create #leftovers for further refinements that were not done (the footer shown last, having the skeleton for the filters and card on the screen to begin with, etc)

Change 626600 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Define minimum height for SE module on desktop

https://gerrit.wikimedia.org/r/626600

Okay, so I think what @RHo is saying is: we should keep using this ticket to do a couple refinements on the small patch that has been merged (the two that she listed in her comment), and then break other unmet specifications into a separate leftovers task? Is that right?

Or @kostajh, perhaps we need you to specify which parts of this task your patch does not do.

I don't want us to spend more time on this now, beyond a couple tweaks, because we want to be focusing on Variants C and D.

hi @kostajh - @MMiller_WMF and I spoke about this earlier in the week, and I think the main thing is to set the height of the module on mobile and desktop to be as if the filters and number of tasks are loaded, so as not to cause the jarring height change and shift in position of the other elements. Is that a reasonably small tweak to be doable on this task? I can create #leftovers for further refinements that were not done (the footer shown last, having the skeleton for the filters and card on the screen to begin with, etc)

I've done something a brittle (hardcoded the minimum heights I see for variant A, C/D), so those will need to get adjusted later if future updates to the module contents change the height. But it's better than nothing :)

Change 626600 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Define minimum height for SE module on desktop

https://gerrit.wikimedia.org/r/626600

Okay, so I think what @RHo is saying is: we should keep using this ticket to do a couple refinements on the small patch that has been merged (the two that she listed in her comment), and then break other unmet specifications into a separate leftovers task? Is that right?

Or @kostajh, perhaps we need you to specify which parts of this task your patch does not do.

I don't want us to spend more time on this now, beyond a couple tweaks, because we want to be focusing on Variants C and D.

hi @kostajh - @MMiller_WMF and I spoke about this earlier in the week, and I think the main thing is to set the height of the module on mobile and desktop to be as if the filters and number of tasks are loaded, so as not to cause the jarring height change and shift in position of the other elements. Is that a reasonably small tweak to be doable on this task? I can create #leftovers for further refinements that were not done (the footer shown last, having the skeleton for the filters and card on the screen to begin with, etc)

OK the patch for this is merged, so moving to QA and I'll let @RHo create follow up tasks in the Growth-Team-Leftovers column. Thanks!

Note that with rEGRE0f41086b7ac4: Show a skeleton animation while the small task card is loading merged the post-edit notice also has a loading animation now. The design is the same but the implementation is different so it could have its own set of bugs. (The task card on the C/D mobile summary will use mostly the same implementation as the post-edit notice, although the way data is loaded is not quite the same.)

I checked in betalabs and production wmf.9 (tested in Chrome and Chrome mobile emulator and a real iOS device); listed some issues below.

@RHo (and @MMiller_WMF) - should the task (and issues) be re-prioritized and maybe some issues should be split into different tickets?

Desktop

  • the footer displayed in the middle of SE card
  • when the card is loading, the counter displays "1 of ?"
  • a white background displayed

Basically, the following specs (from https://phabricator.wikimedia.org/T238231#5884928) are not done yet.

  1. There should not be white background on the loading screen before the pale blue.
  2. The Suggested edits white footer section should not start higher in the screen before shifting into position at the bottom of the screen

SE desktop skeleton1.gif (667×799 px, 136 KB)

Mobile (interesting that when SE card page is reload, it reloads Homepage first).

A var SE mobile skeleton1.gif (693×579 px, 211 KB)

C/D variants

  • have an additional display of SE card
  • when the card is loading, the counter displays "1 of ?"
  • a white background displayed

SE mobile skeleton1.gif (693×579 px, 199 KB)

A variant (production 'wmf.9`)

  • '?' is not present but '0 of 0' message does not convey a useful information either.
    prod SE mobile skeleton1.gif (693×579 px, 234 KB)

A variant betalabs

  • when the card is loading, the counter displays "1 of ?"
  • a white background displayed

A var SE mobile skeleton1.gif (693×579 px, 211 KB)

I tested image loading when I did https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GrowthExperiments/+/626773 and it seemed to work fine for me. The flash of white might be a side effect of having disabled caching in your browser.

We can render the card server-side now, I think we can use div with skeleton styling for the filters to make that bit a little easier.

I tested image loading when I did https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GrowthExperiments/+/626773 and it seemed to work fine for me. The flash of white might be a side effect of having disabled caching in your browser.

Yes, you're right - I was testing with disabled cash.

Hi @MMiller_WMF - the items that are still to be done as noted by @Etonkovidova and me are already listed out on the leftovers task T263040, so please review and close this task per our discussion that skeleton leftovers (bones?) can be worked on in an upcoming maintenance sprint.