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)

I thought about it twice and as the feature is working, having new icons is not blocking 2.0, to my understanding.

Attached are the visual assets for app install.

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!

(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?"

This version of the patch only contains the "installing" state changes

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.

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)

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.

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.

(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

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?

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. :)

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)

Current version of my patch relies on a gecko fix for how events are sent through in Webapps.jsm

Comment on attachment 8442635 [details] 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.

Comment on attachment 8442635 [details] 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

Comment on attachment 8442635 [details] 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.

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.

Comment on attachment 8442635 [details] 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).

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 ..

Comment on attachment 8443603 [details] Failed install from search Looks good. The icon is not the right size but that is a separate issue.

Comment on attachment 8443604 [details] Download fail on verticalhome Looks good.

Comment on attachment 8443608 [details] Download in progress on vertical home Looks good.

Comment on attachment 8443605 [details] Paused download on vertical home Looks good.

This file contains the updated visuals, including a fix to the @3.375x size, which was mistakenly saved out at 189x189 instead of 284x284.

Assets updated and the failing test is ignored until we land 1027458 and it hits nightlies

Comment on attachment 8442635 [details] https://github.com/mozilla-b2g/gaia/pull/20725 Continuing to test, but everything is looking good.

Comment on attachment 8442635 [details] 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.

Comment on attachment 8442635 [details] 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.

In master: https://github.com/mozilla-b2g/gaia/commit/f00f57bd824149a0bc9dede0af530fd712d15b3e

2.0: https://github.com/mozilla-b2g/gaia/commit/9568c5ffb439e17f6623eb5462a279b1636d37bf