[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Page MenuHomePhabricator

[M] Quick View: Add search result page images to quickview on Special:Search
Closed, ResolvedPublic

Description

{F35423970}This story is one part of all the contents that go in the quick view mentioned in T306898

Image specifications in quick view

  • The header image in the quick view should be the same image used in the thumbnail/Go bar (see T306883). This image should match the one shown as thumbnail in the results so that the quick view could be connected to the result shown.
  • The width of an image box should be the width of the quick view component.
  • The height of the image box should be fixed for landscape image.
  • The height of the image box should be variable for portrait image.
    • The height should not go beyond a maximum height as specified in figma for Portrait image.
    • If the width of the quick view component shrinks on small screens then the height will adjust to retain its proportion.
  • The image should fill the image box and align to the top of the image similar to the rules prescribed for thumbnails. See option 4 here as an example.
  • When the image is not present do not show anything. The contents below the image area should take its place.
  • Show placeholder image if the image is taking longer to load.

Check Figma for the specs.

Landscape image (when the image width is larger than its height)

Special_Search (16).png (1×1 px, 277 KB)

Portrait image (when the image height is larger than its width)

Special_Search (17).png (1×1 px, 343 KB)

Event Timeline

Sneha renamed this task from Add search result page images to quickview on Special:Search to Quick View: Add search result page images to quickview on Special:Search.Jun 10 2022, 6:37 PM
CBogen renamed this task from Quick View: Add search result page images to quickview on Special:Search to [M] Quick View: Add search result page images to quickview on Special:Search.Jul 27 2022, 4:51 PM

I have also implemented an additional AC:

  • If the image is in landscape and its height is SMALLER than the fixed height. The space allocated to the image should shrink (Eg the margin at the bottom of the image should always be the same)

@Sneha what shall I do while the image loads? Should I delay the actual image to show (then it will jump when the image loads), or should I show a placeholder image of any sort?

Change 831873 had a related patch set uploaded (by Simone Cuomo; author: Simone Cuomo):

[mediawiki/extensions/SearchVue@master] Quick View: Add search result page images to quickview on Special:Search

https://gerrit.wikimedia.org/r/831873

@SimoneThisDot It should show the placeholder image. Text jumping around will be an abrupt experience. Also approximately how long would it take to load images if the someone has a reasonably good internet? Also thanks for adding the additional AC. I will add the loading one in the description too.

@Sneha It should not take long at all, but I always like to develop this using slow connection that that could also take a second or two, so a placeholder would really improve UX.

For placeholder do you mean the grey boxes shown in the figma design, or do we have an actual placholder image?

@SimoneThisDot yes for placeholder it is the grey boxes with grey icon shown in the figma designs.

Change 833765 had a related patch set uploaded (by Simone Cuomo; author: Simone Cuomo):

[mediawiki/extensions/SearchVue@master] Quick View: Add search result page images to quickview on Special:Search

https://gerrit.wikimedia.org/r/833765

@SimoneThisDot this has similar challenges as the Table of Contents tickets.

  • When switching to an article with an image to one without an image persists.
  • As well as accounting for this, some general graceful error handling so that if we get empty, bad or malformed responses we also suppress the component actively.
  • Same here for when changing between sections, with images we would be needing to wait for the loading as well as the api response, so again might be worthwhile at least when waiting for the API response to smooth out the transition. I'll create a task.

Style note:

  • We should increase the padding at the top of text snippet when there is no image

As with the other task, I am going to work the problem raised (remove image and gracefully handle errors).

@Sneha Vadim has asked a good question during the code review regarding portrait vs Landscape.

The question is: Do you have an expectation of what makes a portrait Image? The current code (the one I developed) just assumes that an image is a portrait if the height is greater than the width, but Vadim argues that it would probably be better to just assume a portrait image if the ratio is greater (so 1.5 height to 1 width).

Before making a decision, I wanted to ask if you have any preferences in which method we use. Remember that the main difference is in the way we set the height. If it is a landscape it is fixed, but if it is a portrait we allow the image to become higher.

@SimoneThisDot I had the same assumption and expectation that if the height is higher than width we would consider it portrait. I would like to better understand the user or technical benefit of the new proposal.
Also want to note that when the image becomes higher (because it is a portrait) we still have max height as defined in Figma. And if the image is taller than the max height we would crop at the bottom.

There is not technical difference, the only produced side effect is that the proposed approach (to set a minimum ratio for portrait like 1.5/1) will probably produce a more consistent results to our Viewers..

So for example assuming the two max height are 300px for landscape and 600px for portrait ( i do not remember the correct number):

Image sizeContainer height with height > width methodContainer height with 1.5 /1 method
200 x 200200200
400 X 400300300
400 x 450450300
400 x 500500300
400 X 550550550
400 x 700600600

As you can see from the above table. The main difference in not using a ratio is that the QuickVue has more chances of returning to a different heights, than if we use the ratio.

NOTE: The amount of "square" or similar size images is very limited, so I am not really sure the issue is really big and the above table is really just an example. but I expect 90% of the portrait to be at least 1.5 ratio anyway!

Change 831873 merged by jenkins-bot:

[mediawiki/extensions/SearchVue@master] Quick View: Add search result page images to quickview on Special:Search

https://gerrit.wikimedia.org/r/831873

@Seddon what is this ticket blocked on? thanks!

Thanks @SimoneThisDot for outlining that. It makes sense to have consistent size for the image specially now that I am thinking of loading scenarios. We want less variations in UI as we load things. My original concern was that the image might be cropped. But now I am thinking about this more and also looking at our page preview implementation. It may be worth exploring having only two sizes one for landscape and one for portrait. And depending on the threshold we define image can either scale up or down to fit within its respective orientations. It looks like page previews are able to do that so we could do the same. I am not sure if you have access to their code but might worth looking.

FYI here's a breakdown dimensions of Commons BITMAP files.
I'm not sure how representative they numbers are for usage as PageImages thumbnail, though, but I'd guess it's going to be relatively similar.

Total
78884020
LandscapeSquarePortrait
5400604990300223974873
Wide (w >= 1.5*h)RegularSquare-ish (w <= 1.05*h)Square-ish (h <= 1.05*w)RegularHigh (h >= 1.5*w)
2411086529173432721752854425149762398144209
Wide landscape30.6%}}
Landscape37.0%}68.5%}67.5%
Square-ish landscape0.9%}}
Square1.1%}3.1%
Square-ish portrait1.0%}}
Portrait19.0%}30.4%}29.3%
High portrait10.3%}}

