Add more image modal tests

isic/core/health.py

@@ -185,6 +185,7 @@ def check_iptc_metadata_consistency() -> HealthCheckResult:
 
     for image in jpg_images:
         with field_file_to_local_path(image.blob) as path, pyexiv2.Image(str(path)) as img:
+            # TODO: attributes
             iptc_credit = img.read_iptc().get("Iptc.Application2.Credit", "")
             if iptc_credit != image.accession.attribution:
                 errors.append(f"{image.isic_id}: attribution mismatch")

isic/core/templates/core/partials/image_modal.html

@@ -8,7 +8,7 @@
   {% comment %}TODO: escape key should exist{% endcomment %}
 
   <div class="flex items-center justify-center min-h-screen p-4 sm:p-8">
-    <div @click.away="open = false" class="bg-white relative p-4 sm:p-8 rounded-lg text-left shadow-xl max-w-5xl w-full" role="dialog" aria-modal="true" aria-labelledby="modal-headline">
+    <div @click.away="open = false" class="bg-white relative p-4 sm:p-8 rounded-lg text-left shadow-xl max-w-5xl w-full max-h-[calc(100vh-2rem)] overflow-y-auto" role="dialog" aria-modal="true" aria-labelledby="modal-headline">
       <template x-if="hovered">
         {% if image.accession.is_cog %}
           <div>

isic/core/tests/test_image_modal_browser.py

@@ -1,11 +1,13 @@
 import io
 
+from django.test import Client
 from django.urls import reverse
 from PIL import Image as PILImage
 from playwright.sync_api import expect
 import pytest
 
 from isic.core.services.collection.image import collection_add_images
+from isic.studies.models import Question, QuestionChoice, StudyTask
 
 
 def _make_jpeg_bytes(width, height, color="red"):
@@ -15,6 +17,32 @@ def _make_jpeg_bytes(width, height, color="red"):
     return buf.getvalue()
 
 
