Update visuals to latest spec for app download/cancel/error states

RESOLVED FIXED in Firefox OS v2.0

Status

RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jlal, Assigned: jlal)

Tracking

({late-l10n})

unspecified
2.0 S4 (20june)
x86
Mac OS X
late-l10n
Dependency tree / graph
Bug Flags:
in-moztrap -

Firefox Tracking Flags

(feature-b2g:2.0, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

(Whiteboard: [systemsfe])

Attachments

(13 attachments, 2 obsolete attachments)

596.36 KB, application/pdf
Details
518.80 KB, image/png
Details
298.33 KB, image/png
amylee
: ui-review-
Details
1.38 MB, image/png
Details
46 bytes, text/x-github-pull-request
kgrandon
: review+
kgrandon
: feedback+
crdlc
: feedback+
Details | Review | Splinter Review
282.15 KB, image/png
Details
282.15 KB, image/png
Details
364.66 KB, image/png
Details
365.95 KB, image/png
pla
: review+
Details
367.68 KB, image/png
Details
151.41 KB, application/zip
Details
46 bytes, text/x-github-pull-request
Details | Review | Splinter Review
46 bytes, text/x-github-pull-request
Details | Review | Splinter Review
(Assignee)

Description

5 years ago
Created attachment 8438504 [details]
App Download June10.pdf

We landed most (if not all) of the work to bring this functionality to vertical home but we need to update it to the latest specs (attached)
QA Whiteboard: [VH-FL-blocking+][VH-FC-blocking+]
blocking-b2g: --- → 2.0?
I thought about it twice and as the feature is working, having new icons is not blocking 2.0, to my understanding.
blocking-b2g: 2.0? → ---
QA Whiteboard: [VH-FL-blocking+][VH-FC-blocking+] → [VH-FL-blocking-][VH-FC-blocking-]

Comment 2

5 years ago
Created attachment 8438824 [details]
AppInstallVisualAssets.zip

Attached are the visual assets for app install.

Comment 3

5 years ago
Johen, please think again.  By not blocking, you are contributing to a poor product, and that is putting it mildly.  We can't keep considering these things as 'small'.  It's NOT polish.  It's the actual icons that are needed to communicate app install states!
blocking-b2g: --- → 2.0?
(In reply to Peter La from comment #3)
> Johen, please think again.  By not blocking, you are contributing to a poor
> product, and that is putting it mildly.  We can't keep considering these
> things as 'small'.  It's NOT polish.  It's the actual icons that are needed
> to communicate app install states!

Let me understand something - is the existing implementation right now, what is the current UX & what are the problems with it?
(In reply to Jason Smith [:jsmith] from comment #4)
> (In reply to Peter La from comment #3)
> > Johen, please think again.  By not blocking, you are contributing to a poor
> > product, and that is putting it mildly.  We can't keep considering these
> > things as 'small'.  It's NOT polish.  It's the actual icons that are needed
> > to communicate app install states!
> 
> Let me understand something - is the existing implementation right now, what
> is the current UX & what are the problems with it?

Fix wording - "What is the existing implementation right now & what is the current UX & what are the problems with it?"

Updated

5 years ago
Whiteboard: [systemsfe]
(Assignee)

Updated

5 years ago
Assignee: nobody → jlal

Comment 6

5 years ago
Created attachment 8439452 [details]
AppInstallStates_20140610.png
(Assignee)

Comment 7

5 years ago
Created attachment 8439508 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20459

This version of the patch only contains the "installing" state changes
(Assignee)

Comment 8

5 years ago
Created attachment 8439511 [details]
Installing an app (visuals)
Attachment #8439511 - Flags: ui-review?
This is super nice to have, and if we get to it we will certainly ask for approval. Being that homescreen1 does not have these nice states, it's considered feature work that we don't block on.

That said, I do hope we fix this and uplift it by next week. Moving to block vhomescreen.next for now.
Blocks: 1017954
No longer blocks: 989848
blocking-b2g: 2.0? → ---
(Assignee)

Comment 10

5 years ago
I am still planning to work on this but after I fix some of our blocking bugs today expect the patch Monday (and all states should work in search/collection)

Updated

5 years ago
blocking-b2g: --- → 2.0?
feature-b2g: --- → -

Comment 11

5 years ago
Amy and Peter, we're trying to make a blocking decision here. Just to clarify the rationale, the new icon assets are required for *both* Visual Refresh (which should be sufficient to block on, as these are the assets partners are expecting) and vertical homescreen. The new vertical homescreen needs the larger icons in order to do the three column display AND four column display.

If either of you can post some side-by-side screenshots of how the old assets would perform in vertical homescreen, that would help, but the new assets *are* part of the Visual Refresh.
Flags: needinfo?(pla)
Flags: needinfo?(amlee)
blocking-b2g: 2.0? → 2.0+
Created attachment 8441365 [details]
InstallApp_Screens.png

Hi, 

Here is a side by side comparison of the spec icons and the current icons. As you can see, the current icons do not convey Installation Stopped or Download Failed and the retry download looks especially bad. I would say this is a blocker.
Flags: needinfo?(amlee)
Target Milestone: --- → 2.0 S4 (20june)
(In reply to Amy Lee [:amylee] from comment #12)
> Created attachment 8441365 [details]
> InstallApp_Screens.png
> 
> Hi, 
> 
> Here is a side by side comparison of the spec icons and the current icons.
> As you can see, the current icons do not convey Installation Stopped or
> Download Failed and the retry download looks especially bad. I would say
> this is a blocker.

Hi Amy,

Should you change the ui-review to -?
Comment on attachment 8439511 [details]
Installing an app (visuals)

Will need to see the rest of the states before giving this a +. Thanks
Attachment #8439511 - Flags: ui-review? → ui-review-
With more info from VD, I'm okay with the need to block on this, but I'd prefer to track this with the feature-b2g flag, as I'd call this feature work, rather than a bug.

Candice - Can you set the feature-b2g flag here?
Blocks: 989848
No longer blocks: 1017954
blocking-b2g: 2.0+ → ---
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking-] → [VH-FL-blocking+][VH-FC-blocking-]
Flags: needinfo?(cserran)

Updated

5 years ago
QA Whiteboard: [VH-FL-blocking+][VH-FC-blocking-] → [VH-FL-blocking+][VH-FC-blocking+]

Comment 16

5 years ago
I'm flagging this for Candice since I was just on IRC w/ her, know she has gone to sleep in Paris, and know she is totally fine with me doing this. :)
feature-b2g: - → 2.0

Updated

5 years ago
Flags: needinfo?(cserran)
(Assignee)

Comment 17

5 years ago
Worth noting that one UX bug my patch will have right off the bat is if you restart your device and you have an app that was in an "Error" state it will be in a "Canceled" state when you see the homescreen again. This is because mozApps does not keep track of previous downloadErrors (there might be a work around)
(Assignee)

Comment 18

5 years ago
Current version of my patch relies on a gecko fix for how events are sent through in Webapps.jsm
Depends on: 1027458
(Assignee)

Updated

5 years ago
Attachment #8439508 - Attachment is obsolete: true
(Assignee)

Comment 20

5 years ago
Comment on attachment 8442635 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20725

Could I get some early feedback on this patch? This is working well for me on the homescreen with my gecko patch but does not yet update the search or collections apps.
Attachment #8442635 - Flags: feedback?(crdlc)
Comment on attachment 8442635 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20725

LGTM, some comment on github but for me it is ok. Take care with my last change for loading asset in master. Thanks
Attachment #8442635 - Flags: feedback?(crdlc) → feedback+
Comment on attachment 8442635 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20725

I will look through more, I can be stubborn about WC, but we can change that in a follow-up. It shouldn't block this from landing.

For now I would probably prefer gaia_confirm/js/dialog.js to be moved outside of the component if possible, and into shared/js/homescreens or just shared/js possibly.
Attachment #8442635 - Flags: feedback?(kgrandon) → feedback+

Comment 23

5 years ago
Hey guys,

Sorry for the delayed response.  Basically, I agree with Amy's assessment, and I think everyone here does.  James. I'll take a look at your patch asap.
Flags: needinfo?(pla)
(Assignee)

Comment 24

5 years ago
Comment on attachment 8442635 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20725

This is ready for a real review pass _but_ it will not handle retries correctly until the gecko patch lands (same bugs as we had before).
Attachment #8442635 - Flags: review?(kgrandon)
(Assignee)

Comment 25

5 years ago
Created attachment 8443601 [details]
2014-06-20-13-46-13.png
(Assignee)

Comment 26

5 years ago
Created attachment 8443603 [details]
Failed install from search
Attachment #8443603 - Flags: ui-review?(pla)
(Assignee)

Comment 27

5 years ago
Created attachment 8443604 [details]
Download fail on verticalhome
Attachment #8443604 - Flags: ui-review?(pla)
(Assignee)

Comment 28

5 years ago
Created attachment 8443605 [details]
Paused download on vertical home
Attachment #8443605 - Flags: review?(pla)
(Assignee)

Comment 29

5 years ago
Created attachment 8443608 [details]
Download in progress on vertical home

Getting to each and very of these states can be difficult I wrote an interactive (command line interface) tool built on top of our testing framework  to test these manually ..
Attachment #8443608 - Flags: ui-review?(pla)

Comment 30

5 years ago
Comment on attachment 8443603 [details]
Failed install from search

Looks good.  The icon is not the right size but that is a separate issue.
Attachment #8443603 - Flags: ui-review?(pla) → ui-review+

Comment 31

5 years ago
Comment on attachment 8443604 [details]
Download fail on verticalhome

Looks good.
Attachment #8443604 - Flags: ui-review?(pla) → ui-review+

Comment 32

5 years ago
Comment on attachment 8443608 [details]
Download in progress on vertical home

Looks good.
Attachment #8443608 - Flags: ui-review?(pla) → ui-review+

Comment 33

5 years ago
Comment on attachment 8443605 [details]
Paused download on vertical home

Looks good.
Attachment #8443605 - Flags: review?(pla) → review+

Comment 34

5 years ago
Created attachment 8443721 [details]
AppInstallGraphicsUpdated_Jun20.zip

This file contains the updated visuals, including a fix to the @3.375x size, which was mistakenly saved out at 189x189 instead of 284x284.
Attachment #8438824 - Attachment is obsolete: true
(Assignee)

Comment 35

5 years ago
Assets updated and the failing test is ignored until we land 1027458 and it hits nightlies
Comment on attachment 8442635 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20725

Continuing to test, but everything is looking good.
Attachment #8442635 - Flags: review?(kgrandon) → review+
Keywords: late-l10n
Comment on attachment 8442635 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20725

Pre-emptive approval request for 2.0 as this will land today once the tree is green and is late-l10n.
Attachment #8442635 - Flags: approval-gaia-v2.0?(bbajaj)

Updated

5 years ago
Attachment #8442635 - Flags: approval-gaia-v2.0?(bbajaj) → approval-gaia-v2.0+
Comment on attachment 8442635 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20725

I made some comments on the bug. Many are nits / inconsistencies with the guidelines / or misused or bind for handleEvent - but the most important is that I really think we should not land the APZ workaround, and we need to open a bug for the comment about the visibilitychange event.

The APZ bug is likely hitting us in other apps, and if I understand it correctly it means that it is a regression (not because of the vertical homescreen) and should be fixed for 2.0, as it affect this app, and third party apps as well.
Created attachment 8443852 [details] [review]
Rebased PR, review comments not addressed.
In master: https://github.com/mozilla-b2g/gaia/commit/f00f57bd824149a0bc9dede0af530fd712d15b3e
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
2.0: https://github.com/mozilla-b2g/gaia/commit/9568c5ffb439e17f6623eb5462a279b1636d37bf
status-b2g-v2.0: --- → fixed
status-b2g-v2.1: --- → fixed
Created attachment 8447498 [details] [review]
Follow-up, enable tests now that we have an updated nightly
You need to log in before you can comment on or make changes to this bug.