The current binary landscape/portrait logic seems a bit off, IMO.

My understanding of the AC:

  • landscape images should always have a fixed height (of 0.64x the available width) = [landscape mode]
  • portrait images are allowed to stretch (up to 1.15x the available width) = [portrait mode]
  • images will fill available space

What this means:

  • very wide landscapes (where width > 1.57 the height; approx 30% of images) are in [landscape mode] but too wide for this fixed landscape width/height ratio; their edges will be cut
    • Note: in current implementation, heights are not exactly fixed; instead of sticking to the fixed height & cutting off sides, they will render in full (at a smaller height)
  • narrower landscapes and squares (approx 38% of images) are in [landscape mode] but too narrow for this fixed landscape width/height ratio; their bottom will be cut
  • short portraits (up to 1.15:1 aspect ratio; 2.3% of images) are allowed some wiggle room to display in accurate dimensions
  • taller-than-1:1.15 portraits are in [portrait mode] but too high for the portrait maximum width/height ratio; their bottom will be cut

Issue with this approach: since we don't mind a flexible height: why are we allowing short portraits to adjust height, but don't allow narrow landscapes & squares to do the same?
If we're allowed to adjust height, we ought to use the full range between certain minimum/maximum and adjust according to whatever best fits the images.
Only 2.3% of images render in accurate dimensions; up to 38% are not optimized for minimum cut-off within the given height ranges. Up to 30% more could do with less (or no) cut-offs by lowering/dropping the [landscape mode] height.

Or, when using the "portrait = h >= 1.5*w" definition:

  • very wide landscapes (where width > 1.57 the height; approx 30% of images) are in [landscape mode] & too wide for this fixed landscape width/height ratio; their edges will be cut
  • narrower landscapes, squares & regular portraits (approx 59% of images) are in [landscape mode] & too narrow for this fixed landscape width/height ratio; their bottom will be cut
  • extra tall portraits (10%) are in [portrait mode] & too high for the portrait maximum width/height ratio; their bottom will be cut

