Skip to content

Commit

Permalink
IframeContentRenderer: pass iframe preview params by search params on…
Browse files Browse the repository at this point in the history
… url (#1564)

## Summary:

The iframe preview used to pass parameters to the iframe content page by setting attributes on the `<iframe>` element. This is not allowed when the hosted page has a different url host than the parent page (which is what we're aiming to do by loading the preview page(s) from `kasandbox.org`). 

As a result, I've changed the preview to use search params on the URL instead. This is easily accessible in the preview page.

Issue: LEMS-1809

## Test plan:

`yarn tsc`
`yarn test`

Author: jeremywiebe

Reviewers: benchristel, jeremywiebe

Required Reviewers:

Approved By: benchristel

Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1564
  • Loading branch information
jeremywiebe authored Aug 28, 2024
1 parent 4bc36d7 commit 9e18d81
Show file tree
Hide file tree
Showing 4 changed files with 181 additions and 9 deletions.
5 changes: 5 additions & 0 deletions .changeset/lemon-grapes-invent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus-editor": major
---

IframeContentRenderer: pass iframe preview params by query string instead of element attributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`IframeContentRenderer should render 1`] = `
<div>
<div
style="width: 100%; height: 100%;"
>
<iframe
src="http://localhost/perseus/frame?frame-id=0&lint-gutter=true"
style="width: 100%; height: 100%;"
/>
</div>
</div>
`;
148 changes: 148 additions & 0 deletions packages/perseus-editor/src/__tests__/iframe-content-renderer.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
import {render} from "@testing-library/react";
import * as React from "react";

import IframeContentRenderer from "../iframe-content-renderer";

expect.extend({
toHaveSearchParam(
url: string,
name: string,
expectedValue: string,
): jest.CustomMatcherResult {
const u = new URL(url);

if (!u.searchParams.has(name)) {
return {
pass: false,
message: () =>
`Url does not have expected '${name}' search parameter (${url})`,
};
}

const actual = u.searchParams.get(name);

return {
pass: actual === expectedValue,
message: () =>
`Url does not have expected '${expectedValue}' search parameter for parameter '${name} (${url})`,
};
},
});

// TODO(FEI-5054): Figure out how to get global .d.ts files working with monorepos
declare global {
// eslint-disable-next-line @typescript-eslint/no-namespace
namespace jest {
interface Matchers<R> {
toHaveSearchParam(name: string, value: string): R;
}
}
}
describe("IframeContentRenderer", () => {
it("should render", () => {
// Arrange

// Act
const {container} = render(
<IframeContentRenderer
seamless={true}
url="http://localhost/perseus/frame"
/>,
);

// Assert
expect(container).toMatchSnapshot();
});

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

// Act
render(
<div>
<IframeContentRenderer
seamless={true}
url="http://localhost/perseus/frame"
/>
<IframeContentRenderer
seamless={true}
url="http://localhost/perseus/frame"
/>
<IframeContentRenderer
seamless={true}
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);
});

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

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

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

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

it("should enable lint-gutter when seamless == true", () => {
// Arrange

// Act
render(
<IframeContentRenderer
seamless={true}
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");
});

it("should not set lint-gutter when seamless == false", () => {
// Arrange

// Act
render(
<IframeContentRenderer
seamless={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();
});
});
23 changes: 14 additions & 9 deletions packages/perseus-editor/src/iframe-content-renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ type Props = {
// The URL that the iframe should load
url: string;
// The data-* suffix for passing information to the iframe's JS
datasetKey: string;
datasetKey?: string;
// The value of the data-* attribute
datasetValue: any;
datasetValue?: string | number | boolean;
// Whether to make the iframe's height match its content's height,
// used to prevent scrolling inside the iframe.
seamless: boolean;
Expand Down Expand Up @@ -178,15 +178,19 @@ class IframeContentRenderer extends React.Component<Props> {
const frame = document.createElement("iframe");
frame.style.width = "100%";
frame.style.height = "100%";
frame.src = this.props.url;
const frameSrc = new URL(this.props.url);

if (this.props.datasetKey) {
// If the user has specified a data-* attribute to place on the
// iframe, we set it here. Right now, this is used to
// communicate if the iframe should be enabling touch emulation.
frame.dataset[this.props.datasetKey] = this.props.datasetValue;
// If the user has provided and extra data attribute to pass to the
// iframe page, we add it to the url here. Right now, this is used
// to communicate if the iframe should be enabling touch emulation.
frameSrc.searchParams.append(
this.props.datasetKey,
(this.props.datasetValue ?? "").toString(),
);
}
frame.dataset.id = String(this.iframeID);

frameSrc.searchParams.append("frame-id", String(this.iframeID));

if (this.props.seamless) {
// The seamless prop is the same as the "nochrome" prop that
Expand All @@ -195,9 +199,10 @@ class IframeContentRenderer extends React.Component<Props> {
// for lint indicators in the right margin. We use the dataset
// as above to pass this information on to the perseus-frame
// component inside the iframe
frame.dataset.lintGutter = "true";
frameSrc.searchParams.append("lint-gutter", "true");
}

frame.src = frameSrc.toString();
this.container.current?.appendChild(frame);

this._frame = frame;
Expand Down

0 comments on commit 9e18d81

Please sign in to comment.