Page MenuHomePhabricator

Make black background in gallery work on device
Closed, ResolvedPublic

Description

Issue

Initially, we were using a black background in gallery but portrait images were displaying horizontally stretched (T248443). In fixing that issue, we learned that the black background is not being displayed correctly on portrait images. This issue is present only on device, the browser shows the correct behavior. After several failed attempts, we've decided to leave the background white only temporarily so as to not hold up the release for this.

What follows is a dump of everything we've learned and gathered so far on this issue.

Notes

The black background adds value to the app experience, it gives the gallery a bit of a special cinema-like feel 📽

The bug is only present when interacting with app on device, the browser works as expected. This makes it a bit trickier to debug. We can’t quite get the behavior on the browser and on the device to reconcile. We have tried a few different things:

  • setting a combination of different properties on flex child item img
    • using align-self to override the default value [1]
    • setting flex [2]
    • using flex-basis [3]
  • adjusting padding [4]

In all those case we either end up with images still stretched vertically or horizontally, or the white background on landscape or portrait. Could it be some of the flex properties not being supported or recognized on device? The issue is getting the same for both landscape or portrait: what's working one way is at the sacrifice of the other.

Note that the cat article has a good gallery to test this bug with because it contains both landscape and portrait images, as well as one svg for which we need the white background on image for (more about that in T236302). Booting up the WebIDE (for on-device debugging) we can further confirm that behavior of the img size overlapping parent divs, here with a red border to visualize debugging:

Screen Shot 2020-04-19 at 10.10.09 PM.png (720×1 px, 930 KB)

We also tried something perhaps a bit more out there [5] with an extra parent div, and this was looking okay(ish?) on device but broken on browser. This I'm leaving in sample branch T248443-images-jio2-gallery-img-idea.

Solution ideas

  • One possible solution the solution is to constrain width OR height based on the aspect ratio of the image vs. the aspect ratio of the space available
  • Is there a way to know beforehand via the respective API call if the image is portrait or landscape? With that info, we could perhaps set the flex properties dynamically
  • if for some reason we had to move away from flex in gallery thumbnail, we could simulate what's being done on desktop Wikipedia using display: table instead

Links

[1] - https://stackoverflow.com/questions/37609642/why-does-flexbox-stretch-my-image-rather-than-retaining-aspect-ratio
[2] - https://stackoverflow.com/questions/29467660/how-to-stretch-children-to-fill-cross-axis
[3] - https://stackoverflow.com/questions/34352140/what-are-the-differences-between-flex-basis-and-width
[4] - https://stackoverflow.com/questions/44009015/use-flexbox-and-maintain-a-11-aspect-ratio-even-though-content-is-sized-differe
[5] - https://stackoverflow.com/questions/30330845/flexbox-image-scale-to-height-and-keep-aspect-ratio

Event Timeline

AMuigai triaged this task as Medium priority.May 12 2020, 6:37 PM

Note that you will see a blink changes to the image size, this can be (hopefully) fixed by covering it when having the loading gif layer in T253466: [IMPROVEMENT] add a loading gif on the gallery when the image is loading

Working as expected, moving it for product signoff.

Is this in production? I still see a white background in some images: examples can be seen in the articles https://en.wikipedia.org/wiki/Nairobi and https://en.wikipedia.org/wiki/Nairobi_National_Park

Looking at the Nairobi article, this is the one picture I'm seeing that I believe you are referring to:

Screen Shot 2020-06-02 at 10.13.26.png (638×480 px, 115 KB)

What's happening in this case is that part of the image is transparent. Many articles have images of this type, known as SVGs, where part or all of the 'background' of the image itself is transparent. To correctly handle the SVG cases, what we're doing is that we apply a white background to the image itself (as opposed to the background of the gallery screen where the image is displayed, which is black). This is what we're doing on the iOS app as well, see this thread for more info.

This is what the 'raw' SVG above looks like:

Screen Shot 2020-06-02 at 10.13.21 AM.png (656×614 px, 216 KB)

The gray squares represent the transparent part. In order to fully appreciate the white background solution for SVGs, check out this case (cat article, fourth image in gallery):

Raw imageWith white backgroundWithout white background
Screen Shot 2020-06-02 at 10.45.47 AM.png (302×608 px, 90 KB)
Screen Shot 2020-06-02 at 10.45.11.png (638×480 px, 55 KB)
Screen Shot 2020-06-02 at 10.45.56.png (638×480 px, 39 KB)

Does this make sense? I wasn't able to find an image with white background in the Nairobi National Park article (tested on banana phone). And yes, this branch has been merged to master already.

Seeing some images with a white background on Jio 2, will file a new bug.