Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IframeContentRenderer: formalize iframe communication #1565

Open
wants to merge 13 commits into
base: feature/editor-preview-cleanup
Choose a base branch
from
Open

IframeContentRenderer: formalize iframe communication #1565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
921bc54
IframeContentRenderer: introduce message types and helper functions t…
jeremywiebe Aug 27, 2024
ea7e30c
Changeset
jeremywiebe Aug 27, 2024
bc9fddd
Add tests
jeremywiebe Aug 27, 2024
ff553e8
Make mobile emulation prop explicit instead of single-use and vague d…
jeremywiebe Aug 28, 2024
9c754f0
Remove debugging code
jeremywiebe Aug 28, 2024
d4da0f7
Formalize accessing url params for iframe further
jeremywiebe Aug 28, 2024
d21f95d
Update snapshots
jeremywiebe Aug 28, 2024
c1b5e13
Cleanup lint suppressions
jeremywiebe Aug 28, 2024
ecfae49
Accept URLSearchParams on getIframeParameter()
jeremywiebe Aug 29, 2024
ad1e516
Accept Record<string, string> on getIframeParameter()
jeremywiebe Aug 29, 2024
b703ee6
Stronly type data-changed message
jeremywiebe Aug 29, 2024
7c0b92c
Type fixes
jeremywiebe Aug 29, 2024
12ecccd
Don't throw for unhandled messages
jeremywiebe Aug 29, 2024
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/proud-baboons-warn.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus-editor": major
---

