Page MenuHomePhabricator

[Engineering EPIC]: Loading screen improvements
Closed, ResolvedPublic

Related Objects

Event Timeline

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

Change 486412 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] [DO NOT MERGE] Artificial loading delay for testing

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

Change 486407 abandoned by Bartosz Dziewoński:
[WIP] VE: New loading screen design

Reason:
Duplicate of https://gerrit.wikimedia.org/r/486408 caused by a Gerrit bug (T214656).

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

Change 488230 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] DesktopArticleTarget: Make the progress bar into a reusable widget

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

Change 488215 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] POC: Move talkOverlay factory function into mobile.startup

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

Change 488230 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] DesktopArticleTarget: Make the progress bar into a reusable widget

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

Current state:

Look good! I've fixed the short page blank flicker in the latest patchset. This makes the transition from read to edit smoother, but reveals some slight differences we can probably fix:

  • Reference font size is 12px in read mode, and 13.3333px in VE
  • Headings have a minimum height of 31px in read mode, due to the edit pencils, but collapse to smaller heights in VE. A min-height should fix this

No doubt there is a long tail of these, some unfixable, but these were the most obvious.

Corresponding bugs:

  • T150377 T208102
  • [none, small change to reference font size in VE]
  • T215694
  • [none, small height change to headings in VE]
JTannerWMF moved this task from 📚 Backlog to 📢 To Do on the Editing Design board.

This is looking solid @matmarex , nice work.

I have a few implementation tweaks and then I'd say we are good to go.

1. Progress bar - it currently only loads to 20% and then disappears - it should animate to 100% to give the perception that it's loading faster than it is.

2. Scroll jump - the screen currently animates up and then bounces back down into position, it should scroll right to the correct position and stop.

3.Close button - should be "X" not "<"

  1. Scroll jump

I wasn't seeing this on Android, could be an iOS issue?

1. Progress bar - it currently only loads to 20% and then disappears - it should animate to 100% to give the perception that it's loading faster than it is.

How far the progress bar gets is going to depend a lot on your internet connection… but still, it will only animate up to 70% (and then stay there until the editor actually loads).

How it works exactly:

  • When the user clicks "Edit":
    • Loading bar animates from 0% to 30% (it takes 3 seconds)
    • We start two network requests: one to load the editor's code, and one to load the article contents (in an editable format)
  • When one of these requests completes:
    • Loading bar animates from whatever position it reached to 70% (this takes 2 seconds)
  • When both requests complete:
    • Loading bar is hidden and the editor appears
    • (We apparently have the code to animate it to 100% over 1 second, but I can't think of a case where this would be visible)

If both of these requests complete quickly (under 2 seconds or so), you would indeed see it animate to 20% and then disappear.

We don't have the information about the real loading progress. Although maybe we should investigate this again and see if we could get them from the browser…

I don't know why these exact thresholds and timings were chosen, but the main goal was for the progress bar to accelerate as it nears completion (see T95137 for sparse notes about this). So I guess not "using" the last 30% of the bar is deliberately trying to create this illusion. I can tweak the timings, but I honestly don't know what they should be.

2. Scroll jump - the screen currently animates up and then bounces back down into position, it should scroll right to the correct position and stop.

I wasn't seeing this on Android, could be an iOS issue?

There may be a small jump (on Android and iOS) if the section or article has a block slug near the beginning (the "+ Insert paragraph" thing), e.g. if it starts with an image or an infobox. Otherwise there should be no visible jumps. @iamjessklein Which page/section do you see this on? Can you record your screen when you see this issue, and upload the video here or to Google Drive?

3.Close button - should be "X" not "<"

Oh, I thought we wanted to do this later! I would actually be glad to make this change sooner. I'll write a patch, should be easy.

Change 490202 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] ve.init.mw.MobileArticleTarget: Change "back" tool icon from "<" to "x"

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

Change 490202 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] ve.init.mw.MobileArticleTarget: Change "back" tool icon from "<" to "x"

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

Here's what the scroll jump looks like to me. I tested this on Android and saw it there as well so I don't believe it's an i0S bug. This is shown in Safari and chrome. I see also that the bottom of the toolbar abruptly fades away as well so, perhaps if that's cleaned up a bit (minimized somehow) perhaps it will lesson the jump effect.

I can work with you @matmarex to play around with the timing for the progress bar. I think this would be a good thing to pair code mob program.

Change 490315 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] ve.init.mw.MobileArticleTarget: Better match surface top spacing to read mode

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

Here's what the scroll jump looks like to me. I tested this on Android and saw it there as well so I don't believe it's an i0S bug. This is shown in Safari and chrome.

Oh, so this is when editing the whole article / lead section. Nicely spotted, I guess I didn't pay enough attention to it! Patch https://gerrit.wikimedia.org/r/490315 fixes this.