Issue with this approach: we seem to prioritize a more consistent height, but 1.5 seems too late to switch from [landscape mode] to [portrait mode] given that max-height will be ~1.15 the width.
This basically gets us stuck with 2 fixed heights; if height consistency is so important, it should be reduced to only 1; if multiple heights are allowed, see previous remarks about ranges.

@Seddon, @SimoneThisDot & @vadim-kovalenko Please correct me if my understanding or anything I've written above is incorrect!

I think we ought to reconsider what want to prioritize: consistent thumbnail height (so elements appear at the same places when clicking multiple results), or optimal thumbnail rendering (with as few sides cut off/detail lost as possible):

  • In the case of consistent thumbnail height: a 4:3 aspect ratio (w = 1.33*h) looks like a reasonable candidate:
    • About half of the images are wider than that (and would see their sides cut off) and half are taller (and would see the bottom cut off)
    • 4:3 is a pretty common photo format, a lot of images (~22%) converge on these dimensions exactly
  • In the case of allowing flexible height for optimal thumbnail rendering, I think we need to stop thinking about landscape/portrait and fixed heights & just figure out how far we are willing to let images naturally stretch. Anything below a certain maximum should be fine.

Another one: for images that exceed whatever dimension limits we impose: do we really want to cut off image detail (object-fit: cover;)?
IMO, cutting off the sides of images to make sure they fit whatever consistent box is important in things like the SpecialSearch results page thumbs & the Commons widget images grid, because an inconsistent presentation makes those lists distracting and harder to peruse.
In QuickView, I'd say the thumbnail becomes more "essential information", and cutting off its edges may cause essential information to be lost.
I like the MediaSearch QuickView approach (object-fit: contain;), where images are allowed a flexible height, but at a certain maximum will start to reduce in width (horizontally centered, sides are whitespace) instead of getting their edges cut off. See e.g. a search for "tower" for some examples.

My preferred implementation would look something like this:

  • no minimum thumbnail height (it is already possible for QuickView to open without a thumbnail anyway)
  • a fixed maximum thumbnail height of e.g. 1.15 times the available width (this is what was proposed now as maximum portrait height)
  • a fixed maximum width, the entire QuickView panel wide
  • images where the height is lower than 1.15 times the width (72% of all images) will render perfectly (at varying heights); not sides cut off anywhere
  • images taller than said maximum height (28%) are not cut, but simply displayed at said max height, with whitespace on both sides (MediaSearch-QuickView-style)

Thoughts? (esp. @Sneha)

Change 837662 had a related patch set uploaded (by Matthias Mullie; author: Matthias Mullie):

[mediawiki/extensions/SearchVue@master] Use existing thumb source while higher res variant is loading

https://gerrit.wikimedia.org/r/837662

As for placeholder: now that Special:Search already shows (lower-res) thumbnails (and should be able to expose that src & dimensions to SearchVue via wgSpecialSearchTextMatches - see below), I think we should progressively enhance that one.
Not only will it look better; we can immediately start rendering the image component with accurate dimensions.

We can probably immediately render the low-res thumbnail url (it'll be a bit noisy because it's going to be upscaled from the low-res thumbnail) while the browser transfers the actual (higher-res) image for QuickView in the background. Once that comes through, we swap sources.

Thoughts? (esp. @SimoneThisDot)

See https://gerrit.wikimedia.org/r/c/mediawiki/extensions/SearchVue/+/837662 for implementation example.

Change 833765 merged by jenkins-bot:

[mediawiki/extensions/SearchVue@master] Quick View: Add image placeholder while loading

https://gerrit.wikimedia.org/r/833765

Blocked on the deploy of SearchVue to beta for QA. A new ticket will be created for the additional improvements proposed by @matthiasmullie and @Sneha above.

@matthiasmullie
Below are my thoughts after processing all the information you gave (which by the way is very useful):

  • User intent: The reason I am leaning more towards fixed dimension is because the user intent here is seeing content (which could be a combination of snippets, image and other things) hence we do not want image to take up most of the space in quick view and push other content down which may be more useful given the user intent is different here than media search. If this was image search then doing what media search is doing (i.e. try to minimize image cropping) would makes sense. But in the text search context I think we should try to give all of the content somewhat equal chance of being seen or to be discovered.
  • Full image view: Also users can click on the thumbnails to see full image. Don't know how many would be aware of this functionality but there is already a way to do so. We could perhaps let users also see full image when they click on image in quick view.
  • Page previews: The current implementation of page previews has fixed ratios too and the consistent box creates a better experience in their context which may be true for our context too. One thing we have already improved is aligning images to the top so if there are portraits of people their heads are not cropped while in page previews they are currently cropped.
  • Possible solution: The maximum visibility of variety of content would be a high priority. Consistent box not so important although if we don't have the dimension info of the image before loading quick view than consistent box can help as a placeholder within which the image would load. We could either
    • have fixed/consistent ratio (and be okay with cropping) like page previews OR
    • only have max height as a constraint and allow for any number of dimension flexibility within it.
  • Loading images: For the loading part, I think we should not show the low res image when we are fetching the high res image. A loading indicator would be better than showing low res image stretched. However, like you said we may already know the dimensions of the image so we can show the appropriate sized grey box instead.

