1133979 - [Hacker Marketplace] View information about apps and add-ons before downloading

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Hackerplace, defect, P1)

Comment 1

[Michael Henretty [:mikehenrty][:mhenretty]]

Attachment: [screenhot] addon w/ affected apps

Can you guys take a look here?

Comment 2

[Amy Lee [:amylee] UX]

Comment on attachment 8578861 [details]
[screenhot] addon w/ affected apps

Hi Michael, 

Here is my feedback:

1. Default app icon – Will confirm if a new icon is needed

2. Background/header colour should be #F4F4F4

3. App name/app description/affected apps list font colour: #4D4D4D

4 Header/Affected App Header/Author name/Open button text: #858585

5. Left align all elements – Icon, text, header,

6. When app is already installed, “open” button should change to white with grey text. See web components for styling - http://gaia-components.github.io/gaia-components/

7. Error message dialogue box - When installing the sharing app I get a "Install Error: No network" pop-up when I don't have a connection. The text looks a bit big, please reference the web components style under Dialogue -> Alert http://gaia-components.github.io/gaia-components/

8. On-Press Button States: If static button is blue, on-press state should be white with grey text (see web component button style). If static button is blue, on-press state should be white with grey text. This also applies to list view buttons.  

Thanks

Comment 3

[Amy Lee [:amylee] UX]

Attachment: Hackerplace_App_Screen_Edits.png

Visual reference for UI review

Comment 4

[Michael Henretty [:mikehenrty][:mhenretty]]

Attachment: [PR] updates based on feedback

Comment 5

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8580191 [details] [review]
[PR] updates based on feedback

Actually, it's clearer if this PR is on bug 1133976. Sorry about the confusion!

Comment 6

[Michael Henretty [:mikehenrty][:mhenretty]]

Attachment: [PR] update based on feedback

Comment 7

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8580240 [details] [review]
[PR] update based on feedback

master: https://github.com/fxos/directory/commit/7691f8d8e3047a921a0db8ccd42c9e4cdf691ba3

Amy, can you have a look now?

Comment 8

[Amy Lee [:amylee] UX]

Attachment: Hackerplace_App_Screen_Edits_v2.png

Hi Michael, 

Attached is my feedback. Thanks!

Comment 9

[Amy Lee [:amylee] UX]

Comment on attachment 8580240 [details] [review]
[PR] update based on feedback

Attached my edits

Comment 10

[Amy Lee [:amylee] UX]

Attachment: icon_colour.png

Minor catch: 
The "X" icon colour should be #858585. Thanks

Comment 11

[Michael Henretty [:mikehenrty][:mhenretty]]

Attachment: [PR] update details view

Comment 12

[Amy Lee [:amylee] UX]

Hey Michael, 

Is it possible to increase the line height in the list view by 4px top and bottom? This would only be for hackerplace and not the web components since we have the unusual case of having a button in a list. This was brought up by the visual team. Thanks!

Comment 13

[Amy Lee [:amylee] UX]

Attachment: 2015-03-24-14-47-46.png

Also I noticed that the alignment is slightly off between the app name and the creator name. Is this an alignment issue with the font? (See attached).

Comment 14

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8582065 [details] [review]
[PR] update details view

Looks good, but you have more refinements to do in comment 12.

Comment 15

[Amy Lee [:amylee] UX]

Attachment: Hacker_Marketplace.png

Hi Michael, 

Apologies I should have just put the additional edits into a list for you. 

Here are the new edits: 

1. Increase the line height in the list view by 4px top and 4px bottom. This would only apply to hackerplace and not the web component since we have an unusual scenario where there are buttons in list view. 

2. Hairline is missing from the Apps/Add-Ons tabs

3. The alignment is slightly off between the app name and the creator name in list view and in description page. Is this an alignment issue with the font? (see attachment in comment 13).

4. Please change the header colour to #9ab0c0 and make the font/icon colour to white #ffffff. 

Thanks you!

Comment 16

[Amy Lee [:amylee] UX]

One more:

5. App/Add-On Description Screen – Can all the content below the header in the add-on/apps description page be bumped down 14px? After the header colour change, the content was looking too close to the header. 

Thanks!

Comment 17

[Michael Henretty [:mikehenrty][:mhenretty]]

master: https://github.com/fxos/directory/commit/ae52285dde0ebbd40cb5b8d541a53006bea232d9