I see also that the bottom of the toolbar abruptly fades away as well so, perhaps if that's cleaned up a bit (minimized somehow) perhaps it will lesson the jump effect.

I guess this is because of the grey border in read mode between action icons and article content. I added a fade animation to it (in https://gerrit.wikimedia.org/r/486408).

Esanders added a comment.EditedFeb 13 2019, 2:42 PM

I've able to get the toolbar to show twice when loading/closing the editor repeatedly:


I think this happens if you start the second load before the aborted first load would've finished, so perhaps we aren't aborting the loading promises, or other guarding against them running when they resolve.

Change 490315 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] ve.init.mw.MobileArticleTarget: Better match surface top spacing to read mode

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

iamjessklein added a subscriber: Volker_E.EditedFeb 14 2019, 1:10 PM

Based on the research that @matmarex did on T216034#4952525 I'm wondering if the best solution here would be to go back to an indeterminate throbber. I did some research and found in A study on tolerable waiting time: how long are Web users willing to wait? that most users’ tolerable wait time is 4 seconds.

So, throbbers (aka spinners) are great for short processes and progress bars are tolerable for > 4 seconds. Then, on top of that

Do we think that the average time for most users here is less than 4 seconds?

In terms of the OOUI component, I propose that we make some guidelines:

  • < 4 second process or completely unknown = throbber
  • > 4 second process = progress bar
  • > 1 minute process = progress bar with motivational copy

Of course this is written knowing that we have no understanding of the connection speed and that it will greatly impact that perceived loading time.

cc/ T214477

cc/ @Volker_E

Stats on Desktop load performance: https://www.mediawiki.org/wiki/Contributors/Projects/Editing_performance

Load time will vary hugely based on page size, network connection and device performance. Our 10th-90th centiles cover anywhere form <1s to >10s, so we need to account for anything in this range, and we won't know before hand.

(Krinkle talked to me yesterday after seeing the loading progress investigation. Apparently there exists a Navigator.connection API that can be used to get some data about the user's connection speed, which we could use to estimate load time. However, it seems sparsely supported by browsers.)

Based on the research that @matmarex did on T216034#4952525 I'm wondering if the best solution here would be to go back to an indeterminate throbber. I did some research and found in A study on tolerable waiting time: how long are Web users willing to wait? that most users’ tolerable wait time is 4 seconds.

Either way seems reasonable to me. I honestly have no opinion. Ed might be more willing to discuss with you, I think he feels more strongly about loading indicators ;)

In terms of the OOUI component, I propose that we make some guidelines:

  • < 4 second process or completely unknown = throbber
  • > 4 second process = progress bar
  • > 1 minute process = progress bar with motivational copy

Note that there is currently no classic throbber/spinner in OOUI (although we used a variety of them in our older interfaces). OOUI's equivalent of a throbber is the "indeterminate progress bar" (https://doc.wikimedia.org/oojs-ui/master/demos/#Progress-bar).

Right, I'll have to work on whatever we ultimately decide on in T214477 but in the interim we can use the most similar interaction. I think that is the indeterminate progress bar.

For completion: We also feature a default standard throbber (sometimes referred to as spinner, manifest as neither of those but three pulsating dots), which isn't part of OOUI, see T75972 and in action on f.e. Special:RecentChanges.

@iamjessklein Your timing differentiations from the study seem useful as part of application guidelines.

Oh, ha, and not to forget one of the two oldest bugs I've ever participated in T1181: VisualEditor loading indicator should feel more integrated into the mobile interface

I've able to get the toolbar to show twice when loading/closing the editor repeatedly: (…)

Fixed in PS22 of https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/486408.

Change 490885 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] init: Use indeterminate progress bar

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

matmarex added a comment.EditedFeb 15 2019, 5:03 PM

Right, I'll have to work on whatever we ultimately decide on in T214477 but in the interim we can use the most similar interaction. I think that is the indeterminate progress bar.

Right, done. Note that the change I submitted applies to both mobile and desktop.

If we are going to rethink the progress bar in T214477, should we just leave it as-is for this iteration, rather than changing it from determinate to indeterminate, only to possibly change in back in a few months?

If the indeterminate component exists, I'd vote for using it because it shows all of the affordances that we are trying to encourage. Then, it's purely a cosmetic fix in T214477. However, if this will require a ton of technical effort, +1 vote for holding off.

No, it’s easy to use the indeterminate progress bar.

Change 490885 abandoned by Bartosz Dziewoński:
init: Use indeterminate progress bar

Reason:
We decided in the standup today that we don't want to make this change on desktop right now, only on mobile, so I will put these changes into https://gerrit.wikimedia.org/r/c/486408/ in MobileFrontend instead.

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

Change 488215 abandoned by Jdlrobson:
POC: Move talkOverlay factory function into mobile.startup

