999171 - Implement a progress indicator to show progress when the viewfinder has not loaded

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Camera, defect)

Description

[Jason Smith [:jsmith]]

bug 994077 is too risky to take onto 1.4, so we won't be able to resolve the performance hit seen in that bug. However, the current implementation state seems problematic for devices like Buri - the load time is far too long, which might cause user frustration waiting for the camera to get up and running. The performance team has suggested that we implement a progress indicator to help mitigate the frustration, as that will indicate to the user that the viewfinder is still loading.

Comment 1

[Jason Smith [:jsmith]]

This is going to be 1.4 solution to bug 994077.

Comment 2

[Jason Smith [:jsmith]]

Over to UX for some design magic.

Comment 3

[Stephany Wilkes]

Flagging Tif on Madai and Rob on that plus the progress indicator pattern.

Comment 4

[Tiffanie Shakespeare [:tif]]

Yep, we should use a spinner since we can't predict the length of delay.

(In reply to Jason Smith [:jsmith] from comment #1)
> This is going to be 1.4 solution to bug 994077.

Comment 5

[Preeti Raghunath(:Preeti)]

David,

Reaching out in Hema's absence.

Can we please review this for camera?

Comment 6

[David Flanagan [:djf]]

Wilson,

Since you own 994077, do you want to take this one, too?

To display a spinner, you just include shared/style_unstable/activity_progress.css and throw a <progress> tag into your html. It will automatically be styled as a spinner.

I just had to do this in the 1.3T camera for image rotation stuff and ended up with this in my CSS file:

#spinner {
  position: absolute;
  top: calc(50% - 2.5rem);
  left: calc(50% - 2.5rem);
  width: 5rem;
  height: 5rem;
  z-index: 1000;
}

/* override progress_activity.css */
#spinner.hidden {  <== this was the only tricky part
  display: none;
}


Tif: I'm not sure what the default size of the spinner is. In my 1.3T patch, I forced it to 5rem. The default size looks too small to me. How big do you want it?  I assume that centered on the screen is what you want, right?

Comment 7

[Wilson Page [:wilsonpage]]

Sure, I'll take this. I will begin work once the perf stuff has landed on master.

Comment 8

[Rob MacDonald [:robmac]]

We raised this during a building blocks meeting yesterday and are proposing the following UI.

Show the standard launch screen (centered icon, black background) for two seconds. If the app has not yet loaded, then add the dark activity bar across the top of the screen. (This is the same one used in Gallery while scanning for images and videos).

This is considered a temporary 1.4 solution until performance can be improved.

Hopefully this doesn't come too late but I'll hop in the IRC channel to confirm.

- Rob

Comment 9

[Tiffanie Shakespeare [:tif]]

clearing my NI since Rob responded.

Comment 10

[Wilson Page [:wilsonpage]]

robmac: I'm not 100% sure what you mean. The launch screen where you seen the app icon is handled by the OS not the Camera App. It disappears when the app is loaded. In the case of the camera app though, we still need to wait for the camera hardware to be loaded, this is the expensive part.

I'm surprised you have decided to go with the loading indicator from Gallery. I always thought that looked broken, like some strange retro marching ants style thingy.

Comment 11

[Rob MacDonald [:robmac]]