IframeContentRenderer: introduce message types and helper functions to formalize iframe communication
2 changes: 1 addition & 1 deletion packages/perseus-editor/src/__tests__/__snapshots__/iframe-content-renderer.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ exports[`IframeContentRenderer should render 1`] = `
style="width: 100%; height: 100%;"
>
<iframe
src="http://localhost/perseus/frame?frame-id=0&lint-gutter=true"
src="http://localhost/perseus/frame?emulate-mobile=false&frame-id=0&lint-gutter=true"
style="width: 100%; height: 100%;"
/>
</div>
Expand Down
166 changes: 151 additions & 15 deletions packages/perseus-editor/src/__tests__/iframe-content-renderer.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import {render} from "@testing-library/react";
/* eslint-disable testing-library/no-node-access */
import {render, waitFor} from "@testing-library/react";
import * as React from "react";

import IframeContentRenderer from "../iframe-content-renderer";
import {sendMessageToIframeParent} from "../iframe-utils";

expect.extend({
toHaveSearchParam(
Expand Down Expand Up @@ -38,14 +40,31 @@ declare global {
}
}
}

function getIframeID(iframe: HTMLIFrameElement | null): string {
const url = iframe?.src;
if (!url) {
return "";
}

const frameID = new URL(url).searchParams.get("frame-id");
expect(frameID).not.toBeNull();
return frameID!;
}

describe("IframeContentRenderer", () => {
beforeEach(() => {
jest.useRealTimers();
});

it("should render", () => {
// Arrange

// Act
const {container} = render(
<IframeContentRenderer
seamless={true}
emulateMobile={false}
url="http://localhost/perseus/frame"
/>,
);
Expand All @@ -54,6 +73,27 @@ describe("IframeContentRenderer", () => {
expect(container).toMatchSnapshot();
});

it("should set iframe.src when only path provided", () => {
// Arrange

// Act
render(
<IframeContentRenderer
seamless={true}
emulateMobile={false}
url="/perseus/frame"
/>,
);

// Assert
const iframe = document.querySelector("iframe");
expect(iframe).not.toBeNull();
expect(iframe!.src).toBe(
document.baseURI +
"perseus/frame?emulate-mobile=false&frame-id=1&lint-gutter=true",
);
});

it("should assign each iframe in page a unique frame ID", () => {
// Arrange

Expand All @@ -62,54 +102,51 @@ describe("IframeContentRenderer", () => {
<div>
<IframeContentRenderer
seamless={true}
emulateMobile={false}
url="http://localhost/perseus/frame"
/>
<IframeContentRenderer
seamless={true}
emulateMobile={false}
url="http://localhost/perseus/frame"
/>
<IframeContentRenderer
seamless={true}
emulateMobile={false}
url="http://localhost/perseus/frame"
/>
</div>,
);

// Assert
// eslint-disable-next-line testing-library/no-node-access
const iframes = document.querySelectorAll("iframe");

// We use a Set() to ensure the frame ids are unique (if we set the
// same value twice, our set will be smaller than the count of iframes
// we have).
const idSet = new Set<string | null>();
[...iframes]
.map((frame) => new URL(frame.src).searchParams.get("frame-id"))
.forEach((id) => {
expect(id).not.toBeNull();
idSet.add(id);
});
[...iframes].map(getIframeID).forEach((id) => {
idSet.add(id);
});

expect(idSet.size).toBe(3);
});

it("should set the dataset key and value if provided", () => {
it("should set the emulate-mobile key", () => {
// Arrange

// Act
render(
<IframeContentRenderer
seamless={true}
url="http://localhost/perseus/frame"
datasetKey="key-123"
datasetValue={"abc-111"}
emulateMobile={false}
/>,
);

// Assert
// eslint-disable-next-line testing-library/no-node-access
const frame = document.querySelector("iframe");
expect(frame?.src).toHaveSearchParam("key-123", "abc-111");
expect(frame?.src).toHaveSearchParam("emulate-mobile", "false");
});

it("should enable lint-gutter when seamless == true", () => {
Expand All @@ -119,12 +156,12 @@ describe("IframeContentRenderer", () => {
render(
<IframeContentRenderer
seamless={true}
emulateMobile={false}
url="http://localhost/perseus/frame"
/>,
);

// Assert
// eslint-disable-next-line testing-library/no-node-access
const frame = document.querySelector("iframe");
expect(frame!.src).toHaveSearchParam("lint-gutter", "true");
});
Expand All @@ -136,13 +173,112 @@ describe("IframeContentRenderer", () => {
render(
<IframeContentRenderer
seamless={false}
emulateMobile={false}
url="http://localhost/perseus/frame"
/>,
);

// Assert
// eslint-disable-next-line testing-library/no-node-access
const frame = document.querySelector("iframe");
expect(new URL(frame!.src).searchParams.get("lint-gutter")).toBeNull();
});

it("should send requested data", async () => {
// Arrange
const iframeRef = React.createRef<IframeContentRenderer>();
const {container} = render(
<IframeContentRenderer
ref={iframeRef}
seamless={false}
emulateMobile={true}
url="http://localhost/perseus/frame"
/>,
);

const messageHandler = jest.fn();
// eslint-disable-next-line testing-library/no-container
const iframe = container.querySelector("iframe");
expect(iframe).not.toBeNull();
expect(iframe?.contentWindow).not.toBeNull();
iframe!.contentWindow!.addEventListener("message", messageHandler);

// Act
iframeRef.current?.sendNewData({
type: "hint",
data: {
hint: {content: "Hello world", images: {}, widgets: {}},
bold: false,
pos: 0,
linterContext: {
contentType: "hint",
highlightLint: true,
paths: [],
stack: [],
},
},
});

// Assert
await waitFor(() => expect(messageHandler).toHaveBeenCalled());
});

it("should handle update-iframe-height message", async () => {
// Arrange
const iframeRef = React.createRef<IframeContentRenderer>();
const {container} = render(
<IframeContentRenderer
ref={iframeRef}
seamless={true}
emulateMobile={true}
url="http://localhost/perseus/frame"
/>,
);

const iframeID = getIframeID(document.querySelector("iframe"));

// Act
sendMessageToIframeParent({
type: "perseus:update-iframe-height",
frameID: iframeID,
height: 929,
});

// Assert
await waitFor(() =>
expect(container.firstElementChild).toHaveStyle({height: "929px"}),
);
});

it("should handle request-data message", async () => {
// Arrange
const iframeRef = React.createRef<IframeContentRenderer>();
render(
<IframeContentRenderer
ref={iframeRef}
seamless={false}
emulateMobile={true}
url="http://localhost/perseus/frame"
/>,
);

const iframeID = getIframeID(document.querySelector("iframe"));
let message: any = null;
const messageHandler = jest.fn().mockImplementation((e) => {
message = e.data;
});
window.parent!.addEventListener("message", messageHandler);

// Act
sendMessageToIframeParent({
type: "perseus:request-data",
frameID: iframeID,
});

// Assert
await waitFor(() => expect(messageHandler).toHaveBeenCalled());
expect(message).toEqual({
type: "perseus:request-data",
frameID: iframeID,
});
});
});
104 changes: 104 additions & 0 deletions packages/perseus-editor/src/__tests__/iframe-utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import {getIframeParameter, setIframeParameter} from "../iframe-utils";

describe("iframe-utils", () => {
it("should set parameter on URL", () => {
// Arrange
const url = new URL("https://www.example.com/path/to/preview");

// Act
setIframeParameter(url, "emulate-mobile", "true");

// Assert
expect(url.toString()).toBe(
"https://www.example.com/path/to/preview?emulate-mobile=true",
);
});

it("should not duplicate parameter if set multiple times", () => {
// Arrange
const url = new URL("https://www.example.com/path/to/preview");

// Act
setIframeParameter(url, "emulate-mobile", "true");
setIframeParameter(url, "emulate-mobile", "false");

// Assert
expect(url.toString()).toBe(
"https://www.example.com/path/to/preview?emulate-mobile=false",
);
});

it("should set all eligible parameters", () => {
// Arrange
const url = new URL("https://www.example.com/path/to/preview");

// Act
setIframeParameter(url, "frame-id", "100");
setIframeParameter(url, "emulate-mobile", "true");
setIframeParameter(url, "lint-gutter", "true");

// Assert
expect(url.toString()).toBe(
"https://www.example.com/path/to/preview?frame-id=100&emulate-mobile=true&lint-gutter=true",
);
});
});

describe("getIframeParameter", () => {
it("should get parameter from url string", () => {
// Arrange
const url = "https://www.example.com/path/to/preview?frame-id=100";

// Act
const value = getIframeParameter(url, "frame-id");

// Assert
expect(value).toBe("100");
});

it("should get parameter from url object", () => {
// Arrange
const url = new URL(
"https://www.example.com/path/to/preview?frame-id=100",
);

// Act
const value = getIframeParameter(url, "frame-id");

// Assert
expect(value).toBe("100");
});

it("should get parameter from search params", () => {
// Arrange
const searchParams = new URLSearchParams("?frame-id=100");

// Act
const value = getIframeParameter(searchParams, "frame-id");

// Assert
expect(value).toBe("100");
});

it("should get parameter from generic key/value pairs", () => {
// Arrange
const searchParams = {"frame-id": "100", "ignored-param": "unkonwn"};

// Act
const value = getIframeParameter(searchParams, "frame-id");

// Assert
expect(value).toBe("100");
});

it("should return null if parameter not set", () => {
// Arrange
const url = new URL("https://www.example.com/path/to/preview");

// Act
const value = getIframeParameter(url, "frame-id");

// Assert
expect(value).toBeNull();
});
});
3 changes: 1 addition & 2 deletions packages/perseus-editor/src/article-editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,7 @@ export default class ArticleEditor extends React.Component<Props, State> {
(this.iframeRenderers["frame-" + i] = frame)
}
key={this.props.screen}
datasetKey="mobile"
datasetValue={isMobile}
Comment on lines -276 to -277
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We only ever passed "mobile" here, so I've encoded that as a single prop instead of this generic key/value pair. This will make it easier to understand on the consuming side also (webapp).

emulateMobile={isMobile}
seamless={nochrome}
url={this.props.previewURL}
/>
Expand Down
Loading
Loading