Draft PR: https://github.com/wikimedia/wikipedia-preview/pull/63

@eamedina can you please take a look at this [1]conversation and see if you can fix this issue on this PR?

[1]https://phabricator.wikimedia.org/T260228#6464710

As a matter of fact I'm in the process of clarifying what's the scope of this ticket (T261745), as it's being reference in a couple of different tickets, so it's good timing. Here is what I am seeing and can gather:

1. Take advantage of iiurlwidth and iiurlheight props to make each fullscreen image as light as possible while still looking good (non-pixelated) on device
2. Include white background for transparent images (from T260228)
3. Load gallery images gradually, as opposed to all at once, where the load order is basically dictated by where in the gallery the user starts browsing and how she swipes throughout after that
4. There's also T259854, where it's correctly pointed out that issue would get resolved by point number 1 in this list

What do we think? Does this scope make sense? I'm happy to update the ticket description here (and probably ticket title as well), just want to make sure all of this is correctly prioritized at the moment. Alternatively, either point 3 could be a ticket on its own, I'm fine either way

1. Take advantage of iiurlwidth and iiurlheight props to make each fullscreen image as light as possible while still looking good (non-pixelated) on device
2. Include white background for transparent images (from T260228)

This is within the scope

4. There's also T259854, where it's correctly pointed out that issue would get resolved by point number 1 in this list

Yes, I think number 1 correct this high solution image problem.

3. Load gallery images gradually, as opposed to all at once, where the load order is basically dictated by where in the gallery the user starts browsing and how she swipes throughout after that

When you read the source code, it doesn't load all the image at once, it reads the image in order current > next > previous, until you slide to next image, and repeat the order.

I think this ticket T263009: Part of next image visible when tapping previous/next button multiple times in Gallery view is a better place to address and revisit the optimization for the order of the loading image.

Thanks @eamedina for writing down all the points. Maybe it's already mentioned on the ticket and my eyes are unable to find it out. Will our effort through this ticket will also take care of different aspect ratio images looking good on different devices(example attached)? If you remember we did talk about it sometime back that we can optimize our existing image gallery code to use the maximum screen size available to us.

When you read the source code, it doesn't load all the image at once, it reads the image in order current > next > previous, until you slide to next image, and repeat the order.

I think this ticket T263009: Weird behaviour when tapping previous/next button multiple times in Gallery view is a better place to address and revisit the optimization for the order of the loading image.

Thanks for clarifying @hueitan, I agree T263009 would be a better place to address load order

Thanks @eamedina for writing down all the points. Maybe it's already mentioned on the ticket and my eyes are unable to find it out. Will our effort through this ticket will also take care of different aspect ratio images looking good on different devices(example attached)? If you remember we did talk about it sometime back that we can optimize our existing image gallery code to use the maximum screen size available to us.

@SGautam_WMF I think T262373 would be a better place for the update to use maximum screen size available because some portrait images conflict with the image attribution without the focus mode:

@SGautam_WMF I think T262373 would be a better place for the update to use maximum screen size available because some portrait images conflict with the image attribution without the focus mode

...and yet, I'm actually realizing it's simpler to fix the white background issue if we actually go ahead and update to use the maximum screen size now. I'm proposing to:

• make within scope for this ticket:
  • use of iiurlwidth and iiurlheight props to make each fullscreen image as light as possible while still looking good (non-pixelated) on device
  • include white background for transparent images
  • resolves T259854
  • use maximum screen size for image with the image info fixed on top with the same gallery background black color [1], as a temporary solution. That leaves the gallery closer to the finish line and sets us up to address image info improvements with T262371 and T262373
  • feel free to check out branch T261745-gallery-image-best-fit to visualize all of the above

• make out of scope for this ticket:
  • load order optimization, which could be addressed in T263009
  • gallery landscape rendering, maybe a new ticket is in order for that. I believe we need the image info improvements to be figured out before we fix gallery landscape view

If this makes sense for everybody I will go ahead and update this ticket description accordingly, and then officially move to code review column

[1]

@eamedina makes sense to me

Okay, I've updated the tickets description and opened the PR for review

https://github.com/wikimedia/wikipedia-preview/pull/63