Skip to content

Commit d069e7c

Browse files
authored
1261: fix for turtle target reference error (#1442)
1 parent b152adf commit d069e7c

4 files changed

Lines changed: 153 additions & 57 deletions

File tree

src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx

Lines changed: 50 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ const SkulptRunner = ({ active, outputPanels = ["text", "visual"] }) => {
7575
const projectIdentifier = useSelector(
7676
(state) => state.editor.project.identifier,
7777
);
78+
const projectImages = useSelector((state) => state.editor.project.image_list);
7879
const user = useSelector((state) => state.auth.user);
7980
const isSplitView = useSelector((state) => state.editor.isSplitView);
8081
const isEmbedded = useSelector((state) => state.editor.isEmbedded);
@@ -89,6 +90,7 @@ const SkulptRunner = ({ active, outputPanels = ["text", "visual"] }) => {
8990
);
9091
const reactAppApiEndpoint = useSelector((s) => s.editor.reactAppApiEndpoint);
9192
const output = useRef();
93+
const visualOutputPaneRef = useRef(null);
9294
const dispatch = useDispatch();
9395
const { t } = useTranslation();
9496
const settings = useContext(SettingsContext);
@@ -128,6 +130,39 @@ const SkulptRunner = ({ active, outputPanels = ["text", "visual"] }) => {
128130
return pageInput || webComponentInput;
129131
};
130132

133+
const getTurtleOutputTarget = () => {
134+
return visualOutputPaneRef.current?.getTurtleTarget?.() || null;
135+
};
136+
137+
const bindTurtleGraphics = () => {
138+
const target = getTurtleOutputTarget();
139+
if (!target) {
140+
return;
141+
}
142+
143+
configureTurtleGraphics({
144+
targetEl: target,
145+
projectImages,
146+
});
147+
};
148+
149+
const installTurtleDomTargetFallback = () => {
150+
const originalGetElementById = document.getElementById;
151+
152+
document.getElementById = (id) => {
153+
if (id === "turtle") {
154+
const turtleTarget = getTurtleOutputTarget();
155+
if (turtleTarget) {
156+
return turtleTarget;
157+
}
158+
}
159+
return originalGetElementById.call(document, id);
160+
};
161+
return () => {
162+
document.getElementById = originalGetElementById;
163+
};
164+
};
165+
131166
useEffect(() => {
132167
if (!codeRunTriggered) {
133168
setCodeHasVisualOutput(
@@ -410,18 +445,6 @@ const SkulptRunner = ({ active, outputPanels = ["text", "visual"] }) => {
410445
}
411446
dispatch(setSenseHatEnabled(false));
412447

413-
// runCode runs from the current runner's useEffect before VisualOutputPane’s useEffect
414-
// so Sk.TurtleGraphics.target / assets are set here so they exist before import
415-
const host = document.querySelector("editor-wc");
416-
const turtleOutputElement =
417-
host?.shadowRoot?.getElementById("turtleOutput") ||
418-
document.getElementById("turtleOutput");
419-
420-
configureTurtleGraphics({
421-
targetEl: turtleOutputElement,
422-
projectImages: project.image_list || [],
423-
});
424-
425448
var prog = mainComponent?.content || "";
426449

427450
if (prog.includes(`# ${t("input.comment.py5")}`)) {
@@ -435,6 +458,14 @@ const SkulptRunner = ({ active, outputPanels = ["text", "visual"] }) => {
435458
}
436459
}
437460

461+
if (Sk.TurtleGraphics?.reset) {
462+
Sk.TurtleGraphics.reset();
463+
} else if (Sk.TurtleGraphics?.stop) {
464+
Sk.TurtleGraphics.stop();
465+
}
466+
467+
Sk.TurtleGraphics = {};
468+
438469
Sk.configure({
439470
inputfun: inf,
440471
output: outf,
@@ -444,6 +475,10 @@ const SkulptRunner = ({ active, outputPanels = ["text", "visual"] }) => {
444475
uncaughtException: handleError,
445476
});
446477

478+
bindTurtleGraphics();
479+
480+
const uninstallTurtleDomTargetFallback = installTurtleDomTargetFallback();
481+
447482
var myPromise = Sk.misceval
448483
.asyncToPromise(() => Sk.importMainWithBody("main", false, prog, true), {
449484
"*": () => {
@@ -456,6 +491,7 @@ const SkulptRunner = ({ active, outputPanels = ["text", "visual"] }) => {
456491
handleError(err);
457492
})
458493
.finally(() => {
494+
uninstallTurtleDomTargetFallback();
459495
dispatch(codeRunHandled());
460496
});
461497
myPromise.then(function (_mod) {});
@@ -535,7 +571,7 @@ const SkulptRunner = ({ active, outputPanels = ["text", "visual"] }) => {
535571
{!isEmbedded && isMobile && <RunnerControls skinny />}
536572
</div>
537573
<TabPanel key={0}>
538-
<VisualOutputPane />
574+
<VisualOutputPane ref={visualOutputPaneRef} />
539575
</TabPanel>
540576
</Tabs>
541577
</div>
@@ -598,7 +634,7 @@ const SkulptRunner = ({ active, outputPanels = ["text", "visual"] }) => {
598634
</div>
599635
{!isOutputOnly && <ErrorMessage />}
600636
<TabPanel key={0}>
601-
<VisualOutputPane />
637+
<VisualOutputPane ref={visualOutputPaneRef} />
602638
</TabPanel>
603639
<TabPanel key={1}>
604640
<pre

src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js

Lines changed: 78 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,58 +1330,108 @@ describe("When not active and a code run has been triggered", () => {
13301330
});
13311331
});
13321332

1333-
describe("Turtle graphics is wired before Skulpt import", () => {
1334-
const asyncToPromise = Sk.misceval.asyncToPromise;
1333+
describe("When turtle run setup is applied", () => {
1334+
let originalAsyncToPromise;
1335+
let originalImportMainWithBody;
1336+
let originalConfigure;
1337+
let originalGetElementById;
1338+
1339+
beforeEach(() => {
1340+
originalAsyncToPromise = Sk.misceval.asyncToPromise;
1341+
originalImportMainWithBody = Sk.importMainWithBody;
1342+
originalConfigure = Sk.configure;
1343+
originalGetElementById = document.getElementById;
1344+
1345+
Sk.configure = jest.fn();
1346+
});
13351347

13361348
afterEach(() => {
1337-
Sk.misceval.asyncToPromise = asyncToPromise;
1338-
});
1339-
1340-
test("sets Sk.TurtleGraphics.target and assets before asyncToPromise runs", () => {
1341-
Sk.misceval.asyncToPromise = jest.fn((_computeFn) => {
1342-
const turtleElement = document.getElementById("turtleOutput");
1343-
expect(turtleElement).not.toBeNull();
1344-
expect(Sk.TurtleGraphics.target).toBe(turtleElement);
1345-
expect(Sk.TurtleGraphics.assets["pic.png"]).toBe(
1346-
"https://example.com/img.png",
1347-
);
1348-
return Promise.resolve();
1349-
});
1349+
Sk.misceval.asyncToPromise = originalAsyncToPromise;
1350+
Sk.importMainWithBody = originalImportMainWithBody;
1351+
Sk.configure = originalConfigure;
1352+
document.getElementById = originalGetElementById;
1353+
});
1354+
1355+
test("binds turtle target from VisualOutputPane ref before execution", () => {
1356+
Sk.importMainWithBody = jest.fn();
1357+
Sk.misceval.asyncToPromise = jest.fn(() => Promise.resolve());
13501358

13511359
const middlewares = [];
13521360
const mockStore = configureStore(middlewares);
1353-
const initialState = {
1361+
const store = mockStore({
13541362
editor: {
13551363
project: {
13561364
components: [
13571365
{
13581366
name: "main",
13591367
extension: "py",
1360-
content: "print('hello')",
1368+
content: "from turtle import *",
13611369
},
13621370
],
1363-
image_list: [
1371+
image_list: [],
1372+
},
1373+
codeRunTriggered: true,
1374+
},
1375+
auth: { user },
1376+
});
1377+
1378+
render(
1379+
<Provider store={store}>
1380+
<SkulptRunner active={true} />
1381+
</Provider>,
1382+
);
1383+
1384+
expect(Sk.TurtleGraphics.target).not.toBeNull();
1385+
expect(Sk.TurtleGraphics.target.id).toBe("turtleOutput");
1386+
});
1387+
1388+
test("temporarily maps getElementById('turtle') during execution", async () => {
1389+
let turtleLookupTarget;
1390+
let resolveRun;
1391+
const runPromise = new Promise((resolve) => {
1392+
resolveRun = resolve;
1393+
});
1394+
1395+
Sk.importMainWithBody = jest.fn(() => {
1396+
turtleLookupTarget = document.getElementById("turtle");
1397+
return {};
1398+
});
1399+
Sk.misceval.asyncToPromise = jest.fn((runner) => {
1400+
runner();
1401+
return runPromise;
1402+
});
1403+
1404+
const middlewares = [];
1405+
const mockStore = configureStore(middlewares);
1406+
const store = mockStore({
1407+
editor: {
1408+
project: {
1409+
components: [
13641410
{
1365-
name: "pic",
1366-
extension: "png",
1367-
url: "https://example.com/img.png",
1411+
name: "main",
1412+
extension: "py",
1413+
content: "from turtle import *",
13681414
},
13691415
],
1416+
image_list: [],
13701417
},
13711418
codeRunTriggered: true,
1372-
isEmbedded: false,
1373-
},
1374-
auth: {
1375-
user,
13761419
},
1377-
};
1378-
const store = mockStore(initialState);
1420+
auth: { user },
1421+
});
1422+
13791423
render(
13801424
<Provider store={store}>
13811425
<SkulptRunner active={true} />
13821426
</Provider>,
13831427
);
13841428

1385-
expect(Sk.misceval.asyncToPromise).toHaveBeenCalled();
1429+
expect(turtleLookupTarget).not.toBeNull();
1430+
expect(turtleLookupTarget.id).toBe("turtleOutput");
1431+
1432+
resolveRun();
1433+
await runPromise;
1434+
await Promise.resolve();
1435+
expect(document.getElementById("turtle")).toBeNull();
13861436
});
13871437
});

src/components/Editor/Runners/PythonRunner/VisualOutputPane.jsx

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
1-
import React, { useEffect } from "react";
1+
import React, { forwardRef, useEffect, useImperativeHandle } from "react";
22
import { useRef } from "react";
33
import { useTranslation } from "react-i18next";
44
import { useDispatch, useSelector } from "react-redux";
55
import Sk from "skulpt";
66
import AstroPiModel from "../../../AstroPiModel/AstroPiModel";
77
import { codeRunHandled, setError } from "../../../../redux/EditorSlice";
8-
import { configureTurtleGraphics } from "../../../../utils/configureTurtleGraphics";
98

10-
const VisualOutputPane = () => {
9+
const VisualOutputPane = forwardRef((_props, ref) => {
1110
const codeRunTriggered = useSelector(
1211
(state) => state.editor.codeRunTriggered,
1312
);
@@ -25,11 +24,25 @@ const VisualOutputPane = () => {
2524
const dispatch = useDispatch();
2625
const { t } = useTranslation();
2726

27+
useImperativeHandle(
28+
ref,
29+
() => ({
30+
getTurtleTarget: () => turtleOutput.current || null,
31+
}),
32+
[],
33+
);
34+
2835
useEffect(() => {
2936
if (codeRunTriggered) {
30-
turtleOutput.current.innerHTML = "";
31-
pygalOutput.current.innerHTML = "";
32-
p5Output.current.innerHTML = "";
37+
if (turtleOutput.current) {
38+
turtleOutput.current.innerHTML = "";
39+
}
40+
if (pygalOutput.current) {
41+
pygalOutput.current.innerHTML = "";
42+
}
43+
if (p5Output.current) {
44+
p5Output.current.innerHTML = "";
45+
}
3346

3447
if (!window.py5) {
3548
window.py5 = {};
@@ -43,11 +56,6 @@ const VisualOutputPane = () => {
4356
window.assets = projectImages;
4457

4558
(Sk.pygal || (Sk.pygal = {})).outputCanvas = pygalOutput.current;
46-
47-
configureTurtleGraphics({
48-
targetEl: turtleOutput.current,
49-
projectImages,
50-
});
5159
}
5260
}, [codeRunTriggered, projectImages]);
5361

@@ -84,6 +92,6 @@ const VisualOutputPane = () => {
8492
{senseHatEnabled || senseHatAlwaysEnabled ? <AstroPiModel /> : null}
8593
</div>
8694
);
87-
};
95+
});
8896

8997
export default VisualOutputPane;

src/components/Editor/Runners/PythonRunner/VisualOutputPane.test.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ describe("When Sense Hat library not used", () => {
7777
describe("When code run is triggered", () => {
7878
let store;
7979
let container;
80+
let turtleOutputRef;
8081

8182
beforeEach(() => {
8283
const middlewares = [];
@@ -91,9 +92,10 @@ describe("When code run is triggered", () => {
9192
},
9293
};
9394
store = mockStore(initialState);
95+
turtleOutputRef = React.createRef();
9496
const renderResult = render(
9597
<Provider store={store}>
96-
<VisualOutputPane />
98+
<VisualOutputPane ref={turtleOutputRef} />
9799
</Provider>,
98100
);
99101
container = renderResult.container;
@@ -108,7 +110,7 @@ describe("When code run is triggered", () => {
108110
expect(Sk.pygal.outputCanvas).not.toBeNull();
109111
});
110112

111-
test("Sets up turtle canvas", () => {
112-
expect(Sk.TurtleGraphics.target).not.toBeNull();
113+
test("Exposes turtle target via ref", () => {
114+
expect(turtleOutputRef.current.getTurtleTarget()).not.toBeNull();
113115
});
114116
});

0 commit comments

Comments
 (0)