(In reply to Doug Sherk (:drs) (use needinfo?) from comment #14)
> Comment on attachment 8582065 [details] [review]
> [PR] update details view
> 
> Looks good, but you have more refinements to do in comment 12.

Yeah, I'll need to do another PR for the changes mentioned in comment 15.

Comment 18

[Amy Lee [:amylee] UX]

Attachment: hackerplace_icon_x1.5.png

Attached is the Hackerplace icon for homescreen. Thanks!

Comment 19

[Amy Lee [:amylee] UX]

Attachment: [e-mail]

Updated default app icon

Comment 20

[Amy Lee [:amylee] UX]

Attachment: Hackerplace_Icons.zip

Attached is default app icon (x1.5, x2.25) and Hackerplace homescreen icon (x1.5, x2.25). Thanks!

Comment 21

[Amy Lee [:amylee] UX]

Hi Michael, 

Just noticed some things with the "affected apps" section in the add-ons description page

1. When there is not internet connection, "affected apps" isn't displayed in the add-on description screen.
2. When add-on is installed, "affected apps" is no longer displayed in the add-on description screen.

Thank you

Comment 22

[Amy Lee [:amylee] UX]

Attachment: Confirmation_Dialogue.png

Can we update the current confirmation screen with the new web component version? (see attached). Thanks

Comment 23

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Amy Lee [:amylee] from comment #22)
> Created attachment 8584008 [details]
> Confirmation_Dialogue.png
> 
> Can we update the current confirmation screen with the new web component
> version? (see attached). Thanks

Unfortunately, this comes from the Gaia system app, so we would have to add the web component there, and update the install functionality to use this web component. We should definitely do this, but it is not in the scope of this bug.

Comment 24

[Michael Henretty [:mikehenrty][:mhenretty]]

Attachment: [PR] update visuals part 3

Doug, could you take a look here? These are updates based on Amy's latest comments. I have not fixed the button states yet, that is waiting on bug 1150308. I also couldn't reproduce the affected apps not showing up part, but I am continuing to investigate.

Comment 25

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8587109 [details] [review]
[PR] update visuals part 3

master: https://github.com/fxos/directory/commit/abfc6eddc56b8dfec8e2b10844fe3ab0cde77565

Comment 26

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8587109 [details] [review]
[PR] update visuals part 3

Flagging Amy for another UI review.

Comment 27

[Amy Lee [:amylee] UX]

Attachment: Hackerplace_Edits.png

Hi Michael, 

Here's my review

1. Please left align the close icon in the header (see attached).
2. The app icon should be 5.0 rems. It looks like it might be 0.5 rems bigger. 
3. Please right align upload icon (see attached). 
4. Please remove rounded corners in the on-press state in the pop-up confirmation screen (see attached). 
5. Add-On Screen: When app is not installed, it lists under "Affected Apps" All apps. When I install the add-on, the "Affect Apps" list changes and lists all of the affected apps. The list goes past the screen but it doesn't allow me to scroll down to see the bottom. Bug? 

Thanks!

Comment 28

[Michael Henretty [:mikehenrty][:mhenretty]]

Attachment: [PR] alignment tweaks

This handles 1-4 of comment 27. Doug?

Comment 29

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8588836 [details] [review]
[PR] alignment tweaks

Looks good, but we should fix this in gaia-header instead if it's a problem there.

Comment 30

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8588836 [details] [review]
[PR] alignment tweaks

master: https://github.com/fxos/directory/commit/4565779aea347ee18b239b1b59cb93390b30068f

Comment 31

[Michael Henretty [:mikehenrty][:mhenretty]]

Attachment: [Github PR] stylistic and small bug fix updates

This PR includes the latest Gaia button which addresses most of Amy's concerns. In this process, I found a bug in the platform (bug 1160403) which prevents us from fixing the small outline around buttons with the pressed state. If platform fixes this, we won't have to do anything in the app.

I also fixed various other small things which allow me to run this in the browser more easily, and handle offline errors better. The next step is to fetch the app list from a database somewhere.

Comment 32

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8600207 [details] [review]
[Github PR] stylistic and small bug fix updates

Generally looks good, though I left a few small comments on the PR.

Maybe it would be a better use of our time to build an easier way to submit apps, rather than pulling the app list from a database. Having to modify the code statically and publishing to gh-pages isn't ideal, but it's not much simpler to maintain a database.

Comment 33

[Michael Henretty [:mikehenrty][:mhenretty]]

Addressed most of your comments (except one I will fix in a followup). Also, discussed on IRC about the way forward for making it easier to submit apps. I'll look into that next.

master: https://github.com/fxos/directory/commit/3ea3b245f8040cf2c60c3219bad7172c73a1706b

Comment 34

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8600207 [details] [review]
[Github PR] stylistic and small bug fix updates

Alright, let's do another UI review. 

You will unfortunately still see the faint blue outline in the buttons. This is a platform bug, which I have filed bug 1160403.

If there are any new tweaks we want to make, let's file a new bug since this one has already had several PR's and is getting hard to track at a glance. Let's keep this UI review for the stuff that has already been mentioned.

Comment 35

[Amy Lee [:amylee] UX]

Comment on attachment 8600207 [details] [review]
[Github PR] stylistic and small bug fix updates

Hi, 

The visual refinements look good now so + for me. I have a question about the Add-Ons description screen. When Wi-Fi is disabled the "Affect Apps" section isn't shown but when it's connected to Wi-Fi, the section appears. Is this a bug?

Also, I think we need a hairline between the install button and the app name in list view so people know that you can tap on the app name to get to the description screen. I'll file a new bug for this. 

Thanks!

Comment 36

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Amy Lee [:amylee] from comment #35)
> Comment on attachment 8600207 [details] [review]
> [Github PR] stylistic and small bug fix updates
> 
> Hi, 
> 
> The visual refinements look good now so + for me. I have a question about
> the Add-Ons description screen. When Wi-Fi is disabled the "Affect Apps"
> section isn't shown but when it's connected to Wi-Fi, the section appears.
> Is this a bug?

Yup, that's a bug. I thought I had fixed that with this commit, but I guess I missed a case. I'll look into it.

> Also, I think we need a hairline between the install button and the app name
> in list view so people know that you can tap on the app name to get to the
> description screen. I'll file a new bug for this. 

Thanks.

Comment 37

[Jacqueline Savory [:jsavory] UX]

Comment on attachment 8600207 [details] [review]
[Github PR] stylistic and small bug fix updates

I reviewed this with Amy and it looks good to me, ui-review+

Comment 38

[Michael Henretty [:mikehenrty][:mhenretty]]

Great, let's close this for now and work on the issues mentioned in comment 35 in other bugs.