Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/olive-ties-sneeze.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

[Image] | (UX) | Scale up SVG images to fill screen when zoomed
4 changes: 4 additions & 0 deletions packages/perseus/src/components/fixed-to-responsive.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ type Props = {
children: React.ReactNode;
className?: string;
constrainHeight?: boolean;
/**
* When the content is at least as wide as the viewport (i.e. mobile),
* allow the content to fill the entire viewport.
*/
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was confused about what this meant, so I added a comment so it wouldn't confuse me again later.

allowFullBleed?: boolean;
};

Expand Down
6 changes: 6 additions & 0 deletions packages/perseus/src/components/svg-image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,9 @@ class SvgImage extends React.Component<Props, State> {
imgSrc={imageSrc}
width={width}
height={height}
// Keep non-SVG images at their original
// size in zoom/focus mode.
allowScaleUp={false}
/>
)}
</FixedToResponsive>
Expand Down Expand Up @@ -568,6 +571,9 @@ class SvgImage extends React.Component<Props, State> {
imgSrc={imageUrl}
width={width}
height={height}
// Allow SVG images to scale up to fill
// the viewport in zoom/focus mode.
allowScaleUp={true}
/>
)}
</FixedToResponsive>
Expand Down
14 changes: 13 additions & 1 deletion packages/perseus/src/components/zoom-image-button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,20 @@ type Props = {
imgSrc: string;
width: number;
height: number;
/**
* When true, scale up to fill viewport (SVGs/Graphies).
* When false, only scale to their original size.
*/
allowScaleUp?: boolean;
};

export const ZoomImageButton = ({imgElement, imgSrc, width, height}: Props) => {
export const ZoomImageButton = ({
imgElement,
imgSrc,
width,
height,
allowScaleUp = false,
}: Props) => {
const i18n = usePerseusI18n();

// Check for "Command + Click" or "Control + Click" to open the image
Expand All @@ -37,6 +48,7 @@ export const ZoomImageButton = ({imgElement, imgSrc, width, height}: Props) => {
imgElement={imgElement}
width={width}
height={height}
allowScaleUp={allowScaleUp}
onClose={closeModal}
/>
)}
Expand Down
62 changes: 47 additions & 15 deletions packages/perseus/src/components/zoomed-image-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,19 @@ type Props = {
imgElement: React.ReactNode;
width: number;
height: number;
/**
* When true, scale up to fill viewport (SVGs/Graphies).
* When false, only scale to their original size.
*/
allowScaleUp?: boolean;
onClose: () => void;
};

export const ZoomedImageView = ({
imgElement,
width,
height,
allowScaleUp = false,
onClose,
}: Props) => {
const i18n = usePerseusI18n();
Expand All @@ -30,17 +36,15 @@ export const ZoomedImageView = ({
const maxWidth = window.innerWidth - WB_MODAL_PADDING_TOTAL;
const maxHeight = window.innerHeight - WB_MODAL_PADDING_TOTAL;

// Figure out the scale for the width and height, and use it to determine
// which dimension to use for the final size.
const scaleWidth = maxWidth / width;
const scaleHeight = maxHeight / height;
// Choose the smaller of the two so that the image fits inside
// the window - no scrolling.
const scale = Math.min(scaleWidth, scaleHeight, 1);
// When allowScaleUp is false (e.g. photos), cap at 1 so we never scale up.
const scale = allowScaleUp
? Math.min(scaleWidth, scaleHeight)
: Math.min(scaleWidth, scaleHeight, 1);

// Calculate the final dimensions, constraine by the window size.
const constrainedWidth = width * scale;
const constrainedHeight = height * scale;
const displayWidth = width * scale;
const displayHeight = height * scale;

return (
<ModalDialog
Expand Down Expand Up @@ -68,14 +72,42 @@ export const ZoomedImageView = ({
// styled correctly. Otherwise the Graphie
// labels may not be in the correct positions.
className="framework-perseus"
style={{
width: displayWidth,
height: displayHeight,
}}
>
<FixedToResponsive
className="svg-image"
width={constrainedWidth}
height={constrainedHeight}
>
{imgElement}
</FixedToResponsive>
{allowScaleUp ? (
<div
style={{
width,
height,
/**
* Use CSS transform to scale
* the whole box, so that
* Graphie + labels scale together.
*/
transform: `scale(${scale}, ${scale})`,
transformOrigin: "top left",
}}
>
<FixedToResponsive
className="svg-image"
width={width}
height={height}
>
{imgElement}
</FixedToResponsive>
</div>
) : (
<FixedToResponsive
className="svg-image"
width={displayWidth}
height={displayHeight}
>
{imgElement}
</FixedToResponsive>
)}
</div>
)}
</Clickable>
Expand Down
78 changes: 78 additions & 0 deletions packages/perseus/src/widgets/image/image.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,84 @@ describe.each([[true], [false]])("image widget - isMobile(%j)", (isMobile) => {
// Assert
expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
});

it("does not scale up normal images (e.g. photos) when zoomed", async () => {
// Arrange - normal image (jpg) uses allowScaleUp: false
const imageQuestion = generateTestPerseusRenderer({
content: "[[☃ image 1]]",
widgets: {
"image 1": generateImageWidget({
options: generateImageOptions({
// Non-SVG image
backgroundImage: earthMoonImage,
}),
}),
},
});
renderQuestion(imageQuestion, apiOptions);

// Act - open the zoom modal
const button = screen.getByRole("button", {
name: "Zoom image.",
});
await userEvent.click(button);

// Assert - no CSS transform scale wrapper (image stays at original size)
const dialog = screen.getByRole("dialog");
expect(dialog).toBeVisible();
const scaleWrappers = Array.from(
// eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
dialog.querySelectorAll("div"),
).filter((el: Element) => {
const style = el.getAttribute("style");
return (
style?.includes("transform") && style?.includes("scale(")
);
});
expect(scaleWrappers).toHaveLength(0);
});

it("scales up Graphie/SVG images when zoomed", async () => {
// Arrange - Graphie image uses allowScaleUp: true
const imageQuestion = generateTestPerseusRenderer({
content: "[[☃ image 1]]",
widgets: {
"image 1": generateImageWidget({
options: generateImageOptions({
// SVG image
backgroundImage: graphieImage,
}),
}),
},
});
renderQuestion(imageQuestion, apiOptions);
act(() => {
jest.runAllTimers();
});

// Act - open the zoom modal
const button = screen.getByRole("button", {
name: "Zoom image.",
});
await userEvent.click(button);

// Assert - zoom uses transform scale so graph + labels scale together.
// When allowScaleUp is true, ZoomedImageView wraps content in a div with
// inline transform (scale(...)).
const dialog = screen.getByRole("dialog");
expect(dialog).toBeVisible();
const scaleWrapper = Array.from(
// eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
dialog.querySelectorAll("div"),
).find((el: Element) => {
const style = el.getAttribute("style");
return (
style?.includes("transform") && style?.includes("scale(")
);
});
expect(scaleWrapper).toBeDefined();
expect(scaleWrapper).toBeInTheDocument();
});
});

describe("upgrade-image-widget feature flag", () => {
Expand Down
Loading