diff --git a/docs/api/media/image_compare.md b/docs/api/media/image_compare.md index 34f9d8352e7..690b95f224b 100644 --- a/docs/api/media/image_compare.md +++ b/docs/api/media/image_compare.md @@ -6,14 +6,23 @@ ```python @app.cell def __(): - before_image = "https://picsum.photos/200/301.jpg" - after_image = "https://picsum.photos/200/300.jpg" - + from PIL import Image, ImageDraw + + # A colorful "before" image, compared against its grayscale "after". + before_image = Image.new("RGB", (600, 400), "white") + _draw = ImageDraw.Draw(before_image) + for _x in range(0, 600, 40): + _draw.rectangle( + [_x, 0, _x + 20, 400], fill=(_x % 256, (_x * 2) % 256, 128) + ) + _draw.ellipse([200, 100, 400, 300], fill=(255, 140, 0)) + after_image = before_image.convert("L").convert("RGB") + mo.image_compare( before_image=before_image, after_image=after_image, value=30, - direction="horizontal" + direction="horizontal", ) return ``` diff --git a/frontend/src/plugins/impl/image-comparison/ImageComparisonComponent.tsx b/frontend/src/plugins/impl/image-comparison/ImageComparisonComponent.tsx index 9cd5bda72f1..547bc7c008d 100644 --- a/frontend/src/plugins/impl/image-comparison/ImageComparisonComponent.tsx +++ b/frontend/src/plugins/impl/image-comparison/ImageComparisonComponent.tsx @@ -12,6 +12,13 @@ export interface ImageComparisonData { height?: string; } +// Truncate long sources (e.g. base64 data URLs) so error messages stay +// readable. +function truncateSrc(src: string): string { + const MAX_LENGTH = 100; + return src.length > MAX_LENGTH ? `${src.slice(0, MAX_LENGTH)}…` : src; +} + const ImageComparisonComponent: React.FC = ({ beforeSrc, afterSrc, @@ -20,17 +27,61 @@ const ImageComparisonComponent: React.FC = ({ width, height, }) => { + const [failedSrcs, setFailedSrcs] = React.useState>( + () => new Set(), + ); + + React.useEffect(() => { + setFailedSrcs(new Set()); + }, [beforeSrc, afterSrc]); + + const handleError = React.useCallback((src: string) => { + setFailedSrcs((prev) => new Set(prev).add(src)); + }, []); + const containerStyle: React.CSSProperties = { width: width || "100%", height: height || (direction === "vertical" ? "400px" : "auto"), maxWidth: "100%", + // The slider derives its height entirely from its (loaded) images, so a + // broken/slow-loading source would otherwise collapse it to nothing and + // render an empty output. Keep a minimum height so it stays visible. + minHeight: "2rem", }; + // If an image fails to load, the slider collapses to nothing; surface a + // visible error instead of silently rendering an empty output. + if (failedSrcs.size > 0) { + return ( +
+ + Failed to load {failedSrcs.size > 1 ? "images" : "image"}:{" "} + {[...failedSrcs].map((src) => `"${truncateSrc(src)}"`).join(", ")} + +
+ ); + } + return (
- Before - After + Before handleError(beforeSrc)} + /> + After handleError(afterSrc)} + />
); diff --git a/frontend/src/plugins/impl/image-comparison/__tests__/ImageComparisonComponent.test.tsx b/frontend/src/plugins/impl/image-comparison/__tests__/ImageComparisonComponent.test.tsx new file mode 100644 index 00000000000..198fde8e73a --- /dev/null +++ b/frontend/src/plugins/impl/image-comparison/__tests__/ImageComparisonComponent.test.tsx @@ -0,0 +1,71 @@ +/* Copyright 2026 Marimo. All rights reserved. */ + +import { fireEvent, render } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; +import ImageComparisonComponent from "../ImageComparisonComponent"; + +const baseProps = { + beforeSrc: "before.png", + afterSrc: "after.png", + value: 50, + direction: "horizontal" as const, +}; + +describe("ImageComparisonComponent", () => { + it("renders the comparison slider with both images", () => { + const { getByAltText, container } = render( + , + ); + expect(getByAltText("Before")).toBeTruthy(); + expect(getByAltText("After")).toBeTruthy(); + expect(container.querySelector("img-comparison-slider")).toBeTruthy(); + }); + + it("shows a visible error instead of collapsing when an image fails to load", () => { + const brokenSrc = "https://example.com/does-not-exist.png"; + const { getByAltText, queryByText, container } = render( + , + ); + + // No error before the image fails. + expect(queryByText(/Failed to load/)).toBeNull(); + + fireEvent.error(getByAltText("Before")); + + // The slider is replaced with a visible error mentioning the source, so + // the output is never a silent blank. + expect(container.querySelector("img-comparison-slider")).toBeNull(); + const error = queryByText(/Failed to load image/); + expect(error).toBeTruthy(); + expect(error?.textContent).toContain(brokenSrc); + }); + + it("clears the error when the sources change so new images can load", () => { + const brokenSrc = "https://example.com/does-not-exist.png"; + const { getByAltText, queryByText, container, rerender } = render( + , + ); + + fireEvent.error(getByAltText("Before")); + expect(queryByText(/Failed to load/)).toBeTruthy(); + + // Re-render with a new source (e.g. the user fixed the notebook): the error + // should clear and the slider should mount again rather than stay stuck. + rerender(); + expect(queryByText(/Failed to load/)).toBeNull(); + expect(container.querySelector("img-comparison-slider")).toBeTruthy(); + }); + + it("truncates long sources (e.g. data URLs) in the error message", () => { + const longSrc = `data:image/png;base64,${"A".repeat(200)}`; + const { getByAltText, queryByText } = render( + , + ); + + fireEvent.error(getByAltText("After")); + + const error = queryByText(/Failed to load image/); + expect(error?.textContent).toContain("…"); + expect(error?.textContent).not.toContain("A".repeat(200)); + }); +}); diff --git a/marimo/_plugins/stateless/image_compare.py b/marimo/_plugins/stateless/image_compare.py index ecceea3cad6..d6b97719528 100644 --- a/marimo/_plugins/stateless/image_compare.py +++ b/marimo/_plugins/stateless/image_compare.py @@ -5,12 +5,12 @@ import os from pathlib import Path from typing import Literal +from urllib.parse import urlparse import marimo._output.data.data as mo_data from marimo._output.hypertext import Html from marimo._output.rich_help import mddoc from marimo._output.utils import normalize_dimension -from marimo._plugins.core.media import io_to_data_url from marimo._plugins.core.web_component import build_stateless_plugin from marimo._plugins.stateless.image import ImageLike, _normalize_image @@ -98,34 +98,44 @@ def _process_image_to_url(src: ImageLike) -> str: Returns: A string URL that can be used in an tag. + + Raises: + ValueError: If the image cannot be processed, e.g. an unsupported + type or a string that is neither an existing file nor a URL. + Failing loudly is preferable to emitting a broken `` that + silently renders nothing. """ - try: - src = _normalize_image(src) - - # different types handling - if isinstance(src, (io.BufferedReader, io.BytesIO)): - src.seek(0) - return mo_data.image(src.read()).url - elif isinstance(src, bytes): - return mo_data.image(src).url - elif isinstance(src, Path): - return mo_data.image(src.read_bytes(), ext=src.suffix).url - elif isinstance(src, str) and os.path.isfile( - expanded_path := os.path.expanduser(src) - ): + src = _normalize_image(src) + + # different types handling + if isinstance(src, (io.BufferedReader, io.BytesIO)): + src.seek(0) + return mo_data.image(src.read()).url + elif isinstance(src, bytes): + return mo_data.image(src).url + elif isinstance(src, Path): + return mo_data.image(src.read_bytes(), ext=src.suffix).url + elif isinstance(src, str): + # An existing file on disk: embed its bytes. + expanded_path = os.path.expanduser(src) + if os.path.isfile(expanded_path): path = Path(expanded_path) return mo_data.image(path.read_bytes(), ext=path.suffix).url - else: - # If it's a URL or other string, try to use it directly - result = io_to_data_url(src, fallback_mime_type="image/png") - return ( - result - if result is not None - else f"data:text/plain,Unable to process image: {src}" - ) - except Exception as e: - # return an error message otherwise - error_message = f"Error processing image: {e!s}" - # Using a comment instead of print for logging - # print(f"Warning: {error_message}") - return f"data:text/plain,{error_message}" + + # A data URL or a remote URL: use it directly. + if src.startswith("data:"): + return src + parsed = urlparse(src) + if parsed.scheme and parsed.netloc: + return src + + # Neither a reachable file nor a URL: this would render an empty + # slider, so surface the problem instead. + raise ValueError( + f"Could not load image from {src!r}: it is not an existing file " + "path or a valid URL." + ) + + # `_normalize_image` only ever returns one of the types handled above (or + # raises), so this is a defensive guard rather than an expected path. + raise ValueError(f"Unsupported image type: {type(src)}") diff --git a/tests/_plugins/stateless/test_image_compare.py b/tests/_plugins/stateless/test_image_compare.py index 6f045af1d32..c01171c361b 100644 --- a/tests/_plugins/stateless/test_image_compare.py +++ b/tests/_plugins/stateless/test_image_compare.py @@ -290,12 +290,26 @@ async def test_image_compare_local_file( assert len(get_context().virtual_file_registry.registry) == 2 -async def test_image_compare_error_handling() -> None: - # This should not raise an exception, but handle the error gracefully - result = image_compare( - before_image="invalid_path_that_does_not_exist.png", - after_image="another_invalid_path.png", - ) +async def test_image_compare_invalid_source_raises() -> None: + # A string that is neither an existing file nor a URL previously produced a + # broken that silently rendered nothing; it should now fail loudly so + # the user gets feedback instead of an empty output. + with pytest.raises(ValueError, match="not an existing file path or a"): + image_compare( + before_image="invalid_path_that_does_not_exist.png", + after_image="https://marimo.io/logo.png", + ) + + with pytest.raises(ValueError, match="not an existing file path or a"): + image_compare( + before_image="https://marimo.io/logo.png", + after_image="another_invalid_path.png", + ) + - # Should still generate HTML even with invalid images +async def test_image_compare_data_url() -> None: + # Data URLs are already renderable and should be passed through as-is. + data_url = "data:image/png;base64,iVBORw0KGgo=" + result = image_compare(before_image=data_url, after_image=data_url) assert "marimo-image-comparison" in result.text + assert "data:image/png;base64" in result.text