User story: As a user, I want to see the file type, size, resolution and date of creation of the content displayed in the gallery app

I added an "i" (info) button to the interface. The icon is same as in the video app. (Since this feature is basically the copy of the video app's info screen) @Rob: Just wanted to let you know about that, so that we can keep count of all the buttons :-)

I attached a screenshot of the current (and very basic) version of the information view for gallery images. We were debating whether we should a histogram of the image in there as well. It would be easy to do, but its value to the user may be limited. @kyee David Flanagan mentioned you were a (hobby) photographer. How valuable is a histogram really? Also, for the future, I could image that we could display additional metadata information like GPS coordinates, camera manufacturer or whatever else we can read from the EXIF data. :dmarcos has similar intentions with all the metadata stuff. So, the real question here. Do you want anything else here? Should we add the histogram here or just ship it as is?

I'd think that the Close button should be opaque. Is it unintentionally inheriting opacity < 1 from its container?

(In reply to David Flanagan [:djf] from comment #6) > I'd think that the Close button should be opaque. Is it unintentionally > inheriting opacity < 1 from its container? Agree. Tom, please take a look at the building blocks and they'll show you examples of what this should look like. https://developer.mozilla.org/en-US/Apps/Design/Building_Blocks/Confirmation?redirectlocale=en-US&redirectslug=Web%2FApps%2FGraphics_and_UX%2FBuilding_Blocks%2FConfirmation Also, I'm not sure if this is the right font. I'll flag Eric in visual for feedback as well.

Jacqueline will be providing additional feedback on the interaction side.

Everything looks great, however the only issue I can see is that the floating back icon is not ideal. I've attached a single page spec to illustrate an idea we could use instead of the back icon. Essentially, replacing the camera icon with the grid icon will allow users to quickly return to the grid through the toolbar. Users will still have easy access to the camera in the grid view. Let me know if you have any questions.

Comment on attachment 816817 [details] Screenshot.png Hi Tom... Please see Jacqueline's and my earlier comments. Almost there! Thanks, Rob

Thanks Tom for working on this, overall the code looks good, few points noted that i have commented in the pull request. Also, i tried taking screen shot of the info view in gallery app (press power + home icon at the same time), the screen shot of the info view loses it background grey (see attached)

(In reply to Punam Dahiya from comment #12) > Created attachment 821369 [details] > screenshot_info_view_missing_greybkg.png Hi Tom, I think this looks good but a semi transparent background is not needed. Seeing as the Gallery's background is black you can make the background 100% black, this will also solve the issue of the transparent close button. Thanks!

(In reply to Eric Pang [:epang] from comment #13) > (In reply to Punam Dahiya from comment #12) > > Created attachment 821369 [details] > > screenshot_info_view_missing_greybkg.png > > Hi Tom, I think this looks good but a semi transparent background is not > needed. Seeing as the Gallery's background is black you can make the > background 100% black, this will also solve the issue of the transparent > close button. Thanks! Regarding the background: Fixing the opacity of button is no problem. It is a very easy fix. Do you really want me to make the background 100% black? What about the pattern that is applied to the current background. Do you still want this? Th current background loads 2 images. It seems to me that "gradient.png" has some transparency. So if you want an opaque background but still keep the gradient I need an new image: url("images/ui/pattern.png") repeat scroll left top, url("images/ui/gradient.png") no-repeat scroll left top / 100% 100% transparent Regarding the fonts: The screenshot was taken by running Gaia on my desktop browser. It used my default system font Helvetica. (and Lucia Grande for the close button) In the device it falls back to FFOX standard font.

I used a very subtle darkish gradient for the background now plus the standard FFOX grainy pattern on top of that. I found the gradient in the defauts of the video app's CSS. (Again, this picture was taken on the desktop, hence Helvetica)

(In reply to Punam Dahiya from comment #12) > Created attachment 821369 [details] > screenshot_info_view_missing_greybkg.png I can not reproduce that on my Leo phone. I tried taking sreenshots several time, but they always came out right.

Hi Tom, Pull request looks good. One last thing that i commented in pr also is about calling updateimageinfo on click of info button instead of showfile, previousfile and nextfile. This will avoid extra work every time a user is opening gallery file. what do you think? With above point fixed, its r+ from me, i tested the screen shot and it came out fine. I will suggest getting a ui-review done of the updated flow and info view background before landing the patch. Thanks

Also, it will be good to see unit/integration tests added for this feature, I know we have a separate PR for gallery tests, not sure if the info view test cases are included in bug 915661 PR

@jsavory: I hope I packed the app correctly for a UX-review. Please let me know if you are satisfied with the background, the layout of stuff and the new "back" button workflow. Thanks

Comment on attachment 820066 [details] Github Pull Request Looks good, Thanks Tom for working on it!

Jacqueline, Could you please do the ux review so we can close this one down? Thanks Hema

Removing the flag for Casey; this one is on Jacqueline's list for today.

Tom or David, UX needs to see a screenshot of this (or a pointer to what the correct screenshot is, if one exists) before we can understand the patch and advise. If someone can post one, Jacqueline can review. Thanks!

DJF - In Tom's absence could you please provide the screenshot for UX

Stephany, Jacqueline: Tom attached an app.zip file that should enable you to try out his patch live. This is the one he set the ui-review flag on for Jacqueline. Or, failing that, it looks like this attachment is the latest screenshot: https://bugzilla.mozilla.org/attachment.cgi?id=822389 If neither of those work, please let me know what you need.

Not a 1.3 blocker, however targeted for 1.3

Thanks for the screenshot, David. Everything looks good to me. Peter - I was wondering if you could take a quick look at the screenshot David references in comment 25 to ensure that it looks correct to you.

Hi Jacqueline/David, Please see the attached zip for a Spec Patryk did for Video Details. It should be followed for this feature as well. The current visuals and layout are off (as of the image provided in comment 25 by David). It should follow our pattern for dialogs. The current one has the wrong background and button, as well as layout and font issues. Patryk's PSDs spec this out nicely.

I had no idea the Spec Sheet existed. I will take a look at it again next week. @djf: Are we in a rush to deliver this? If not, I'd be happy to take a look at it on Monday and make the necessary fixes.

(In reply to Tom Herold from comment #29) > I had no idea the Spec Sheet existed. I will take a look at it again next > week. > > @djf: Are we in a rush to deliver this? If not, I'd be happy to take a look > at it on Monday and make the necessary fixes. Tom, This is a target feature for 1.3. If you are able to address it next week, that would be wonderful! (Updating the milestone) Thanks Hema

Hey Peter, I need some more concrete feedback for fixing the style of this. I attached a screenshot comparing this view with the video app's info view. Looking at the design spec for the video app it seems that it was never followed to the point or changed a long the way. However, since I so far have been following the style of the video app there are some difference that I need input for: 1) Both don't have the dividers as outlined in the spec. Do you want them in both or none? 2) Both apps use the "i" button icon with a straight "i". (I just copied the video app's icon.) Do you want the slightly angled one? 3) Higher up in the discussion it was proposed to use an opaque background for the gallery's view. Is this still the case? If so, please let me know whether you want a gradient or not. (and what colors for the gradient) 4) As far as offsets and fonts are concerned they are the exact same as in the video app. (see screenshot) I hope this covers everything to get this finished.

(In reply to Tom Herold from comment #31) > Created attachment 8334133 [details] > GalleryVsVideo.jpg > > Hey Peter, > > I need some more concrete feedback for fixing the style of this. I attached > a screenshot comparing this view with the video app's info view. > > Looking at the design spec for the video app it seems that it was never > followed to the point or changed a long the way. However, since I so far > have been following the style of the video app there are some difference > that I need input for: > > 1) > Both don't have the dividers as outlined in the spec. Do you want them in > both or none? > Peter: Both apps should use the divider lines as spec'd. > 2) > Both apps use the "i" button icon with a straight "i". (I just copied the > video app's icon.) Do you want the slightly angled one? > Peter: The angled info icon is new for 1.3, so it should be the one that is used. The new angled icon was attached with the spec zip file. > 3) > Higher up in the discussion it was proposed to use an opaque background for > the gallery's view. Is this still the case? If so, please let me know > whether you want a gradient or not. (and what colors for the gradient) > Peter: Both gallery and video apps should use the same background, which is the same background used for ALL dialogs. This dialog background has a radial semi-transparency in it and also a noise pattern. However, the one they implemented for video app is actually not correct. Please align both apps to use this background. This background image was included in the zip file I attached in comment 28 in the file called Video_Details_130820.psd. I will also attach it in the following comment. > 4) > As far as offsets and fonts are concerned they are the exact same as in the > video app. (see screenshot) > Peter: Again, both apps should be following the spec as far as text size and layout are concerned. If they are off, they need to be aligned. > I hope this covers everything to get this finished.

Tom, Could you please provide an update -- I am assuming that you are working on this based on your last email. If not, please let us know so we can reassign it. Thanks Hema

Hey Hema. Sorry for the silence, but I was out of town for some days. I am working on this and should expect the final patch this weekend.

Update whiteboard tag to follow format [ucid:{id}, {release}:p{1,2}, ft:{team-id}]

Update whiteboard tag to follow format [ucid:{id}, {release}:p{1,2}, ft:{team-id}]

I updated the pull request to make the dialog look ore in line with the UX spec. The CSS changes are both to the gallery and the video app. Since I didn't find a reference dialog in the system I could only manually compare this patch with the UX spec when fine tuning the background opacity.

NI Fabrice (1.3 sheriff) to get his approval for landing this feature for 1.3 once we have all the reviews completed this week.

Comment on attachment 820066 [details] Github Pull Request Hi Tom, Thanks for fixing styles in both video and gallery, it looks good. Only thing that we should fix is the opacity in background. As Peter clarified in comment https://bugzilla.mozilla.org/show_bug.cgi?id=925216#c32, info dialog background should have radial semi-transparency, similar to other dialogs, we can take out the opacity.I have left comment in github also. With that fixed let's set it for ui-review with Przemek Abratowski, as Peter is OOO.

Form left to right: 1) Background pattern/image + 0.95 opacity applied 2) Example from the spec Peter provided 3) Background pattern/image with no additional opacity. #1 looks more like the original, but I agree it doesn't feel radial. On the other hand #3 hardly shows any transparency at all. I get the feeling that in order to get this right and have more prominent radial transparency effect the background image has to be modified. Or am I missing something here? I will post a comparison image later.

Well, this is interesting. It looks like if the background is computed with the CSS styles that mix the pattern and the gradient it looks different then when using Peter's background image (which already merges the pattern+gradient into one image) from comment #33. Either Gecko renders it differently then Photoshop or the new background image just has a lot more radial transparency. @Peter can you please send me the background image again, but this time as separate gradient and pattern. Thank you.

Peter La is OOO, NI Przemek for the images (separate images) so we can wrap this up soon.

You should be using the background that's already in the building blocks for this dialog. Here is a link to a bug that contains the correct building block. https://docs.google.com/a/mozilla.com/spreadshee/ccc?key=0AtVT90hlMtdSdEd4TVVjWXNfU3ctMlVhWFRrWkpweVE#gid=39 -Przemek

Sorry wrong I sent the wrong link, this is the correct link for that background building block, http://buildingfirefoxos.com/building-blocks/confirm.html

What's the latest status on this for 1.3? Thank you!

(In reply to Jean Gong from comment #46) > What's the latest status on this for 1.3? Thank you! I'm waiting for everything to be r+ before I give approval. There are many attachments, if some are obsolete please mark them as such.

Tom, Thanks so much for your contributions to get this feature completed. Could you please provide an ETA on when we can update the background image to address the opacity concern. We are trying to see if we are able to get this wrapped up before our 1.3 feature complete date (12/9). If not, it will have to move on to the next release vehicle. Please provide an update. Thanks Hema

I have serious reservations about comment #9 however. The floating back button was a solution (before Jacqueline's time, maybe?) to a serious usability problem. Switching back to the grid button does not seem like a good idea, and I'm not willing to land that as part of this patch without more UX discussions. In any case the back button would more properly be handled as a separate bug, not tacked on to this info display bug. I've added some additional review comments on github to Tom's patch. Since this bug is still assigned to me and the deadline is approaching, I'm going to start working on it myself, I guess.

I wonder if we can get the background right by actually using the building block that Prezmik refers to in comment #45.

Geez. The icons attached in comment 28 are 170kb each. And they're in this pull request in that form!

This new pull request is based on Tom's, but has a number of changes: 1) it is based on the dialogs in shared/style/confirm.css, so we automatically get the right backgrounds and buttons, and don't have the CSS headaches that have held this feature up for a month. 2) The feature has been moved into separate info.js and info.css files which are lazy-loaded when needed. 3) I've removed the back button change that Jacqueline requested in comment #9. I think we have user research showing that a grid icon in the lower left just does not work for users. We have the floating back button for a good reason. If UX really has changed its mind about this, they can file a new bug to change the back button and not tack it on to this feature. 4) The info is now a <dl> with <dt> and <dd> elements, which makes more semantic sense than a <ul> with <li><span/><span/></li> elements. 5) The icons supplied by Peter in comment 28 had a bizarrely large file size. I've used the icons from the video app instead: they are visually indistinguishable. 6) I've modified the video app to use a confirm dialog with a <dl> list and the same new info.css file that the gallery app does. So now both apps use identical markup and styles Punam: you were reviewing Tom's version of the patch, so I'm asking you to review this one as well. John: I'm also flagging you for review because this patch modifies the video app, and because your Monday starts well before Punam's does. (I'd like to land this on Monday before the FC deadline) If you have time to review this, that would be great. But if not, Punam can do it. Fabrice: I'm setting needinfo for you because this cleaned up patch will need your approval to land. Note that I've deleted most of the attachments, so it should be simpler to understand what is going on.

