Delay the loading icon display

[calixteman]

In most of the cases, showing the loading icon is useless because
it's for a very short time, consequently it doesn't bring any useful
information for the user.
After a delay (400ms), the icon is shown in order to inform the user
that the viewer isn't stuck but it's doing something.

[Snuffleupagus]

Sorry, but I don't really agree that this is a good/necessary change to make and it also adds a bunch of (what feels like unneeded) complexity.

If we do this it becomes generally impossible for the user to tell if a blank page means that there's actually no content on said page, or if the page has just not been loaded/rendered yet. To me this really doesn't feel like an improvement, but rather the opposite.

[calixteman]

Sorry, but I don't really agree that this is a good/necessary change to make and it also adds a bunch of (what feels like unneeded) complexity.

If we do this it becomes generally impossible for the user to tell if a blank page means that there's actually no content on said page, or if the page has just not been loaded/rendered yet. To me this really doesn't feel like an improvement, but rather the opposite.

If the page is really blank, the icon will be displayed for 1 or 2 ms, maybe it'll never been displayed.
Especially if the user is scrolling, then they'll never see it on a "normal" page because the page will be likely rendered before they reach the page center.
I don't really see how it's useful for the user to have this icon during few hundred of millis, but I can change my mind.
Note that I don't want to remove the icon since I really think it's useful, but under a certain delay it's really useless.
I chose 1500ms but we can change this value: I've no strong opinions about a good value here.

[calixteman]

That said, I really rely on your judgement: you really know this project better than me.
So if it's a strong no, just tell me and I'll remove this PR.
If you're so-so, I can ask to UI/UX what they think about that, but as far as I can tell, in general we want to try to avoid useless stuff which wastes resources like power.

[Snuffleupagus]

If you're so-so, I can ask to UI/UX what they think about that,

Let's start there, please. (Since UI design isn't exactly my strong suite.)

[Snuffleupagus]

Implementation-wise, the timeout usage is what I'm the least "excited" about here since that'll lead to a bunch of complexity that doesn't currently exist.
So how about if we only display the loadingIcon-div on the page which currently has RenderingState.RUNNING (without any timeouts), i.e. the most visible page, since that would actually allow us some overall simplification already; my take on this in master...Snuffleupagus:pdf.js:loadingIcon-toggle

[calixteman]

Implementation-wise, the timeout usage is what I'm the least "excited" about here since that'll lead to a bunch of complexity that doesn't currently exist. So how about if we only display the loadingIcon-div on the page which currently has RenderingState.RUNNING (without any timeouts), i.e. the most visible page, since that would actually allow us some overall simplification already; my take on this in master...Snuffleupagus:pdf.js:loadingIcon-toggle

Clearly what you're proposing will simplify this stuff: so +1 for me.
I understand you aren't excited by the timeout stuff: I'm not either. I asked to UX but I've no answer for the moment (a lot of people are in pto right now), but as far as I can tell, this is something we'll do in other parts of firefox.
Anyway, what about using https://developer.mozilla.org/en-US/docs/Web/CSS/transition-delay ? which should do exactly what we want here.

[Snuffleupagus]

I think that it makes sense to wait for UX feedback before moving forward here, and in any case we probably don't want to land it just before a release.

Also, based on reading the patch I've got one "larger" question: Will this apply to both showing and hiding of the loadingIcons? Because if so, that's probably not what we want here since the hiding needs to be immediate.

[calixteman]

400ms is the value we're using for the tab spinners, so use the same.

[calixteman]

UI/UX agreed on 400ms.

[Snuffleupagus]

r=me, with comments addressed, since this seems to work in my testing (and a delay of only 400ms isn't so long that it renders the indicators pointless).

[calixteman]

/botio integrationtest

[pdfjsbot]

From: Bot.io (Linux m4)

---

Received

Command cmd_integrationtest from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/ab0dc5c2fa3aaa6/output.txt

[pdfjsbot]

From: Bot.io (Windows)

---

Received

Command cmd_integrationtest from @calixteman received. Current queue size: 0

Live output at: http://54.193.163.58:8877/f7d0264a6770d15/output.txt

[pdfjsbot]

From: Bot.io (Linux m4)

---

Success

Full output at http://54.241.84.105:8877/ab0dc5c2fa3aaa6/output.txt

Total script time: 4.34 mins

• Integration Tests: Passed

[pdfjsbot]

From: Bot.io (Windows)

---

Failed

Full output at http://54.193.163.58:8877/f7d0264a6770d15/output.txt

Total script time: 10.46 mins

• Integration Tests: FAILED