(In reply to Wilson Page [:wilsonpage] from comment #10)

Sounds like I may have misunderstood the requirement. I've asked Tif to sync up with you tomorrow.

And you don't like ants? :) Actually the reason we went with this one is because it was less obtrusive than the other progress indicators. This is also considered a temporary solution as Przemek is working on new start-up animations for 2.0/2.1.

Comment 12

[Amy Lee [:amylee] UX]

Attachment: Loading_Screen_Camera_85opacity_grey.png

Hi, 

I've attached a spec of the proposed loading screen for camera. Thanks!

Overlay
Transparency: 85%
Colour: Dark Grey (#333333)

Comment 13

[Tiffanie Shakespeare [:tif]]

Hey Wilson - let us know if you need anything else! :)

Comment 14

[Amy Lee [:amylee] UX]

(In reply to Amy Lee [:amylee] from comment #12)
> Created attachment 8412976 [details]
> Loading_Screen_Camera_85opacity_grey.png
> 
> Hi, 
> 
> I've attached a spec of the proposed loading screen for camera. Thanks!
> 
> Overlay
> Transparency: 85%
> Colour: Dark Grey (#333333)

Just to clarify, the spinner is the default building block spinner (I think it's 2.9 rems) and it should be centered to the screen.

Comment 15

[Wilson Page [:wilsonpage]]

Attachment: pull-request (master)

Comment 16

[Amy Lee [:amylee] UX]

Comment on attachment 8414587 [details]
pull-request (master)

Hey Wilson, 

I can't get the spinner to show up after I flashed the patch. I'm on a Nexus 4. I'm not sure if the device is too fast. When I open the camera app, a black screen with the icon shows up and then a brief black screen with the controls show up with maybe a 1-2 second delay before the viewfinder appears.

Comment 17

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8414587 [details]
pull-request (master)

Looks good. But, please look at my comments in GitHub before you land. I had a question about the clock stuff in the unit tests (not sure if that could cause an intermittent test failure or not).

Comment 18

[Tiffanie Shakespeare [:tif]]

Comment on attachment 8414587 [details]
pull-request (master)

Same as Amy - I can't get a spinner to show. Unfortunately, I don't have any other device other than a Nexus 4. :-/

Comment 19

[Justin D'Arcangelo [:justindarc]]

Tif/Amy: since your devices aren't slow enough to show the spinner and the spinner is just using the Building Blocks, would a screenshot be sufficient for a visual review?

Comment 20

[Amy Lee [:amylee] UX]

Hey Justin/Wilson, 

Is it possible to get a screenshot and a video of it? I just want to see how it looks when it transitions from the app splash page (black background and camera icon) to the spinner. Thanks!

Comment 21

[Justin D'Arcangelo [:justindarc]]

Video: http://youtu.be/Pw0dbPKvDUY

Comment 22

[Justin D'Arcangelo [:justindarc]]

Attachment: Screenshot - index.png

Comment 23

[Amy Lee [:amylee] UX]

Comment on attachment 8414587 [details]
pull-request (master)

Hi, 

The visuals are good :-) Tiff has some comments about the UX.

Comment 24

[Tiffanie Shakespeare [:tif]]

Comment on attachment 8414587 [details]
pull-request (master)

Spinner looks awesome. I think we missed just one bit during our communications.

During the 400ms while we wait to see if the hardware is ready (and before we show the spinner), let's make the shutter also dimmed. 

So we launch camera, see launch screen (which goes away pretty quickly), see black viewfinder with dimmed shutter button for 400ms, than the spinner appears. All disappear when the camera hardware is ready to go.

Make sense? Thanks Wilson

Comment 25

[Wilson Page [:wilsonpage]]

tiff: Cheers I have made those changes.

Comment 26

[Wilson Page [:wilsonpage]]

Latest: https://www.youtube.com/watch?v=rSRiMnvDf1M

Comment 27

[Tiffanie Shakespeare [:tif]]

Comment on attachment 8414587 [details]
pull-request (master)

Couldn't get the spinner to work myself - phone is just too fast, but verified that everything should be working as it should; buttons are now dimmed out until after the spinner disappears.

Thanks!

Comment 28

[Tiffanie Shakespeare [:tif]]

Terrible video Wilson! But he solemnly swears the buttons are dimmed out and Justin backs him up too. Seems good guys!

(In reply to Wilson Page [:wilsonpage] from comment #26)
> Latest: https://www.youtube.com/watch?v=rSRiMnvDf1M

Comment 29

[Justin D'Arcangelo [:justindarc]]

Landed on master:

https://github.com/mozilla-b2g/gaia/commit/30aa24a9cfd4384ca67da927bbe0ec98518e410e

Comment 30

[Justin D'Arcangelo [:justindarc]]

Wilson: As mentioned on IRC, you will need to make a v1.4-specific patch for this as well. The PR to master won't apply cleanly to the v1.4 branch.

Comment 31

[Wilson Page [:wilsonpage]]

Attachment: pull-request (v1.4)

Comment 32

[Wilson Page [:wilsonpage]]

justindarc: I've attached PR for the v1.4 uplift. I've had to do some workarounds to handle the fact that this patch was based on code that hasn't been uplifted.

Comment 33

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8415811 [details] [review]
pull-request (v1.4)

A couple nits, but otherwise looks alright to me.

Comment 34

[Wilson Page [:wilsonpage]]

Landed on 'v1.4': https://github.com/wilsonpage/gaia/commit/4129edbd552e6f8b22b13233a0dc8bea5ca07078