Skip to content

Commit 1bc8942

Browse files
committed
deleting a lot of complicated code from the jupyter cell list which isn't needed with our new approach
1 parent 06c8550 commit 1bc8942

File tree

3 files changed

+21
-88
lines changed

3 files changed

+21
-88
lines changed

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

Lines changed: 17 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@ import {
2121
useRef,
2222
} from "react";
2323
import { Virtuoso, VirtuosoHandle } from "react-virtuoso";
24-
import { useDebounce } from "use-debounce";
25-
2624
import { CSS, React, useIsMountedRef } from "@cocalc/frontend/app-framework";
2725
import { Loading } from "@cocalc/frontend/components";
2826
import {
@@ -38,7 +36,6 @@ import { JupyterActions } from "./browser-actions";
3836
import { Cell } from "./cell";
3937
import HeadingTagComponent from "./heading-tag";
4038
interface StableHtmlContextType {
41-
iframeDivRef?: MutableRefObject<any>;
4239
cellListDivRef?: MutableRefObject<any>;
4340
iframeOnScrolls?: { [key: string]: () => void };
4441
}
@@ -51,7 +48,6 @@ export const useStableHtmlContext: () => StableHtmlContextType = () => {
5148
// - iframe cell (hidden at top)
5249
// - style cell (hidden at top)
5350
// - padding (at the bottom)
54-
const EXTRA_TOP_CELLS = 2;
5551
const EXTRA_BOTTOM_CELLS = 1;
5652

5753
const CELL_VISIBLE_THRESH = 50;
@@ -67,11 +63,6 @@ const BOTTOM_PADDING_CELL = (
6763
></div>
6864
);
6965

70-
const ITEM_STYLE: CSS = {
71-
height: "1px",
72-
overflow: "hidden",
73-
};
74-
7566
interface CellListProps {
7667
actions?: JupyterActions; // if not defined, then everything is read only
7768
cell_list: immutable.List<string>; // list of ids of cells in order
@@ -284,9 +275,6 @@ export const CellList: React.FC<CellListProps> = (props: CellListProps) => {
284275
}
285276

286277
function scrollCellListVirtuoso(scroll: Scroll) {
287-
// NOTE: below we add EXTRA_TOP_CELLS to the index to compensate
288-
// for the first fixed hidden cell that contains all
289-
// of the output iframes!
290278
if (typeof scroll == "number") {
291279
// scroll to a number is not meaningful for virtuoso; it might
292280
// be requested maybe (?) due to scroll restore and switching
@@ -304,7 +292,7 @@ export const CellList: React.FC<CellListProps> = (props: CellListProps) => {
304292
// We ONLY scroll if the cell is not in the visible, since
305293
// react-virtuoso's "scrollIntoView" aggressively scrolls, even
306294
// if the item is in view.
307-
const n = index + EXTRA_TOP_CELLS;
295+
const n = index;
308296
let isNotVisible = false;
309297
let align: "start" | "center" | "end" = "start";
310298
if (n < virtuosoRangeRef.current.startIndex) {
@@ -346,40 +334,40 @@ export const CellList: React.FC<CellListProps> = (props: CellListProps) => {
346334
}
347335
} else if (scroll == "cell top") {
348336
virtuosoRef.current?.scrollToIndex({
349-
index: index + EXTRA_TOP_CELLS,
337+
index,
350338
});
351339
// hack which seems necessary for jupyter at least.
352340
requestAnimationFrame(
353341
() =>
354342
virtuosoRef.current?.scrollToIndex({
355-
index: index + EXTRA_TOP_CELLS,
343+
index,
356344
}),
357345
);
358346
}
359347
} else if (scroll.startsWith("list")) {
360348
if (scroll == "list up") {
361349
const index = virtuosoRangeRef.current?.startIndex;
362350
virtuosoRef.current?.scrollToIndex({
363-
index: index + EXTRA_TOP_CELLS,
351+
index,
364352
align: "end",
365353
});
366354
requestAnimationFrame(
367355
() =>
368356
virtuosoRef.current?.scrollToIndex({
369-
index: index + EXTRA_TOP_CELLS,
357+
index,
370358
align: "end",
371359
}),
372360
);
373361
} else if (scroll == "list down") {
374362
const index = virtuosoRangeRef.current?.endIndex;
375363
virtuosoRef.current?.scrollToIndex({
376-
index: index + EXTRA_TOP_CELLS,
364+
index,
377365
align: "start",
378366
});
379367
requestAnimationFrame(
380368
() =>
381369
virtuosoRef.current?.scrollToIndex({
382-
index: index + EXTRA_TOP_CELLS,
370+
index,
383371
align: "start",
384372
}),
385373
);
@@ -515,7 +503,7 @@ export const CellList: React.FC<CellListProps> = (props: CellListProps) => {
515503
onScroll: (scrollState) => {
516504
lastScrollStateRef.current = {
517505
...scrollState,
518-
id: cellListRef.current?.get(scrollState.index - EXTRA_TOP_CELLS),
506+
id: cellListRef.current?.get(scrollState.index),
519507
};
520508
for (const key in iframeOnScrolls) {
521509
iframeOnScrolls[key]();
@@ -539,16 +527,13 @@ export const CellList: React.FC<CellListProps> = (props: CellListProps) => {
539527
if (index == null) {
540528
return;
541529
}
542-
// index + EXTRA_TOP_CELLS because of iframe and style cells
543-
// the offset+1 is I think compensating for a bug maybe in
544-
// virtuoso or our use of it.
545530
virtuosoRef.current?.scrollToIndex({
546-
index: index + EXTRA_TOP_CELLS,
531+
index,
547532
offset: offset + 1,
548533
});
549534
requestAnimationFrame(() => {
550535
virtuosoRef.current?.scrollToIndex({
551-
index: index + EXTRA_TOP_CELLS,
536+
index,
552537
offset: offset + 1,
553538
});
554539
});
@@ -564,62 +549,26 @@ export const CellList: React.FC<CellListProps> = (props: CellListProps) => {
564549
}
565550
}, [cells]);
566551

567-
// allStyles -- the CSS in <style> blocks in text/html outputs
568-
// of all cells. We gather this and place it in a special cell
569-
// at the top, since that such css doesn't disappear when the cells
570-
// that produced it are scrolled off the screen. See
571-
// https://github.com/sagemathinc/cocalc/issues/5943
572-
// We only update allStyles with a debounce of 1s, since it
573-
// can be time consuming as it involves a scan of the entire notebook.
574-
const [debouncedCells] = useDebounce(cells, 1000);
575-
const allStyles = useMemo(() => {
576-
if (!use_windowed_list) return "";
577-
let value = "";
578-
cell_list.forEach((id) => {
579-
(debouncedCells.getIn([id, "output"]) as any)?.forEach((output) => {
580-
// I hit a case in prod of output not being defined. Given the
581-
// debounce and how debouncedCells might not match up with cell_list,
582-
// and how output is going from markdown cells or maybe cleared cells,
583-
// it seems plausible output could contain an undefined entry.
584-
const html = output?.getIn(["data", "text/html"]);
585-
if (html?.includes("style")) {
586-
// parse out and include style tags
587-
for (const x of $("<div>" + html + "</div>").find("style")) {
588-
value += x.innerHTML.trim() + "\n\n";
589-
}
590-
}
591-
});
592-
});
593-
return value;
594-
}, [debouncedCells, use_windowed_list]);
595-
596552
const fileContext = useFileContext();
597553

598554
let body;
599555

600-
const iframeDivRef = useRef<HTMLDivElement>(null);
601556
const cellListDivRef = useRef<HTMLDivElement>(null);
602557
const virtuosoHeightsRef = useRef<{ [index: number]: number }>({});
603558
if (use_windowed_list) {
604559
body = (
605-
<StableHtmlContext.Provider
606-
value={{ iframeDivRef, cellListDivRef, iframeOnScrolls }}
607-
>
560+
<StableHtmlContext.Provider value={{ cellListDivRef, iframeOnScrolls }}>
608561
<div ref={cellListDivRef} className="smc-vfill">
609562
<Virtuoso
610563
ref={virtuosoRef}
611564
onClick={actions != null && complete != null ? on_click : undefined}
612-
topItemCount={EXTRA_TOP_CELLS}
565+
topItemCount={0}
613566
style={{
614567
fontSize: `${font_size}px`,
615568
flex: 1,
616569
overflowX: "hidden",
617570
}}
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
622-
}
571+
totalCount={cell_list.size + EXTRA_BOTTOM_CELLS}
623572
itemSize={(el) => {
624573
// We capture measured heights -- see big coment above the
625574
// the DivTempHeight component below for why this is needed
@@ -637,29 +586,17 @@ export const CellList: React.FC<CellListProps> = (props: CellListProps) => {
637586
return h;
638587
}}
639588
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) {
589+
if (index == cell_list.size) {
653590
return BOTTOM_PADDING_CELL;
654591
}
655-
const id = cell_list.get(index - EXTRA_TOP_CELLS);
592+
const id = cell_list.get(index);
656593
if (id == null) return null;
657594
const h = virtuosoHeightsRef.current[index];
658595
if (actions == null) {
659596
return render_cell({
660597
id,
661598
isScrolling: false,
662-
index: index - EXTRA_TOP_CELLS,
599+
index,
663600
});
664601
}
665602
return (
@@ -668,7 +605,7 @@ export const CellList: React.FC<CellListProps> = (props: CellListProps) => {
668605
{render_cell({
669606
id,
670607
isScrolling: false,
671-
index: index - EXTRA_TOP_CELLS,
608+
index,
672609
isFirst: id === cell_list.get(0),
673610
isLast: id === cell_list.get(-1),
674611
})}

src/packages/frontend/jupyter/cell.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ interface Props {
6060
isLast?: boolean;
6161
dragHandle?: JSX.Element;
6262
read_only?: boolean;
63+
outputDivRef?;
6364
}
6465

6566
function areEqual(props: Props, nextProps: Props): boolean {
@@ -164,6 +165,7 @@ export const Cell: React.FC<Props> = React.memo((props: Props) => {
164165
trust={props.trust}
165166
complete={props.is_current && props.complete != null}
166167
llmTools={props.llmTools}
168+
divRef={props.outputDivRef}
167169
/>
168170
);
169171
}

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import ReactDOM from "react-dom";
1313
import useIsMountedRef from "@cocalc/frontend/app-framework/is-mounted-hook";
1414
import useCounter from "@cocalc/frontend/app-framework/counter-hook";
1515
import { get_blob_url } from "../server-urls";
16-
import { useStableHtmlContext } from "@cocalc/frontend/jupyter/cell-list";
1716
import HTML from "./mime-types/html";
1817

1918
// This impact loading the iframe data from the backend project (via the sha1 hash).
@@ -35,12 +34,7 @@ interface Props {
3534

3635
export default function IFrame(props: Props) {
3736
// we only use cached iframe if the iframecontext is setup, e.g., it is in Jupyter notebooks, but not in whiteboards.
38-
const stableHtmlContext = useStableHtmlContext();
39-
if (
40-
stableHtmlContext.iframeDivRef == null ||
41-
props.cacheId == null ||
42-
!props.trust
43-
) {
37+
if (props.cacheId == null || !props.trust) {
4438
return <NonCachedIFrame {...props} />;
4539
} else {
4640
const src = get_blob_url(props.project_id, "html", props.sha1);
@@ -90,7 +84,7 @@ function NonCachedIFrame({ sha1, project_id }: Props) {
9084
ref={iframeRef}
9185
src={get_blob_url(project_id, "html", sha1) + `&attempts=${attempts}`}
9286
onError={load_error}
93-
style={{ border: 0, width: "100%", minHeight: "500px" }}
87+
style={{ border: 0, width: WIDTH, minHeight: HEIGHT }}
9488
/>
9589
);
9690
}

0 commit comments

Comments
 (0)