Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Markdown images (ThumbnailImage) have inconsistent sizing behaviour #6359

Open
shaunanoordin opened this issue Oct 4, 2024 · 10 comments · May be fixed by #6360 or zooniverse/thumbnailer#27
Open

Markdown images (ThumbnailImage) have inconsistent sizing behaviour #6359

shaunanoordin opened this issue Oct 4, 2024 · 10 comments · May be fixed by #6360 or zooniverse/thumbnailer#27
Labels
bug Something isn't working

Comments

@shaunanoordin
Copy link
Member

UI Bug

Package: lib-react-components
Components: Media > ThumbnailImage, possibly Markdownz itself
Noted as of: fef769f

Images in Markdown (e.g. as used in a Project's About page) can have inconsistent sizing behaviour. To be specific:

  • Sub-Issue 1: Images in Tables will always shrink to the smallest size possible. (Screenshot: the animal image in the left column should have been 800px wide, but is instead as tiny as it can get)
    image
  • Sub-Issue 1a: the inconsistent sizing behaviour is, itself, inconsistent between PFE and FEM. (Screenshots: left is FEM, right if PFE. The animal image are both smaller than the intended 800px, but the FEM image is practically microscopic)
    image image
  • Sub-Issue 2: Markdown Images in FEM can't be sized larger than the original image (e.g. if your image is 400x400, you can't render it in Markdown as 800x800.)
  • Sub-Issue 2a: ...but in PFE, you CAN size an image larger than its original, resulting in inconsistent inconsistent behaviour between the two codebases.
  • Sub-Issue 3: the thumbnail service used by ThumbnailImage can't resize to 1000px wide or more.
    • But you won't notice it because ThumbnailImage has a fallback that displays the original image instead.

Analysis

