Page MenuHomePhabricator

Images in reference drawer don't lazy load
Closed, ResolvedPublic0 Estimated Story Points

Assigned To
None
Authored By
Esanders
Mar 28 2024, 11:28 AM
Referenced Files
F54809002: screenshot 101.mov.gif
May 31 2024, 9:27 PM
F54809001: screenshot 102.mov.gif
May 31 2024, 9:27 PM
F43645837: image.png
Mar 28 2024, 11:28 AM
F43645714: image.png
Mar 28 2024, 11:28 AM
F43645586: image.png
Mar 28 2024, 11:28 AM

Description

For example, Math formulae. All that is shown is the grey placeholder:

image.png (454×483 px, 45 KB)

If you scroll to the references section the reference now renders (as black on black because of T129054, but if you disable the black background it is there)

image.png (454×483 px, 47 KB)
image.png (454×483 px, 47 KB)

Requirement

Ensure that images in the reference drawer lazy load correctly on mobile devices in both light and dark themes.

BDD

gherkin
Feature: Image Lazy Loading in Reference Drawer

  Scenario: Ensure images lazy load in reference drawer on mobile devices
    Given the user is viewing the page on a mobile device
    When the user expands the reference drawer
    Then the images in the reference drawer should lazy load correctly
    And the images should be visible in both light and dark themes

Test Steps

Test Case 1: Ensure Images Lazy Load in Reference Drawer

  1. Visit this page on a mobile device.
  2. Expand section T.
  3. Click [proof 1].
  4. AC1: Confirm that images in the reference drawer lazy load correctly and are visible in both light and dark themes.

(Does not need to be tested in production)

QA Results - Beta

ACStatusDetails
1T361212#9851761

Event Timeline

Change #1015291 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/MobileFrontend@master] Trigger image lazy load when drawer opens

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

My understanding is Math images might be going away (T353000). Not sure if this is the only use case though.

It is possible to include any image in a reference using [[File: syntax, so this should work regardless of what happens with Math rendering.

I know it's possible, I just seldom see it happen so I'm wondering what % of articles this bug fixes.

ovasileva moved this task from Incoming to Groomed on the Web-Team-Backlog-Archived board.
ovasileva subscribed.

I know it's possible, I just seldom see it happen so I'm wondering what % of articles this bug fixes.

Any thoughts on whether we have this data? Marking as low for now

Adding to sprint 2 for consideration as there is a patch open from Ed.

I know it's possible, I just seldom see it happen so I'm wondering what % of articles this bug fixes.

I imagine the number of articles with <math> in a <ref> is already pretty low, maybe even lower that all the other cases (such as small inline images, or other image-rendering extensions like <hiero>).

Change #1031809 had a related patch set uploaded (by Jdlrobson; author: Esanders):

[mediawiki/extensions/MobileFrontend@master] lazyImageLoader: Only set width/height if known

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

Change #1015291 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Trigger image lazy load when drawer opens

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

Change #1031809 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] lazyImageLoader: Only set width/height if known

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

Edtadros subscribed.

Test Result - Beta

Status: ❓Need More Info ✅ PASS
Environment: Beta
OS: macOS Sonoma
Browser: Chrome
Device: MBA
Emulated Device: NA

Test Artifact(s):

Test Steps

Test Case 1: Ensure Images Lazy Load in Reference Drawer

  1. Visit this page on a mobile device.
  2. Expand section T.
  3. Click [proof 1].
  4. AC1: Confirm that images in the reference drawer lazy load correctly and are visible in both light and dark themes.

@Jdlrobson, the dark looks good, but the light mode doesn't look like the light mode in the description. Is the drawer background supposed to be dark for both?. This is a pass per T361212#9851808

screenshot 102.mov.gif (1×1 px, 822 KB)

screenshot 101.mov.gif (1×1 px, 839 KB)

Sorry for the confusion. Yes this is fine. We decided now was not the time for a big change to the citation drawer.

Edtadros updated the task description. (Show Details)