From 6d284fcab634ea5f31edc3c1ee547ed7997ee5ba Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Wed, 11 Dec 2024 17:00:23 -0800 Subject: [PATCH 1/6] Allow keyboard navigation of images --- .../perseus/src/components/image-loader.tsx | 1 + .../src/widgets/image/image.stories.tsx | 32 ++++++++++++++++- .../src/widgets/image/image.testdata.ts | 35 +++++++++++++++++++ packages/perseus/src/zoom.ts | 5 ++- 4 files changed, 71 insertions(+), 2 deletions(-) diff --git a/packages/perseus/src/components/image-loader.tsx b/packages/perseus/src/components/image-loader.tsx index e4c3420af5..ad3953c424 100644 --- a/packages/perseus/src/components/image-loader.tsx +++ b/packages/perseus/src/components/image-loader.tsx @@ -144,6 +144,7 @@ class ImageLoader extends React.Component { return ( { ); }; +export const ImageWithZoom = (args: StoryArgs): React.ReactElement => { + const apiOptions: APIOptions = { + isMobile: args.isMobile, + }; + const imageOptions = questionWithZoom.widgets["image 1"].options; + + const questionWithCaptionAndArgs = { + ...questionWithZoom, + widgets: { + ...questionWithZoom.widgets, + "image 1": { + ...questionWithZoom.widgets["image 1"], + options: { + ...imageOptions, + alignment: "full-width", + title: args.title, + caption: + "There is neither happiness nor unhappiness in this world; there is only the comparison of one state with another. Only a man who has felt ultimate despair is capable of feeling ultimate bliss. It is necessary to have wished for death in order to know how good it is to live.....the sum of all human wisdom will be contained in these two words: Wait and Hope", + }, + }, + }, + } as const; + return ( + + ); +}; + export default { title: "Perseus/Widgets/Image", args: { diff --git a/packages/perseus/src/widgets/image/image.testdata.ts b/packages/perseus/src/widgets/image/image.testdata.ts index 83be27ef10..ddf0ddf57b 100644 --- a/packages/perseus/src/widgets/image/image.testdata.ts +++ b/packages/perseus/src/widgets/image/image.testdata.ts @@ -34,3 +34,38 @@ export const question = { } as ImageWidget, }, } as const; + +export const questionWithZoom = { + content: + "[[☃ image 1]]\n\n=====\n\nA quilter wants to make the design shown at left using the Golden Ratio. Specifically, he wants the ratio of the triangle heights $A:B$ and $B:C$ to each equal $1.62$. If the quilter makes the triangle height $A=8\\ \\text{in}$, approximately how tall should he make triangle height $C$?", + images: { + "https://cdn.kastatic.org/ka-perseus-images/01f44d5b73290da6bec97c75a5316fb05ab61f12.jpg": + {height: 955, width: 1698}, + }, + widgets: { + "image 1": { + alignment: "block", + graded: true, + options: { + alt: "An array of isosceles triangles. A triangle has height A. Two smaller triangle, one with height B and one with height C, have approximately the same combined height as A.", + title: "Image Title", + caption: "Image Caption", + backgroundImage: { + height: 955, + url: "https://cdn.kastatic.org/ka-perseus-images/01f44d5b73290da6bec97c75a5316fb05ab61f12.jpg", + width: 1698, + }, + box: [1698, 955], + labels: [], + range: [ + [0, 10], + [0, 10], + ], + static: false, + }, + static: false, + type: "image", + version: {major: 0, minor: 0}, + } as ImageWidget, + }, +} as const; diff --git a/packages/perseus/src/zoom.ts b/packages/perseus/src/zoom.ts index daca6f9c56..18b7e4ff18 100644 --- a/packages/perseus/src/zoom.ts +++ b/packages/perseus/src/zoom.ts @@ -265,7 +265,9 @@ ZoomServiceClass.prototype._scrollHandler = function (e: any) { }; ZoomServiceClass.prototype._keyHandler = function (e: any) { - if (e.keyCode === 27) { + // 27: Esc, 13: Enter, 32: Space + const keyCodes = [27, 13, 32]; + if (keyCodes.includes(e.keyCode)) { this._activeZoomClose(); } }; @@ -367,6 +369,7 @@ Zoom.prototype.zoomImage = function () { img.src = this._targetImage.src; img.alt = this._targetImage.alt; + img.tabIndex = 0; this.$zoomedImage = $zoomedImage; }; From 5d8707c680446cc5d0243127489c33075865dc13 Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Wed, 11 Dec 2024 17:01:22 -0800 Subject: [PATCH 2/6] changeset --- .changeset/itchy-phones-look.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/itchy-phones-look.md diff --git a/.changeset/itchy-phones-look.md b/.changeset/itchy-phones-look.md new file mode 100644 index 0000000000..b3172c2df3 --- /dev/null +++ b/.changeset/itchy-phones-look.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": minor +--- + +Allow keyboards to navigate and interact with images From 346e7ed79c899559d81841c186d23dcc1f870a67 Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Thu, 12 Dec 2024 11:31:17 -0800 Subject: [PATCH 3/6] New tests! --- .../perseus/src/widgets/image/image.test.ts | 97 +++++++++++++++++-- packages/perseus/src/zoom.ts | 2 + 2 files changed, 90 insertions(+), 9 deletions(-) diff --git a/packages/perseus/src/widgets/image/image.test.ts b/packages/perseus/src/widgets/image/image.test.ts index 11645a9de1..f385b912a3 100644 --- a/packages/perseus/src/widgets/image/image.test.ts +++ b/packages/perseus/src/widgets/image/image.test.ts @@ -1,4 +1,6 @@ import {describe, beforeEach, it} from "@jest/globals"; +import {act, screen, waitFor} from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import {testDependencies} from "../../../../../testing/test-dependencies"; import * as Dependencies from "../../dependencies"; @@ -6,25 +8,30 @@ import {scorePerseusItemTesting} from "../../util/test-utils"; import {isAccessible} from "../../widgets"; import {renderQuestion} from "../__testutils__/renderQuestion"; -import {question} from "./image.testdata"; +import {question, questionWithZoom} from "./image.testdata"; import type {APIOptions} from "../../types"; describe.each([true, false])("image widget - isMobile %b", (isMobile) => { const apiOptions: APIOptions = {isMobile}; + const images: Array> = []; + let originalImage; beforeEach(() => { + jest.clearAllMocks(); + originalImage = window.Image; + // Mock HTML Image so we can trigger onLoad callbacks and see full + // image rendering. + // @ts-expect-error - TS2322 - Type 'Mock, [], any>' is not assignable to type 'new (width?: number | undefined, height?: number | undefined) => HTMLImageElement'. + window.Image = jest.fn(() => { + const img: Record = {}; + images.push(img); + return img; + }); + jest.spyOn(Dependencies, "getDependencies").mockReturnValue( testDependencies, ); - - // Mocked for loading graphie in svg-image - global.fetch = jest.fn(() => - Promise.resolve({ - text: () => "", - ok: true, - }), - ) as jest.Mock; }); it("should snapshot", () => { @@ -71,4 +78,76 @@ describe.each([true, false])("image widget - isMobile %b", (isMobile) => { // Act and Assert expect(isAccessible(inaccessibleWidgetInfo)).toBe(false); }); + + it("should zoom on click for applicable images", async () => { + // Arrange + const altText = questionWithZoom.widgets["image 1"].options.alt!; + renderQuestion(questionWithZoom, apiOptions); + + // Tells the image loader 1, or all, of our images loaded + const markImagesAsLoaded = (imageIndex?: number) => { + if (imageIndex != null) { + const img = images[imageIndex]; + if (img?.onload) { + act(() => img.onload()); + } + } else { + images.forEach((i) => { + if (i?.onload) { + act(() => i.onload()); + } + }); + } + }; + + // Act + // Act + markImagesAsLoaded(); // Tell the ImageLoader that our images are loaded + await waitFor(async () => { + await screen.getByAltText(altText).focus(); + screen.getByAltText(altText).click(); + }); + + // Assert + // The image should have the zoomed class + await waitFor(() => { + expect(screen.getByTestId("zoomed-image")).toBeVisible(); + }); + }); + + it("should zoom on keyboard interaction for applicable images", async () => { + // Arrange + const altText = questionWithZoom.widgets["image 1"].options.alt!; + renderQuestion(questionWithZoom, apiOptions); + + // Tells the image loader 1, or all, of our images loaded + const markImagesAsLoaded = (imageIndex?: number) => { + if (imageIndex != null) { + const img = images[imageIndex]; + if (img?.onload) { + act(() => img.onload()); + } + } else { + images.forEach((i) => { + if (i?.onload) { + act(() => i.onload()); + } + }); + } + }; + + // Act + // Act + markImagesAsLoaded(); // Tell the ImageLoader that our images are loaded + await waitFor(async () => { + await screen.getByAltText(altText).focus(); + await userEvent.keyboard("{enter}"); + }); + + // Assert + // The image should have the zoomed class + await waitFor(() => { + expect(screen.getByTestId("zoomed-image")).toBeVisible(); + }); + }); }); diff --git a/packages/perseus/src/zoom.ts b/packages/perseus/src/zoom.ts index 18b7e4ff18..46daf2977b 100644 --- a/packages/perseus/src/zoom.ts +++ b/packages/perseus/src/zoom.ts @@ -376,6 +376,8 @@ Zoom.prototype.zoomImage = function () { Zoom.prototype._zoomOriginal = function () { this.$zoomedImage.addClass("zoom-img").attr("data-action", "zoom-out"); + + this.$zoomedImage.attr("data-test-id", "zoomed-image"); $(this._targetImage).css("visibility", "hidden"); this._backdrop = document.createElement("div"); From 7851c102e78f13090608e18cd5113034176ceb9c Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Thu, 12 Dec 2024 11:42:53 -0800 Subject: [PATCH 4/6] No tests for you --- .../perseus/src/widgets/image/image.test.ts | 97 ++----------------- 1 file changed, 9 insertions(+), 88 deletions(-) diff --git a/packages/perseus/src/widgets/image/image.test.ts b/packages/perseus/src/widgets/image/image.test.ts index f385b912a3..11645a9de1 100644 --- a/packages/perseus/src/widgets/image/image.test.ts +++ b/packages/perseus/src/widgets/image/image.test.ts @@ -1,6 +1,4 @@ import {describe, beforeEach, it} from "@jest/globals"; -import {act, screen, waitFor} from "@testing-library/react"; -import userEvent from "@testing-library/user-event"; import {testDependencies} from "../../../../../testing/test-dependencies"; import * as Dependencies from "../../dependencies"; @@ -8,30 +6,25 @@ import {scorePerseusItemTesting} from "../../util/test-utils"; import {isAccessible} from "../../widgets"; import {renderQuestion} from "../__testutils__/renderQuestion"; -import {question, questionWithZoom} from "./image.testdata"; +import {question} from "./image.testdata"; import type {APIOptions} from "../../types"; describe.each([true, false])("image widget - isMobile %b", (isMobile) => { const apiOptions: APIOptions = {isMobile}; - const images: Array> = []; - let originalImage; beforeEach(() => { - jest.clearAllMocks(); - originalImage = window.Image; - // Mock HTML Image so we can trigger onLoad callbacks and see full - // image rendering. - // @ts-expect-error - TS2322 - Type 'Mock, [], any>' is not assignable to type 'new (width?: number | undefined, height?: number | undefined) => HTMLImageElement'. - window.Image = jest.fn(() => { - const img: Record = {}; - images.push(img); - return img; - }); - jest.spyOn(Dependencies, "getDependencies").mockReturnValue( testDependencies, ); + + // Mocked for loading graphie in svg-image + global.fetch = jest.fn(() => + Promise.resolve({ + text: () => "", + ok: true, + }), + ) as jest.Mock; }); it("should snapshot", () => { @@ -78,76 +71,4 @@ describe.each([true, false])("image widget - isMobile %b", (isMobile) => { // Act and Assert expect(isAccessible(inaccessibleWidgetInfo)).toBe(false); }); - - it("should zoom on click for applicable images", async () => { - // Arrange - const altText = questionWithZoom.widgets["image 1"].options.alt!; - renderQuestion(questionWithZoom, apiOptions); - - // Tells the image loader 1, or all, of our images loaded - const markImagesAsLoaded = (imageIndex?: number) => { - if (imageIndex != null) { - const img = images[imageIndex]; - if (img?.onload) { - act(() => img.onload()); - } - } else { - images.forEach((i) => { - if (i?.onload) { - act(() => i.onload()); - } - }); - } - }; - - // Act - // Act - markImagesAsLoaded(); // Tell the ImageLoader that our images are loaded - await waitFor(async () => { - await screen.getByAltText(altText).focus(); - screen.getByAltText(altText).click(); - }); - - // Assert - // The image should have the zoomed class - await waitFor(() => { - expect(screen.getByTestId("zoomed-image")).toBeVisible(); - }); - }); - - it("should zoom on keyboard interaction for applicable images", async () => { - // Arrange - const altText = questionWithZoom.widgets["image 1"].options.alt!; - renderQuestion(questionWithZoom, apiOptions); - - // Tells the image loader 1, or all, of our images loaded - const markImagesAsLoaded = (imageIndex?: number) => { - if (imageIndex != null) { - const img = images[imageIndex]; - if (img?.onload) { - act(() => img.onload()); - } - } else { - images.forEach((i) => { - if (i?.onload) { - act(() => i.onload()); - } - }); - } - }; - - // Act - // Act - markImagesAsLoaded(); // Tell the ImageLoader that our images are loaded - await waitFor(async () => { - await screen.getByAltText(altText).focus(); - await userEvent.keyboard("{enter}"); - }); - - // Assert - // The image should have the zoomed class - await waitFor(() => { - expect(screen.getByTestId("zoomed-image")).toBeVisible(); - }); - }); }); From da0845f5556b1ceec531c3edbecdd39c7627cc9d Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Thu, 12 Dec 2024 13:18:40 -0800 Subject: [PATCH 5/6] No longer need test id, and new cypress test --- .../src/widgets/image/image.cypress.ts | 58 +++++++++++++++++++ packages/perseus/src/zoom.ts | 1 - 2 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 packages/perseus/src/widgets/image/image.cypress.ts diff --git a/packages/perseus/src/widgets/image/image.cypress.ts b/packages/perseus/src/widgets/image/image.cypress.ts new file mode 100644 index 0000000000..f90e9f60a0 --- /dev/null +++ b/packages/perseus/src/widgets/image/image.cypress.ts @@ -0,0 +1,58 @@ +import renderQuestionWithCypress from "../../../../../testing/render-question-with-cypress"; +import {cypressTestDependencies} from "../../../../../testing/test-dependencies"; +import * as Dependencies from "../../dependencies"; +import * as Perseus from "../../index"; + +import {questionWithZoom} from "./image.testdata"; + +// NOTE: The regression tests in this file use Cypress because they are intended to validate styling that is applied. +// Since React Testing Library isn't applying the CSS to the elements, +// we can't use Jest to verify that some keyboard interactions work properly. + +describe("Image Widget", () => { + beforeEach(() => { + Dependencies.setDependencies(cypressTestDependencies); + Perseus.init(); + window.innerWidth = 1024; + }); + + afterEach(() => { + // remove classes that are added to the body + cy.get("body").invoke("removeClass", "zoom-overlay-open"); + cy.get("img.zoom-img").invoke("remove"); + }); + + it("opens and closes zoomable images on click", () => { + // Arrange + renderQuestionWithCypress(questionWithZoom); + + // Act - click on the image + cy.get(".zoomable img").click(); + + // Assert + // The zoomed image should be visible and the zoomable image should be hidden + cy.get("img.zoom-img").should("be.visible"); + cy.get(".zoomable img").should("not.be.visible"); + + cy.get(".zoom-img").click(); + + // Assert - the zoomable image should be hidden + cy.get("img.zoom-img").should("not.be.visible"); + }); + + it("opens and closes on keyboard interaction", () => { + // Arrange + renderQuestionWithCypress(questionWithZoom); + + // Act - focus on the zoomable image and press enter + cy.get(".zoomable img").focus().type("{enter}"); + + // Assert + // The zoomed image should be visible and the zoomable image should be hidden + cy.get("img.zoom-img").should("be.visible"); + cy.get(".zoomable img").should("not.be.visible"); + + // Act - focus on the zoomed image and press escape + cy.get("img.zoom-img").focus().type("{esc}"); + }); +}); diff --git a/packages/perseus/src/zoom.ts b/packages/perseus/src/zoom.ts index 46daf2977b..747c4d4c54 100644 --- a/packages/perseus/src/zoom.ts +++ b/packages/perseus/src/zoom.ts @@ -377,7 +377,6 @@ Zoom.prototype.zoomImage = function () { Zoom.prototype._zoomOriginal = function () { this.$zoomedImage.addClass("zoom-img").attr("data-action", "zoom-out"); - this.$zoomedImage.attr("data-test-id", "zoomed-image"); $(this._targetImage).css("visibility", "hidden"); this._backdrop = document.createElement("div"); From 4ecee88b419e1c6c842fcf831ba01d35178fbc04 Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Thu, 12 Dec 2024 13:40:25 -0800 Subject: [PATCH 6/6] Updating snapshots --- .../components/__tests__/__snapshots__/svg-image.test.tsx.snap | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/perseus/src/components/__tests__/__snapshots__/svg-image.test.tsx.snap b/packages/perseus/src/components/__tests__/__snapshots__/svg-image.test.tsx.snap index 448e7ebb6e..a56b7255d1 100644 --- a/packages/perseus/src/components/__tests__/__snapshots__/svg-image.test.tsx.snap +++ b/packages/perseus/src/components/__tests__/__snapshots__/svg-image.test.tsx.snap @@ -8,6 +8,7 @@ exports[`SvgImage should load and render a localized graphie svg 1`] = ` svg image @@ -21,6 +22,7 @@ exports[`SvgImage should load and render a normal graphie svg 1`] = ` svg image @@ -31,6 +33,7 @@ exports[`SvgImage should load and render a png 1`] = ` png image `;