Analysis of Sub-Issue 1: the chicken-and-egg hypothesis

  • From what I can tell, the Markdown image (ThumbnailImage component) is...
    1. rendered in a way that uses a lot of CSS flex, and
    2. has its intended size determined by the thumbnailer service instead of via HTML/CSS attributes (e.g. instead of using <img src="example.jpg" width="800", it goes <img src="thumbnails.zooniverse.org/800x/example.jpg") (Note: the image does have a max-width FWIW)
  • 🐔: the image will render itself in a way that will never exceed its container (in this case, a table's <td>)
  • 🥚: However, a <table>'s columns will size itself depending on the size of its content
  • As a result, the image renders itself in the smallest way possible.

Screenshot: we see the same animal image in two different kinds of tables. In both cases, the table layout logic seems to determine the width of left & right columns by looking for "fixed content" first - in this case, the text in the right column. It then leaves the remaining space for the "flexible content", meaning the image just fills up whatever space is left.

image

Analysis of Sub-Issue 2 & 2a:

Analysis of Sub-Issue 3:

Testing

To test this, simply try adding Markdown images with various explicit sizes (smaller than the original, larger than the original, etc) and in various situations (inside a table with a lot of other content, etc) to a test project's About page.

For example, heres my test project: https://www.zooniverse.org/lab/19242/about/results

And here's the Markdown code I used
## Markdown Test

This page is used to test how images are rendered in Markdown code on the Zooniverse. Specifically, how are _images with specified widths, inside tables,_ handled on both PFE and FEM?

--------

**Test 1: Image in Table**

|||
|:------------------:|-----|
|![animal.jpeg](https://panoptes-uploads.zooniverse.org/project_attached_image/1a4198cb-635a-4c0f-806e-796f3ac3877f.jpeg =800x)|This is a cell in a table. To your left, you should see a picture of a prairie dog(? or a gopher? I'm not a biologist dangit). The [source image](https://panoptes-uploads.zooniverse.org/project_attached_image/1a4198cb-635a-4c0f-806e-796f3ac3877f.jpeg) is 206x183 pixels in size, but the markdown specifies that the image should be presented as 800px wide. Question: how does the image appear to you on the About Page? On PFE? On FEM?|

--------

**Test 2: Image in less verbose Table**

|||
|:------------------:|-----|
|![animal.jpeg](https://panoptes-uploads.zooniverse.org/project_attached_image/1a4198cb-635a-4c0f-806e-796f3ac3877f.jpeg =800x)|Similar to table above, but text is shorter|

--------

**Test 3: Image, no Table**

Below, we see the same markdown code for the prairie dog(? or meerkat??) image, with the requested 800px width, but outside of the bounds of a table. How does it look like? On PFE? On FEM?

![animal.jpeg](https://panoptes-uploads.zooniverse.org/project_attached_image/1a4198cb-635a-4c0f-806e-796f3ac3877f.jpeg =800x)

(On FEM, we expect the image to be displayed 206px wide, because the thumbnailer service won't increase the size of an image.)

--------

**Test 4: Image, no Table, bigger**

For fun, here's what it looks like when we say the image should be 1000px wide:

![animal.jpeg](https://panoptes-uploads.zooniverse.org/project_attached_image/1a4198cb-635a-4c0f-806e-796f3ac3877f.jpeg =1000x)

(On FEM, this should fallback to a non-thumbnailed image, because [the thumbnailer service returns a 415 error at 1000px width and above.](https://thumbnails.zooniverse.org/1000x/panoptes-uploads.zooniverse.org/project_attached_image/1a4198cb-635a-4c0f-806e-796f3ac3877f.jpeg))

--------

The end (of the tests)

Now, compare the rendered Markdown on both PFE and FEM codebases:

Status

Priority analysis and dev work assignment pending.

We know that, as of 3 Oct, this is affecting at least one project.

@shaunanoordin shaunanoordin added the bug Something isn't working label Oct 4, 2024
@shaunanoordin shaunanoordin changed the title Markdown images (ThumbnailImage) has inconsistent sizing behaviour Markdown images (ThumbnailImage) have inconsistent sizing behaviour Oct 4, 2024
@shaunanoordin
Copy link
Member Author

Note

For the moment, we'll need to recommend that FEM project owners avoid using images in tables. (Slack) This will be the workaround while we consider a better fix.

@eatyourgreens
Copy link
Contributor

eatyourgreens commented Oct 4, 2024

The 999px limitation is set by these rules, so edit these regexes to allow bigger thumbnails.

https://github.com/zooniverse/thumbnailer/blob/a034b9123ccab5e55b316b5994460e369f55ddc2/nginx.conf#L42-L55

You’ll probably need to edit the URL rewrites in that file too, to catch URLs with bigger numbers.

EDIT: I've opened a PR to make that change.

@eatyourgreens
Copy link
Contributor

eatyourgreens commented Oct 4, 2024

The source image is 206x183 pixels in size, but the markdown specifies that the image should be presented as 800px wide.

Markdown doesn’t have a syntax for specifying image sizes, so FEM looks for the height and width in the image alt text. markdownz, on the other hand, uses a markdown convention that puts the height and width at the end of the URL. So that difference might cause problems.

Here’s the code that expects height and width to be in the alt text.

const match = alt.match(imgSizeRegex)
if (match && match.length > 0) {
width = parseInt(match[1])
if (match[3]) height = parseInt(match[3])
alt = alt.split(match[0])[0].trim()
}

EDIT: the storybook tests both PFE and FEM variations on markdown image sizing.
https://zooniverse.github.io/front-end-monorepo/@zooniverse/react-components/?path=/story/components-markdownz--default

@eatyourgreens
Copy link
Contributor

eatyourgreens commented Oct 4, 2024

This needs testing, but removing the 100% height and width from the image CSS fixes the teeny tiny Grommet Image.

The test page with the dev console open to show the image height and width styles disabled, and the image displaying at the expected size.

So removing fill here might fix the problem, by letting the browser set the image height and width.

@eatyourgreens eatyourgreens linked a pull request Oct 4, 2024 that will close this issue
12 tasks
@eatyourgreens
Copy link
Contributor

eatyourgreens commented Oct 4, 2024

#6360 looks like it will fix the tiny images in project About pages, after testing in Firefox and Chrome on Shaun's test project and WildWatch Burrowing Owl.

I marked that PR as closing this issue, but feel free to amend that, as this issue is actually several different issues (across multiple GitHub repo's.)

I’ve opened zooniverse/thumbnailer#27 to increase the maximum thumbnail size from 999px to 9999px.

@eatyourgreens
Copy link
Contributor

eatyourgreens commented Oct 4, 2024

In PFE, the problem is resolved simply by setting the width/height attribute in the img element: e.g. <img src="meerkat-206x183.jpg" width=800 />

This doesn’t really solve the problem, because you get a low resolution, pixelated image. The subject images on the new home page are an example of images that look terrible because they are rendered larger than their natural size (#6333.)

The new code makes a deliberate design decision not to enlarge undersized images. If you want an 800px image on your About page, you have to upload a suitably sized image.

Conversely, images also look terrible if you size a large image down in the browser eg. <img src='big-photo-1000x400.jpg' width=100> will look terrible unless you resize the 1000px image down to 200px or smaller. Roughly speaking, your image should be, at most, twice the rendered size.

@eatyourgreens
Copy link
Contributor

eatyourgreens commented Oct 5, 2024

FWIW, the CSS that caused sub-issue 1 (tiny images) was pretty easy to find in the browser DOM inspector. The image component was styled to fit its container, but the container td itself has no explicit height or width.

You can reproduce the same bug in PFE by adding width: 100%; or width: auto; to .project-about-page .markdown img.

https://master.pfe-preview.zooniverse.org/projects/darkeshard/test-project-2022/about/results

width: auto;
A markdown image in PFE, styled with width:auto; so that it shrinks down to a tiny size inside a table.

width: 100%;
A markdown image in PFE, styled with width:100%; so that it shrinks down to a tiny size inside a table.

@eatyourgreens
Copy link
Contributor

eatyourgreens commented Oct 5, 2024

Screenshot: we see the same animal image in two different kinds of tables. In both cases, the table layout logic seems to determine the width of left & right columns by looking for "fixed content" first - in this case, the text in the right column. It then leaves the remaining space for the "flexible content", meaning the image just fills up whatever space is left.

Sorry, one other comment. The fixed size element in the DOM here is the img element, not the text node. Text in the DOM is flexible and reflows to fill whatever space is left after fixed-size elements like images have rendered. If an image has no height or width specified, this leads to cumulative layout shift (CLS), where the text on a web page shifts position after the images have loaded. Not related to this bug, but that's why FEM project classify pages have poor CLS scores in their Core Web Vitals. The classifier reflows after the subject image loads.

@eatyourgreens
Copy link
Contributor

eatyourgreens commented Oct 5, 2024

Accessibility for people with disabilities

When projects do use tables for layout in markdown, you should probably consider the layout on small screens, and the accessibility barriers for dyslexic or blind visitors too.

As well as being an accessibility barrier for screen readers, which can enter table mode and read the table content as tabular data, layout tables don't allow for a responsive layout that adapts to screen size.

Here’s the test page in PFE on my phone.
Shaun's test page on a phone, showing that the text layout becomes difficult to read.

@eatyourgreens
Copy link
Contributor

eatyourgreens commented Oct 5, 2024

Regarding sub-issue 2, which can't be fixed by a PR, here's a link to the documentation for the nginx image filter, which resizes the thumbnail images. It can shrink and sharpen an image, but it can't make it any bigger than it already is.

https://nginx.org/en/docs/http/ngx_http_image_filter_module.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants