1029765 - Determine how screenshots should scale in featured app

Status: RESOLVED FIXED

(Marketplace Graveyard :: Curation Tools, defect, P4)

Description

[Chuck Harmston [:chuck]]

There's some ambiguity regarding how screenshots chosen for featured apps should scale and be framed while in featured apps. Filing this bug to track discussion and resolution.

Davor, would you mind explaining the limitations?

Comment 1

[Kevin Ngo [:ngoke]]

All the mocks for a screenshot featured app show a wireframe taking up the whole frame. When I worked on the screenshots, I scaled the image to cover the width for the time being as it was the quickest solution.

https://github.com/mozilla/fireplace/commit/e21ae060d3dc8c716ef9e472368bfa7f0807d02c#diff-503ea51444f4a463b7ad7b46f00d860bR102

Unfortunately, vertical screenshots have to be blown up to cover the width. Horizontal screenshots generally look fine since they have to scale down rather than scale up. 

Good-for-now proposal: scale vertical images to height rather than width. And show the background color in the surrounding space. Continue to scale horizontal images to width.

Comment 2

[Kevin Ngo [:ngoke]]

I guess it's more simply put to use background-size: contain instead of cover.

Comment 3

[Kevin Ngo [:ngoke]]

https://github.com/mozilla/fireplace/commit/5c95057aade911e6dd7d7f4cdd88c8f295b1c211

Comment 4

[Kevin Ngo [:ngoke]]

Attachment: background contain

Comment 5

[Chuck Harmston [:chuck]]

ni?tsmuse for verification

(Tony: either change status to resolved/verified if fixed, or reopened if not)

Comment 6

[krupa raj[:krupa]]

A featured app with a screenshot looks like https://marketplace-dev.allizom.org/curate/manage/apps/-

I don't think the current view is ideal. But I'll wait for Tony to pitch in.

Comment 7

[David Durst [:ddurst]]

If we're going to keep this with the choice of color in the background of the screenshot area, we should add some top & bottom padding to the screenshot image in the preview, so it sits inside the area with some cushion. My two cents.

Comment 8

[Tony Santos [:tsmuse]]

Phil, will you weigh in on this?

Comment 9

[krupa raj[:krupa]]

We can just match this section to look like the screenshot section in details page where we list all screenshots and allow the user to scroll. My two cents (We can collect all these cents and may be get a drink)

Comment 10

[Philip [:pwalm]]

Attachment: screenshot feed block options.png

I think originally we had intended that graphic to be less of a screenshot and more of a custom promotional image (which we could still do, but is more work on Scott's end, editorially-speaking). The attached png outlines some issues with that block design and a possible solution. Let me know what you guys think.

Comment 11

[Kevin Ngo [:ngoke]]

The swiping screenshots is sensible visually, but it is less performant to have more images, and the images in the swiping mock are actually even smaller than the awkward "but not really in portrait" mock.

How about: add some top and bottom padding to portrait screenshots, give more vertical space for portrait screenshots (double the height), and have a tap of the screenshot expands it to full screen.

Comment 12

[Philip [:pwalm]]

Yep, I like everything suggested except the tap to go to full screen in the feed. I think if you tap the screenshot it should take you to the app detail page.

Comment 13

[Kevin Ngo [:ngoke]]

https://github.com/mozilla/transonic/commit/cbc9c39c6eafa1b013b458c82441bf17b6d9fdf4
https://github.com/mozilla/zamboni/pull/2323