Once we agree we can document our solutions here T319209.

I agree that the image in QuickView, while important, is not necessarily the most vital piece of information and certainly shouldn't be allowed to grow so large as to drown out the rest of the content.
The maximum height I suggested matches the maximum portrait height that was used before (allowing it to grow even taller is a point of diminishing returns anyway) - I'm not advocating for allocating more space for the thumbnail (I think the current maximum portrait height is a very sensible one and would propose to keep that maximum), just for making better use of the space available.

1. Heights

The current constraints are not optimal: with the current fixed-width landscapes and flexible-up-to-a-point portraits, we're now allowing these heights:

  • 0 (in case there is no page image)
  • 268px (fixed landscape height)
    • Note: current implementation doesn't fully match spec & actually allows full 0-268px; something to fix
  • 420-486px (flexible portrait height)

I feel quite strongly that we cannot stick with the current implementation, because now we have the worst of 2 world:

  • there's very little consistency in image height
  • we're still cutting edges off of 97% of images
  • lastly, even within these height contraints, we're not optimizing for minimum image cutoff because of the binary landscape/portrait logic: a near-square landscape would actually look better (almost no cutoff) in the current portrait range, but is current forced in the fixed landscape height (causing significant vertical cutoff)

I'm sure that the acceptance criteria weren't meant to be taken so literally, which is exactly why I'm bringing it up again in order to make sure we're not interpreting things the wrong way & come out of this with a shared understanding that actually works.
I think we should make up our minds about what we think is most important.

Either we settle on a minimum amount of actually fixed heights, optimized to cut off as little as possible on average (4:3 ratio is a good candidate if we want only 1; I can run numbers for optimal spread if we want 2 or more) without getting in the way of other content (i.e. not exceed the current portrait maximum)
And if we go with multiple fixed heights, we probably shouldn't constrain it to landscape of portrait, but to whichever allows displaying most of the image.

Or we simply allow complete flexibility up to a certain maximum. Allowing the full range of 0-<current portrait maximum> would allow 72% of images to be rendered without losing any side detail.

Note: I prefer the latter, flexible one for 2 reasons:

  • there's not much need for a consistent height; different heights aren't jarring because users will only see 1 quickview at a time
  • we already can't have perfect consistency anyway, since some articles don't have images

2. Object fit

What we have now is that whatever parts of the image don't fit within the semi-fixed aspect ratios, ends up being cut off.
Even if we go with the most flexible implementation (see above), there's still going to be a maximum height that some images are going to exceed.

If we go with the flexible implementation from 0px up to a certain maximum, we'll never have to worry about too-wide images not fitting. Too high, however, is a possibility.
With fixed heights, we're guaranteed to have almost all images not fit perfectly.