Reason:
Done in I791b22ac8b5060c4620168a3bf8db81a96f3d022

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

@matmarex @iamjessklein so we've made some progress on this - the LanguageOverlay and TalkOverlay have been rewritten and are feeling much more responsive than before (the overlay loads instantly and the body loads later):

The code changes to make that possible are T215370 and T215657
Super keen to making some of these changes in EditorOverlay, but would be keen to have someone from editing write those patches or at least review them! Editing overlays are likely to be the most complicated given the existence of EditorOverlayBase

@Jdlrobson The current patch for this task is: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/486408

Our current plan is to have it merged next week, after Mar 5 (so that it is deployed with the train on Mar 12-14). Can we talk about the rewrite after that happens?

(For anyone else reading this, the context is Jon's / Reading team's work on T212465)

Looking at the video that @matmarex posted (T210630#4938902), the Edit overlay (at least in the case of section editing) has the appearance that the content on-screen before the overlay is called, remains on-screen, and that the overlay is really just the toolbar on top of the content that was already there. I'm mainly just curious what's actually happening in that case, and ultimately if these are two different types of overlays: one that is seamlessly transitioning using existing content (e.g. section editing overlay), and another that transitions an entirely new view on top of the existing one (e.g. Talk or Languages).

To be clear, @alexhollender gave a +1 to the design review of the loading screen overlay approach in this ticket.
We should move into the next stages for getting this shipped and when the OOUI component T214477 is updated, that should be shipped separately.

Perhaps we should move the conversation posted T210630#4998895 to another ticket focused on streamlining/documenting our approach to overlays.
Thoughts @JTannerWMF ?

Looking at the video that @matmarex posted (T210630#4938902), the Edit overlay (at least in the case of section editing) has the appearance that the content on-screen before the overlay is called, remains on-screen, and that the overlay is really just the toolbar on top of the content that was already there. I'm mainly just curious what's actually happening in that case, and ultimately if these are two different types of overlays: one that is seamlessly transitioning using existing content (e.g. section editing overlay), and another that transitions an entirely new view on top of the existing one (e.g. Talk or Languages).

Yes, this is perfectly correct. We want to make it seem like it's just a toolbar shown over the original content, so that the user can easily find the place they wanted to edit.

In actuality, the content is slightly different, but in most cases it should look the same. (For example: the read mode content has markup for collapsing top-level sections, the edit mode doesn't; the edit mode content has markup describing which templates were used to generate the page, the read mode doesn't.)

This is a small difference between the visual editing overlay and other overlays, but I don't think it's a fundamentally different type of overlay. The only difference is that the background of "normal" overlays is fully opaque white, and the background of visual editing overlay is semi-transparent while it's loading to generate this effect, and turns opaque after the content is loaded.

Perhaps we should move the conversation posted T210630#4998895 to another ticket focused on streamlining/documenting our approach to overlays.
Thoughts @JTannerWMF ?

I'm not sure if we need to do anything about this? But yeah, if you want to propose some changes based on this knowledge, rather than just being curious, please file another task :)

matmarex added a comment.EditedMar 5 2019, 7:32 PM

Here's the before-after comparison for the latest version of the patch.

This one also shows using the new section editing we've been working on in another task.

BeforeAfter

After looks already like a good step forward!
One user expectation comment: Could we show the toolbar a bit earlier while the progressbar is still visible, in order to further distract people from loading time and allow them to accomodate with the bar?

Thanks for the review @Volker_E - I added a new ticket T217784 for this to track the progress on it. I've added you as design reviewer.

Change 486408 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] VE: New loading screen design

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

While testing a different issue, Rummana identified a minor issue on wikis with section editing disabled:

@matmarex: Alrighty! So I checked this today on en.wiki, looks okay to me. I just have one question:
Towards the end of a document if the sections are too short as in there is just not enough content to fill in the entire screen, it's opening the article slightly above the section, which makes sense to me, but just wanted to confirm with you that this is indeed the expected behavior in this case :)
Video capture for better understanding: (I clicked the section edit link next to the last section "new section edit"

Basically, the scroll-to-align effect doesn't scroll to the correct place, because in editing mode the section can't be aligned with the top of the editor (we can't scroll past the end of the document).

I started working on this, but it turns out to be problematic to implement for a few reasons. I think it's not really worth spending more time on, especially since we aim to have section editing enabled everywhere soon. But I'm noting this here for completeness.

Change 499965 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@master] VE loading overlay tweaks for small sections near the end

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

Change 499965 abandoned by Bartosz Dziewoński:
VE loading overlay tweaks for small sections near the end

Reason:
Incomplete proof of concept, see T210630#5068517

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

Change 486412 abandoned by Bartosz Dziewoński:
[DO NOT MERGE] Artificial loading delay for testing

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

Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptMay 22 2019, 1:42 AM