Przemek: would you review these two screenshots and give UX approval to land? As noted above, I'm now using the building blocks directly, so the dialog background and the Close buttons should be exactly the same as anything from shared/style/confirm.css. I'm pretty sure that I implemented Peter's spec from comment 28 precisely. I tried to measure a screenshot and the pixels distances match Peter's spec to the best of my ability to judge text baselines by eye.

Jacqueline, I've set needinfo just to let you know that I have not removed the floating back button as you requested in comment #9. As noted above in comment #52, I think going back to the grid icon would be a bad idea. If you really want to get rid of the floating back button though, please file a separate bug for that and we can argue about it there :-)

Hi David, This looks great, the only difference that I see between your screenshot and Peters spec on comment 28 is that the lines of text are left aligned at 20px in your spec and 30px in Peters. Everything else, background and buttons looks the same. Przemek

Comment on attachment 8344154 [details] screenshot of the video app displaying file information Hi David, This looks great, the only difference that I see between your screenshot and Peters spec on comment 28 is that the lines of text are left aligned at 20px in your spec and 30px in Peters. Everything else, background and buttons looks the same. Przemek

Comment on attachment 8344155 [details] screenshot of the gallery app displaying file information Hi David, This looks great, the only difference that I see between your screenshot and Peters spec on comment 28 is that the lines of text are left aligned at 20px in your spec and 30px in Peters. Everything else, background and buttons looks the same. Przemek