+def _intercept_image_urls(page, urls):
+    for url, jpeg_bytes in urls:
+
+        def _fulfill(body):
+            def _handler(route, *_args):
+                route.fulfill(content_type="image/jpeg", body=body)
+
+            return _handler
+
+        page.route(url, _fulfill(jpeg_bytes))
+
+
+def _assert_modal_fits_viewport(modal, viewport):
+    box = modal.bounding_box()
+    assert box is not None, "Modal dialog has no bounding box"
+    assert box["x"] >= 0, f"Modal left edge ({box['x']}) is off-screen"
+    assert box["y"] >= 0, f"Modal top edge ({box['y']}) is off-screen"
+    assert box["x"] + box["width"] <= viewport["width"], (
+        f"Modal right edge ({box['x'] + box['width']}) exceeds viewport width ({viewport['width']})"
+    )
+    assert box["y"] + box["height"] <= viewport["height"], (
+        f"Modal bottom edge ({box['y'] + box['height']}) "
+        f"exceeds viewport height ({viewport['height']})"
+    )
+
+
 IMAGE_SIZES = [
     pytest.param((100, 100), id="small-square"),
     pytest.param((300, 1200), id="tall-portrait"),
@@ -33,7 +61,7 @@ def _make_jpeg_bytes(width, height, color="red"):
 @pytest.mark.playwright
 @pytest.mark.parametrize("image_size", IMAGE_SIZES)
 @pytest.mark.parametrize("viewport", VIEWPORT_SIZES)
-def test_image_modal_fits_viewport(
+def test_collection_detail_image_modal_fits_viewport(
     new_context,
     live_server,
     collection_factory,
@@ -51,50 +79,168 @@ def test_image_modal_fits_viewport(
     )
     collection_add_images(collection=collection, image=image)
 
-    # MinIO URLs aren't browser-accessible in the test environment,
-    # so intercept image requests and serve valid JPEGs directly.
     blob_bytes = _make_jpeg_bytes(w, h)
     thumb_bytes = _make_jpeg_bytes(256, 256)
 
     ctx = new_context(base_url=live_server.url, viewport=viewport)
     ctx.set_default_timeout(15_000)
     page = ctx.new_page()
 
-    page.route(
-        image.blob.url,
-        lambda route: route.fulfill(content_type="image/jpeg", body=blob_bytes),
-    )
-    page.route(
-        image.thumbnail_256.url,
-        lambda route: route.fulfill(content_type="image/jpeg", body=thumb_bytes),
+    _intercept_image_urls(
+        page,
+        [
+            (image.blob.url, blob_bytes),
+            (image.thumbnail_256.url, thumb_bytes),
+        ],
     )
 
     page.goto(reverse("core/collection-detail", args=[collection.pk]))
 
-    # Hover the thumbnail first (triggers hovered=true which loads the full image URL),
-    # then click to open the modal.
     thumb_el = page.locator("img").first
     thumb_el.hover()
     thumb_el.click()
     modal = page.get_by_role("dialog")
     expect(modal).to_be_visible()
 
-    # Wait for the full-size image to actually render
     modal_img = modal.locator("img")
-    modal_img.wait_for(state="visible")
-    page.wait_for_function(
-        "el => el.naturalWidth > 0 && el.complete",
-        arg=modal_img.element_handle(),
+    expect(modal_img).to_be_visible()
+    expect(modal_img).to_have_js_property("complete", value=True)
+
+    _assert_modal_fits_viewport(modal, viewport)
+
+
+@pytest.mark.playwright
+@pytest.mark.parametrize("image_size", IMAGE_SIZES)
+@pytest.mark.parametrize("viewport", VIEWPORT_SIZES)
+def test_accession_modal_fits_viewport(
+    new_context,
+    live_server,
+    staff_authenticated_user,
+    cohort_factory,
+    accession_factory,
+    image_size,
+    viewport,
+):
+    w, h = image_size
+
+    cohort = cohort_factory()
+    accession = accession_factory(cohort=cohort, ingested=True, width=w, height=h)
+
+    blob_bytes = _make_jpeg_bytes(w, h)
+    thumb_bytes = _make_jpeg_bytes(256, 256)
+
+    ctx = new_context(base_url=live_server.url, viewport=viewport)
+    ctx.set_default_timeout(15_000)
+    page = ctx.new_page()
+
+    client = Client()
+    client.force_login(staff_authenticated_user)
+    session_cookie = client.cookies["sessionid"]
+    ctx.add_cookies(
+        [
+            {
+                "name": "sessionid",
+                "value": session_cookie.value,
+                "url": live_server.url,
+            }
+        ]
     )
 
-    box = modal.bounding_box()
-    assert box is not None, "Modal dialog has no bounding box"
-    assert box["x"] >= 0, f"Modal left edge ({box['x']}) is off-screen"
-    assert box["y"] >= 0, f"Modal top edge ({box['y']}) is off-screen"
-    assert box["x"] + box["width"] <= viewport["width"], (
-        f"Modal right edge ({box['x'] + box['width']}) exceeds viewport width ({viewport['width']})"
+    _intercept_image_urls(
+        page,
+        [
+            (accession.blob_.url, blob_bytes),
+            (accession.thumbnail_.url, thumb_bytes),
+        ],
     )
-    assert box["y"] + box["height"] <= viewport["height"], (
-        f"Modal bottom edge ({box['y'] + box['height']}) "
-        f"exceeds viewport height ({viewport['height']})"
+
+    page.goto(reverse("cohort-review", args=[cohort.pk]))
+
+    thumb_el = page.locator("img").first
+    thumb_el.hover()
+    thumb_el.click()
+    modal = page.get_by_role("dialog")
+    expect(modal).to_be_visible()
+
+    modal_img = modal.locator("img")
+    expect(modal_img).to_be_visible()
+    expect(modal_img).to_have_js_property("complete", value=True)
+
+    _assert_modal_fits_viewport(modal, viewport)
+
+
+@pytest.mark.playwright
+@pytest.mark.parametrize("image_size", IMAGE_SIZES)
+@pytest.mark.parametrize("viewport", VIEWPORT_SIZES)
+def test_study_task_image_modal_fits_viewport(
+    new_context,
+    live_server,
+    authenticated_user,
+    collection_factory,
+    image_factory,
+    study_factory,
+    image_size,
+    viewport,
+):
+    w, h = image_size
+
+    collection = collection_factory(creator=authenticated_user)
+    image = image_factory(public=True, accession__width=w, accession__height=h)
+    collection_add_images(collection=collection, image=image)
+
+    question = Question.objects.create(
+        prompt="Diagnosis?", type=Question.QuestionType.SELECT, official=False
+    )
+    QuestionChoice.objects.create(question=question, text="Choice A")
+
+    study = study_factory(
+        creator=authenticated_user,
+        collection=collection,
+        public=False,
+        questions=[question],
+    )
+
+    task = StudyTask.objects.create(study=study, annotator=authenticated_user, image=image)
+
+    blob_bytes = _make_jpeg_bytes(w, h)
+    thumb_bytes = _make_jpeg_bytes(256, 256)
+
+    ctx = new_context(base_url=live_server.url, viewport=viewport)
+    ctx.set_default_timeout(15_000)
+    page = ctx.new_page()
+
+    client = Client()
+    client.force_login(authenticated_user)
+    session_cookie = client.cookies["sessionid"]
+    ctx.add_cookies(
+        [
+            {
+                "name": "sessionid",
+                "value": session_cookie.value,
+                "url": live_server.url,
+            }
+        ]
     )
+
+    _intercept_image_urls(
+        page,
+        [
+            (image.blob.url, blob_bytes),
+            (image.thumbnail_256.url, thumb_bytes),
+        ],
+    )
+
+    page.goto(reverse("study-task-detail", args=[task.pk]))
+
+    # The study task page shows the full image directly (not a thumbnail),
+    # click it to open the modal.
+    page.locator("img").first.hover()
+    page.locator("img").first.click()
+    modal = page.get_by_role("dialog")
+    expect(modal).to_be_visible()
+
+    modal_img = modal.locator("img")
+    expect(modal_img).to_be_visible()
+    expect(modal_img).to_have_js_property("complete", value=True)
+
+    _assert_modal_fits_viewport(modal, viewport)

isic/ingest/templates/ingest/partials/accession_modal.html

@@ -8,10 +8,10 @@
 
     <!-- This element is to trick the browser into centering the modal contents. -->
     <span class="hidden sm:inline-block sm:align-middle sm:h-screen" aria-hidden="true">&#8203;</span>
-    <div @click.away="open = false" class="inline-block align-top bg-white rounded-lg p-8 text-left overflow-hidden shadow-xl transform transition-all my-8 align-middle max-w-2xl p-6" role="dialog" aria-modal="true" aria-labelledby="modal-headline">
+    <div @click.away="open = false" class="inline-block align-top bg-white rounded-lg text-left overflow-y-auto shadow-xl transform transition-all my-8 align-middle max-w-2xl max-h-[calc(100vh-4rem)] p-6" role="dialog" aria-modal="true" aria-labelledby="modal-headline">
       <template x-if="hovered">
           {% comment %}Note the single quotes: https://github.com/alpinejs/alpine/issues/466{% endcomment %}
-        <img :src="'{{ accession.blob_.url }}'" />
+        <img :src="'{{ accession.blob_.url }}'" class="max-w-full max-h-[70vh] object-contain mx-auto" />
       </template>
 
       <div x-data="{selectedTab: 'Metadata'}">