Skip to content

Commit 815484d

Browse files
williamsteinwilliamstein
authored
Merge pull request #7745 from sagemathinc/issue-5991
Fix Issue 5991
2 parents 7055d84 + 9a62934 commit 815484d

File tree

13 files changed

+687
-406
lines changed

13 files changed

+687
-406
lines changed

‎src/packages/frontend/editors/slate/elements/codemirror.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ export const SlateCodeMirror: React.FC<Props> = React.memo(
9494
} else {
9595
// everything selected, so now select all editor content.
9696
// NOTE that this only makes sense if we change focus
97-
// to the enclosing select editor, thus loosing the
97+
// to the enclosing select editor, thus losing the
9898
// cm editor focus, which is a bit weird.
9999
ReactEditor.focus(editor);
100100
selectAll(editor);

‎src/packages/frontend/jupyter/cell-list.tsx

Lines changed: 79 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import { Cell } from "./cell";
3939
import HeadingTagComponent from "./heading-tag";
4040
interface IFrameContextType {
4141
iframeDivRef?: MutableRefObject<any>;
42+
cellListDivRef?: MutableRefObject<any>;
4243
iframeOnScrolls?: { [key: string]: () => void };
4344
}
4445
const IFrameContext = createContext<IFrameContextType>({});
@@ -597,85 +598,90 @@ export const CellList: React.FC<CellListProps> = (props: CellListProps) => {
597598
let body;
598599

599600
const iframeDivRef = useRef<HTMLDivElement>(null);
601+
const cellListDivRef = useRef<HTMLDivElement>(null);
600602
const virtuosoHeightsRef = useRef<{ [index: number]: number }>({});
601603
if (use_windowed_list) {
602604
body = (
603-
<IFrameContext.Provider value={{ iframeDivRef, iframeOnScrolls }}>
604-
<Virtuoso
605-
ref={virtuosoRef}
606-
onClick={actions != null && complete != null ? on_click : undefined}
607-
topItemCount={EXTRA_TOP_CELLS}
608-
style={{
609-
fontSize: `${font_size}px`,
610-
flex: 1,
611-
overflowX: "hidden",
612-
}}
613-
totalCount={
614-
cell_list.size +
615-
EXTRA_TOP_CELLS /* +EXTRA_TOP_CELLS due to the iframe cell and style cell at the top */ +
616-
EXTRA_BOTTOM_CELLS
617-
}
618-
itemSize={(el) => {
619-
// We capture measured heights -- see big coment above the
620-
// the DivTempHeight component below for why this is needed
621-
// for Jupyter notebooks (but not most things).
622-
const h = el.getBoundingClientRect().height;
623-
// WARNING: This uses perhaps an internal implementation detail of
624-
// virtuoso, which I hope they don't change, which is that the index of
625-
// the elements whose height we're measuring is in the data-item-index
626-
// attribute.
627-
const data = el.getAttribute("data-item-index");
628-
if (data != null) {
629-
const index = parseInt(data);
630-
virtuosoHeightsRef.current[index] = h;
605+
<IFrameContext.Provider
606+
value={{ iframeDivRef, cellListDivRef, iframeOnScrolls }}
607+
>
608+
<div ref={cellListDivRef} className="smc-vfill">
609+
<Virtuoso
610+
ref={virtuosoRef}
611+
onClick={actions != null && complete != null ? on_click : undefined}
612+
topItemCount={EXTRA_TOP_CELLS}
613+
style={{
614+
fontSize: `${font_size}px`,
615+
flex: 1,
616+
overflowX: "hidden",
617+
}}
618+
totalCount={
619+
cell_list.size +
620+
EXTRA_TOP_CELLS /* +EXTRA_TOP_CELLS due to the iframe cell and style cell at the top */ +
621+
EXTRA_BOTTOM_CELLS
631622
}
632-
return h;
633-
}}
634-
itemContent={(index) => {
635-
if (index == 0) {
636-
return (
637-
<div key="iframes" ref={iframeDivRef} style={ITEM_STYLE}>
638-
iframes here
639-
</div>
640-
);
641-
} else if (index == 1) {
623+
itemSize={(el) => {
624+
// We capture measured heights -- see big coment above the
625+
// the DivTempHeight component below for why this is needed
626+
// for Jupyter notebooks (but not most things).
627+
const h = el.getBoundingClientRect().height;
628+
// WARNING: This uses perhaps an internal implementation detail of
629+
// virtuoso, which I hope they don't change, which is that the index of
630+
// the elements whose height we're measuring is in the data-item-index
631+
// attribute.
632+
const data = el.getAttribute("data-item-index");
633+
if (data != null) {
634+
const index = parseInt(data);
635+
virtuosoHeightsRef.current[index] = h;
636+
}
637+
return h;
638+
}}
639+
itemContent={(index) => {
640+
if (index == 0) {
641+
return (
642+
<div key="iframes" ref={iframeDivRef} style={ITEM_STYLE}>
643+
iframes here
644+
</div>
645+
);
646+
} else if (index == 1) {
647+
return (
648+
<div key="styles" ref={iframeDivRef} style={ITEM_STYLE}>
649+
<style>{allStyles}</style>
650+
</div>
651+
);
652+
} else if (index == cell_list.size + EXTRA_TOP_CELLS) {
653+
return BOTTOM_PADDING_CELL;
654+
}
655+
const id = cell_list.get(index - EXTRA_TOP_CELLS);
656+
if (id == null) return null;
657+
const h = virtuosoHeightsRef.current[index];
658+
if (actions == null) {
659+
return render_cell({
660+
id,
661+
isScrolling: false,
662+
index: index - EXTRA_TOP_CELLS,
663+
});
664+
}
642665
return (
643-
<div key="styles" ref={iframeDivRef} style={ITEM_STYLE}>
644-
<style>{allStyles}</style>
645-
</div>
666+
<SortableItem id={id} key={id}>
667+
<DivTempHeight height={h ? `${h}px` : undefined}>
668+
{render_cell({
669+
id,
670+
isScrolling: false,
671+
index: index - EXTRA_TOP_CELLS,
672+
isFirst: id === cell_list.get(0),
673+
isLast: id === cell_list.get(-1),
674+
})}
675+
</DivTempHeight>
676+
</SortableItem>
646677
);
647-
} else if (index == cell_list.size + EXTRA_TOP_CELLS) {
648-
return BOTTOM_PADDING_CELL;
649-
}
650-
const id = cell_list.get(index - EXTRA_TOP_CELLS);
651-
if (id == null) return null;
652-
const h = virtuosoHeightsRef.current[index];
653-
if (actions == null) {
654-
return render_cell({
655-
id,
656-
isScrolling: false,
657-
index: index - EXTRA_TOP_CELLS,
658-
});
659-
}
660-
return (
661-
<SortableItem id={id} key={id}>
662-
<DivTempHeight height={h ? `${h}px` : undefined}>
663-
{render_cell({
664-
id,
665-
isScrolling: false,
666-
index: index - EXTRA_TOP_CELLS,
667-
isFirst: id === cell_list.get(0),
668-
isLast: id === cell_list.get(-1),
669-
})}
670-
</DivTempHeight>
671-
</SortableItem>
672-
);
673-
}}
674-
rangeChanged={(visibleRange) => {
675-
virtuosoRangeRef.current = visibleRange;
676-
}}
677-
{...virtuosoScroll}
678-
/>
678+
}}
679+
rangeChanged={(visibleRange) => {
680+
virtuosoRangeRef.current = visibleRange;
681+
}}
682+
{...virtuosoScroll}
683+
/>
684+
</div>
679685
</IFrameContext.Provider>
680686
);
681687
} else {

‎src/packages/frontend/jupyter/output-messages/cached-iframe.tsx

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,18 @@
11
/*
22
3-
It is completely impossible in modern times to move an iframe in the DOM without loosing state,
4-
as explained here:
5-
https://stackoverflow.com/questions/8318264/how-to-move-an-iframe-in-the-dom-without-losing-its-state
3+
It is completely impossible in modern times to move an iframe's position in the DOM *tree*
4+
without losing state, as explained here:
65
7-
An annoying issue is that if you do shift+tab to get docs or hit the TimeTravel button or anything to
8-
split the Jupyter frame, then the iframes of course get reset. That was a problem before this though,
9-
i.e., it's not special to using windowing.
6+
https://stackoverflow.com/questions/8318264/how-to-move-an-iframe-in-the-dom-without-losing-its-state
7+
8+
That's what we instead use position absolute, and literally move the iframe's position on the screen.
9+
We never move it in the dom.
10+
11+
Another unsolved issue is that if you do shift+tab to get docs or hit the TimeTravel button
12+
or anything to split the Jupyter frame, then the iframes of course get reset. That was
13+
a problem before this though, i.e., it's not special to using windowing. However, it
14+
is a problem we intend to solve, e.g., maybe we put the iframes somewhere else in the
15+
DOM and can reuse when rendering the notebook after the frame split happens.
1016
*/
1117

1218
import { Button, Tooltip } from "antd";
@@ -18,7 +24,8 @@ import { delay } from "awaiting";
1824
import useIsMountedRef from "@cocalc/frontend/app-framework/is-mounted-hook";
1925
import useResizeObserver from "use-resize-observer";
2026

21-
// This is just an initial default height; the actual height of the iframe resizes to the content.
27+
// This is just an initial default height; the actual height of the iframe should
28+
// resize to the content.
2229
const HEIGHT = "70vh";
2330

2431
interface Props {

‎src/packages/frontend/jupyter/output-messages/iframe.tsx

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,27 +15,47 @@ import useCounter from "@cocalc/frontend/app-framework/counter-hook";
1515
import { get_blob_url } from "../server-urls";
1616
import CachedIFrame from "./cached-iframe";
1717
import { useIFrameContext } from "@cocalc/frontend/jupyter/cell-list";
18+
import HTML from "./mime-types/html";
1819

1920
// This impact loading the iframe data from the backend project (via the sha1 hash).
2021
// Doing retries is useful, e.g., since the project might not be running.
2122
const MAX_ATTEMPTS = 10;
2223
const MAX_WAIT = 5000;
2324
const BACKOFF = 1.3;
2425

26+
const HEIGHT = "70vh";
27+
const WIDTH = "max(800px,70vw)";
28+
2529
interface Props {
2630
sha1: string;
2731
project_id: string;
2832
cacheId?: string;
33+
index?: number;
34+
trust?: boolean;
2935
}
3036

3137
export default function IFrame(props: Props) {
32-
const iframeContext = useIFrameContext(); // we only use cached iframe if the iframecontext is setup, e.g., it is in Jupyter notebooks, but not in whiteboards.
33-
return iframeContext.iframeDivRef == null || props.cacheId == null ? (
34-
<NonCachedIFrame {...props} />
35-
) : (
38+
// we only use cached iframe if the iframecontext is setup, e.g., it is in Jupyter notebooks, but not in whiteboards.
39+
const iframeContext = useIFrameContext();
40+
if (
41+
iframeContext.iframeDivRef == null ||
42+
props.cacheId == null ||
43+
!props.trust
44+
) {
45+
return <NonCachedIFrame {...props} />;
46+
} else {
47+
const src = get_blob_url(props.project_id, "html", props.sha1);
48+
return (
49+
<HTML
50+
id={props.cacheId}
51+
index={props.index}
52+
trust={props.trust}
53+
value={`<iframe src="${src}" style="border:0;height:${HEIGHT};width:${WIDTH}"/>`}
54+
/>
55+
);
3656
// @ts-ignore
37-
<CachedIFrame {...props} />
38-
);
57+
return <CachedIFrame {...props} />;
58+
}
3959
}
4060

4161
function NonCachedIFrame({ sha1, project_id }: Props) {

‎src/packages/frontend/jupyter/output-messages/javascript.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import { List } from "immutable";
77
import $ from "jquery";
8-
import React, { useRef, useState } from "react";
8+
import { useEffect, useRef, useState } from "react";
99
import { is_array } from "@cocalc/util/misc";
1010
import { javascript_eval } from "./javascript-eval";
1111
import ShowError from "@cocalc/frontend/components/error";
@@ -24,7 +24,7 @@ export const Javascript: React.FC<JavascriptProps> = (
2424

2525
const [errors, set_errors] = useState<string | undefined>(undefined);
2626

27-
React.useEffect(() => {
27+
useEffect(() => {
2828
if (value == null || node.current == null) {
2929
return;
3030
}

‎src/packages/frontend/jupyter/output-messages/message.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ interface CellOutputMessageProps {
5656
actions?: JupyterActions; // optional - not needed by most messages
5757
name?: string;
5858
id?: string; // optional, and not usually needed either; this is the id of the cell. It is needed for iframe + windowing.
59+
index?: number;
5960
trust?: boolean; // is notebook trusted by the user (if not won't eval javascript)
6061
}
6162

@@ -71,6 +72,7 @@ export const CellOutputMessage: React.FC<CellOutputMessageProps> = React.memo(
7172
name={props.name}
7273
trust={props.trust}
7374
id={props.id}
75+
index={props.index}
7476
/>
7577
);
7678
},
@@ -137,6 +139,7 @@ export const CellOutputMessages: React.FC<CellOutputMessagesProps> = React.memo(
137139
v.push(
138140
<CellOutputMessage
139141
key={n}
142+
index={n}
140143
message={mesg}
141144
project_id={project_id}
142145
directory={directory}

‎src/packages/frontend/jupyter/output-messages/mime-types/html.tsx

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,24 @@
11
import register from "./register";
22
import HTML from "@cocalc/frontend/components/html-ssr";
3+
import StableUnsafeHtml from "../stable-unsafe-html";
34

4-
const Html = ({ value }: { value: string }) => {
5+
const Html = ({
6+
value,
7+
id,
8+
index,
9+
trust,
10+
}: {
11+
value: string;
12+
id?: string;
13+
index?: number;
14+
trust?: boolean;
15+
}) => {
16+
if (!trust) {
17+
<HTML value={value} />;
18+
}
519
return (
620
<div style={{ margin: "5px 0" }}>
7-
<HTML value={value} />
21+
<StableUnsafeHtml html={value} docId={`${id}-${index}`} />
822
</div>
923
);
1024
};

‎src/packages/frontend/jupyter/output-messages/mime-types/iframe.tsx

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,17 @@
11
import register from "./register";
22
import IFrame from "../iframe";
33

4-
register("iframe", 7, ({ id, project_id, value }) => {
4+
register("iframe", 7, ({ id, project_id, value, index, trust }) => {
55
if (value == null || project_id == null) {
66
return <pre>iframe must specify project_id and sha1</pre>;
77
}
8-
return <IFrame cacheId={id} sha1={value} project_id={project_id} />;
8+
return (
9+
<IFrame
10+
cacheId={id}
11+
sha1={value}
12+
project_id={project_id}
13+
index={index}
14+
trust={trust}
15+
/>
16+
);
917
});

‎src/packages/frontend/jupyter/output-messages/mime-types/register.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export interface HandlerProps {
2424
name?: string;
2525
trust?: boolean;
2626
id?: string;
27+
index?: number;
2728
}
2829

2930
type Handler = React.FC<HandlerProps>;

0 commit comments

Comments
 (0)