Przemik, Thanks for spotting that. Its a trivial change to add 1rem of padding on the left for the text. I've attached two new screenshots showing the results.

I've updated the github pull request with a new version of info.css for both gallery and video apps that adds the extra indent for the text.

Comment on attachment 8344287 [details] screenshot of the video app displaying file information Thank you David, this is great.

Comment on attachment 8344288 [details] screenshot of the gallery app displaying file information Thank you David, this is great.

Comment on attachment 8344152 [details] link to patch on github Making info-view a confirm dialog form element, has made styles neat and simplified in both gallery and video app. Looked at all the flows and the patch looks good. Thanks for working on it, r+ from me.

David, approval for 1.3 from me.

The tree is reopened. I've rebased and pushed the new commit to github in order to restart the Travis job.

Restarting Travis again. It was green on all tests except the test_agent tests which had an unrelated failure. Previously I was green on the test_agent tests and had a failure on the marionette tests. If I don't get all green this time, I'm going to take those past two runs and count all the overlapping greens and call it good enough.

Landed to master: https://github.com/mozilla-b2g/gaia/commit/55cc0a04d9041231f169dc669e2f5256eecd8855

Comment on attachment 8344152 [details] link to patch on github It uses building block for info dialog. Thanks about this.

Removing ni? flag as bug is fixed.