Current AC state that the sides should be cut off (in case of too wide), or the bottom should be cut off (in the case of too high)
Another option I wanted to float is the MediaSearch approach, where we show the full image in accurate aspect ratio, but shrink it (as to not exceed whatever maximum height we've defined) and have white space on the sides.

Personally, I have a mild preference over the latter. While I think that using the full width & cutting off overflow looks a tad bit better (or probably a lot better if we go for very constrained fixed heights), I think there's more value in being able to show the full image & not risk important parts being cut off.

FYI: Here's an example search for a bunch of images that wouldn't fit (too high).
https://commons.wikimedia.org/w/index.php?search=filew%3A%3C300+fileh%3A%3E450&title=Special:MediaSearch&go=Go&type=image

3. Loading

FYI: since landing Special:Search thumbnails, we do know the image's aspect ratio ahead of time, so we can immediately reserve the correct space and no content will have to jump around.
Note: this has not yet been implemented - should also be included in T319209 once we have the rest of the details figured out!

Progressive image loading is quite a commonplace optimization technique - many JPGs around the web have been built to front-load small parts of the image in order to be able to show something vague as soon as possible while the rest is still being transfered. See https://www.liquidweb.com/kb/what-is-a-progressive-jpeg/
Many users (especially those with poor connections) have experienced progressive loading before.
Given that it's already a popular technique to reach faster contentful paints & make things feel more responsive, and that we already have a low-res version available, I figured it may be of more value than showing nothing at all.

Here's 2 quick videos of both options in action (with load times severely exaggerated to mimick poor connectivity circumstances)

I prefer the progressive loading, but it's not a hill I'll die on :p

We can go either way, but we certainly must use the available information to set the dimensions ahead of time to prevent content from jumping around once the image actually completes loading.

Heights

  • Okay I understand the original AC of mixing of rules (fixed landscape height and variable portrait height with max) is problematic.
  • The fixed heights work well for page previews however quick view is much more dynamic and can expand based on amount of content so I understand it does not necessarily have to stay fixed. The consistent height originaly was deemed to be less distracting but perhaps it is not as distracting as I thought after trying out few examples. And the % of images that will be cut off is high as per your calculations with fixed approach. Also since we know the dimensions of the image beforehand the content will not have to jump around after loading. Given all these things makes sense to not go with this option.
  • I think that leaves us with the option "only have max height as a constraint and allow for any number of dimension flexibility within it." Which is something I also agreed as an option so we are on the same page. Perhaps for a maximum height we can take inspiration from page preview and restrain to that as maximum. Anything longer than that would be cut off from sides and bottom. That was the relative max height I tried to match in my wireframes as well.

Object fit

  • PageImage extension already ranks images that are beyond a certain ratio as low, so there is less chance of us getting something that is very wide or very long. Which reduces the % of long images that will get cut off.
  • While we want to reduce the amount of cutting of images we also want to ensure the page is scannable and looks good. I would go with the option of NOT having white space on the side especially because the user intent here is not searching media and by changing our height rules above we have already reduced the % of images that will get off. Also we are top aligning so that ensures the important part of the images are not cut off.

Loading

  • Thanks for these videos. It helps to actually see things in action. Btw option without low res image however needs a grey background. It would be good however to have loading indicator instead of the placeholder image.
  • So regarding Progressive Image loading: While it does create a user perception of faster loading, I would be wary of using blurry/pixelated images as it could also feel distracting especially for folks with poor eye sight. From the accessibility point of view I think I would be hesitant to use this approach. There was a study were users' expressed some discomfort with such a approach. And since we don't have a lot of user studies on this yet (especially with the wide range of users with eye conditions) I would not use this approach just yet.
  • And since it is not a hill you will die on :P I think we should stick with grey box with some loading indicator which we may cover in the loading ticket. It is great that we already know the dimensions. We can always reconsider progressive image approach later with more supporting data.

So, conclusions, AUIU (please let me know if I misconstrued anything):

  • The aspect ratio of the portrait image in Figma is 0.856236... (405/473px) - let's round that to 0.85. That will become the maximum height:
    • All images (about 72%) with a higher (=wider) aspect ratio will be able to simply render in their accurate aspect ratio within/below that maximum height, with no data being cut off
    • Taller images will be top-aligned & their bottoms will be cut off past that maximum height (i.e. not reducing width to accommodate for that max. height)
  • We'll make sure to immediately preserve a grey box in the exact dimensions the image will have (to avoid content jumping around once the image arrives)
    • Do we have a loading indicator SVG somewhere?

I have updated the AC in the follow-up ticket (T319209) to reflect above conclusions.
Can you update that ticket with details about what loading indicator to use?

Great thanks for updating AC. I think we can cover the agreed upon loading indicator in the loading ticket T318951. I will update T319209 for clarity.

And the conclusions in summary looks good to me.

Testing in enwikibetalabs

(1) landscape images - search term 'music'

Screen Shot 2022-11-02 at 5.31.27 PM.png (884×456 px, 311 KB)
Screen Shot 2022-11-02 at 4.17.22 PM.png (1×1 px, 572 KB)

(2) portrait images search term 'lilac' 2,200 × 2,835 pixels.

Screen Shot 2022-11-02 at 5.44.01 PM.png (906×452 px, 233 KB)