Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@ export function useNotebookActions() {

const downloadPDF = async (progress: ProgressState) => {
await updateCellOutputsWithScreenshots({
progress,
takeScreenshots,
takeScreenshots: () =>
takeScreenshots({ progress, snappy: false }),
updateCellOutputs,
});
await downloadAsPDF({
Expand Down
105 changes: 72 additions & 33 deletions frontend/src/core/export/__tests__/hooks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ describe("useEnrichCellOutputs", () => {

const { result } = renderHook(() => useEnrichCellOutputs(), { wrapper });

const enrichCellOutputs = result.current;
const output = await enrichCellOutputs(progress);
const takeScreenshots = result.current;
const output = await takeScreenshots({ progress, snappy: false });

expect(output).toEqual({});
expect(document.getElementById).not.toHaveBeenCalled();
Expand Down Expand Up @@ -134,8 +134,8 @@ describe("useEnrichCellOutputs", () => {

const { result } = renderHook(() => useEnrichCellOutputs(), { wrapper });

const enrichCellOutputs = result.current;
const output = await enrichCellOutputs(progress);
const takeScreenshots = result.current;
const output = await takeScreenshots({ progress, snappy: false });

expect(document.getElementById).toHaveBeenCalledWith(
CellOutputId.create(cellId),
Expand All @@ -152,6 +152,50 @@ describe("useEnrichCellOutputs", () => {
});
});

it("should pass snappy=true to toPng with includeStyleProperties", async () => {
const cellId = "cell-1" as CellId;
const mockElement = document.createElement("div");
const mockDataUrl = "data:image/png;base64,mockImageData";

// Mock document.getElementById
vi.spyOn(document, "getElementById").mockReturnValue(mockElement);
vi.mocked(toPng).mockResolvedValue(mockDataUrl);

setCellsRuntime(
createMockCellRuntimes({
[cellId]: {
output: {
channel: "output",
mimetype: "text/html",
data: "<div>Chart</div>",
timestamp: 0,
},
},
}),
);

const { result } = renderHook(() => useEnrichCellOutputs(), { wrapper });

const takeScreenshots = result.current;
const output = await takeScreenshots({ progress, snappy: true });

expect(document.getElementById).toHaveBeenCalledWith(
CellOutputId.create(cellId),
);
// When snappy=true, includeStyleProperties should be set
expect(toPng).toHaveBeenCalledWith(
mockElement,
expect.objectContaining({
filter: expect.any(Function),
onImageErrorHandler: expect.any(Function),
includeStyleProperties: expect.any(Array),
}),
);
expect(output).toEqual({
[cellId]: ["image/png", mockDataUrl],
});
});

it("should skip cells where output has not changed", async () => {
const cellId = "cell-1" as CellId;
const mockElement = document.createElement("div");
Expand Down Expand Up @@ -179,17 +223,17 @@ describe("useEnrichCellOutputs", () => {
});

// First call - should capture
let enrichCellOutputs = result.current;
let output = await enrichCellOutputs(progress);
let takeScreenshots = result.current;
let output = await takeScreenshots({ progress, snappy: false });
expect(output).toEqual({ [cellId]: ["image/png", mockDataUrl] });
expect(toPng).toHaveBeenCalledTimes(1);

// Rerender to get updated atom state
rerender();

// Second call with same output - should not capture again
enrichCellOutputs = result.current;
output = await enrichCellOutputs(progress);
takeScreenshots = result.current;
output = await takeScreenshots({ progress, snappy: false });
expect(output).toEqual({}); // Empty because output hasn't changed
expect(toPng).toHaveBeenCalledTimes(1); // Still only 1 call
});
Expand Down Expand Up @@ -217,8 +261,8 @@ describe("useEnrichCellOutputs", () => {

const { result } = renderHook(() => useEnrichCellOutputs(), { wrapper });

const enrichCellOutputs = result.current;
const output = await enrichCellOutputs(progress);
const takeScreenshots = result.current;
const output = await takeScreenshots({ progress, snappy: false });

expect(output).toEqual({}); // Failed screenshot should be filtered out
expect(Logger.error).toHaveBeenCalledWith(
Expand Down Expand Up @@ -247,8 +291,8 @@ describe("useEnrichCellOutputs", () => {

const { result } = renderHook(() => useEnrichCellOutputs(), { wrapper });

const enrichCellOutputs = result.current;
const output = await enrichCellOutputs(progress);
const takeScreenshots = result.current;
const output = await takeScreenshots({ progress, snappy: false });

expect(output).toEqual({});
expect(Logger.error).toHaveBeenCalledWith(
Expand Down Expand Up @@ -296,8 +340,8 @@ describe("useEnrichCellOutputs", () => {

const { result } = renderHook(() => useEnrichCellOutputs(), { wrapper });

const enrichCellOutputs = result.current;
const output = await enrichCellOutputs(progress);
const takeScreenshots = result.current;
const output = await takeScreenshots({ progress, snappy: false });

expect(output).toEqual({
[cell1]: ["image/png", mockDataUrl1],
Expand Down Expand Up @@ -342,8 +386,8 @@ describe("useEnrichCellOutputs", () => {

const { result } = renderHook(() => useEnrichCellOutputs(), { wrapper });

const enrichCellOutputs = result.current;
const output = await enrichCellOutputs(progress);
const takeScreenshots = result.current;
const output = await takeScreenshots({ progress, snappy: false });

// Only the successful screenshot should be in the result
expect(output).toEqual({
Expand Down Expand Up @@ -384,14 +428,14 @@ describe("useEnrichCellOutputs", () => {
});

// First screenshot
let enrichCellOutputs = result.current;
let output = await enrichCellOutputs(progress);
let takeScreenshots = result.current;
let output = await takeScreenshots({ progress, snappy: false });
expect(output).toEqual({ [cellId]: ["image/png", mockDataUrl1] });

// Second call - same output, should not be captured
rerender();
enrichCellOutputs = result.current;
output = await enrichCellOutputs(progress);
takeScreenshots = result.current;
output = await takeScreenshots({ progress, snappy: false });
expect(output).toEqual({});

// Third call - output changed, should be captured
Expand All @@ -409,8 +453,8 @@ describe("useEnrichCellOutputs", () => {
);

rerender();
enrichCellOutputs = result.current;
output = await enrichCellOutputs(progress);
takeScreenshots = result.current;
output = await takeScreenshots({ progress, snappy: false });
expect(output).toEqual({ [cellId]: ["image/png", mockDataUrl2] });
expect(toPng).toHaveBeenCalledTimes(2);
});
Expand Down Expand Up @@ -449,8 +493,8 @@ describe("useEnrichCellOutputs", () => {

const { result } = renderHook(() => useEnrichCellOutputs(), { wrapper });

const enrichCellOutputs = result.current;
const output = await enrichCellOutputs(progress);
const takeScreenshots = result.current;
const output = await takeScreenshots({ progress, snappy: false });

// None of these should trigger screenshots
expect(output).toEqual({});
Expand All @@ -474,8 +518,8 @@ describe("useEnrichCellOutputs", () => {

const { result } = renderHook(() => useEnrichCellOutputs(), { wrapper });

const enrichCellOutputs = result.current;
const output = await enrichCellOutputs(progress);
const takeScreenshots = result.current;
const output = await takeScreenshots({ progress, snappy: false });

expect(output).toEqual({});
expect(document.getElementById).not.toHaveBeenCalled();
Expand Down Expand Up @@ -506,8 +550,8 @@ describe("useEnrichCellOutputs", () => {

const { result } = renderHook(() => useEnrichCellOutputs(), { wrapper });

const enrichCellOutputs = result.current;
const output = await enrichCellOutputs(progress);
const takeScreenshots = result.current;
const output = await takeScreenshots({ progress, snappy: false });

// Verify the exact return type structure
expect(output).toHaveProperty(cellId);
Expand Down Expand Up @@ -539,7 +583,6 @@ describe("updateCellOutputsWithScreenshots", () => {
const updateCellOutputs = vi.fn().mockResolvedValue(null);

await updateCellOutputsWithScreenshots({
progress,
takeScreenshots,
updateCellOutputs,
});
Expand All @@ -556,7 +599,6 @@ describe("updateCellOutputsWithScreenshots", () => {
const updateCellOutputs = vi.fn().mockResolvedValue(null);

await updateCellOutputsWithScreenshots({
progress,
takeScreenshots,
updateCellOutputs,
});
Expand All @@ -583,7 +625,6 @@ describe("updateCellOutputsWithScreenshots", () => {
const updateCellOutputs = vi.fn().mockResolvedValue(null);

await updateCellOutputsWithScreenshots({
progress,
takeScreenshots,
updateCellOutputs,
});
Expand All @@ -600,7 +641,6 @@ describe("updateCellOutputsWithScreenshots", () => {

// Should not throw - errors are caught and shown via toast
await updateCellOutputsWithScreenshots({
progress,
takeScreenshots,
updateCellOutputs,
});
Expand Down Expand Up @@ -633,7 +673,6 @@ describe("updateCellOutputsWithScreenshots", () => {

// Should not throw - errors are caught and shown via toast
await updateCellOutputsWithScreenshots({
progress,
takeScreenshots,
updateCellOutputs,
});
Expand Down
25 changes: 17 additions & 8 deletions frontend/src/core/export/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,13 @@ export function useAutoExport() {

useInterval(
async () => {
const screenshotFn = () =>
takeScreenshots({
progress: ProgressState.indeterminate(),
snappy: true,
});
await updateCellOutputsWithScreenshots({
progress: ProgressState.indeterminate(),
takeScreenshots,
takeScreenshots: screenshotFn,
updateCellOutputs,
});
await autoExportAsIPYNB({
Expand Down Expand Up @@ -107,7 +111,13 @@ export function useEnrichCellOutputs() {
const [richCellsOutput, setRichCellsOutput] = useAtom(richCellsToOutputAtom);
const cellRuntimes = useAtomValue(cellsRuntimeAtom);

return async (progress: ProgressState): Promise<ScreenshotResults> => {
return async ({
progress,
snappy,
}: {
progress: ProgressState;
snappy: boolean;
}): Promise<ScreenshotResults> => {
const trackedCellsOutput: Record<CellId, unknown> = {};

const cellsToCaptureScreenshot: [CellId, unknown][] = [];
Expand Down Expand Up @@ -138,7 +148,7 @@ export function useEnrichCellOutputs() {
const results: ScreenshotResults = {};
for (const [cellId] of cellsToCaptureScreenshot) {
try {
const dataUrl = await getImageDataUrlForCell(cellId, false);
const dataUrl = await getImageDataUrlForCell(cellId, snappy);
if (!dataUrl) {
Logger.error(`Failed to capture screenshot for cell ${cellId}`);
continue;
Expand All @@ -159,13 +169,12 @@ export function useEnrichCellOutputs() {
* Utility function to take screenshots of cells with HTML outputs and update the cell outputs.
*/
export async function updateCellOutputsWithScreenshots(opts: {
progress: ProgressState;
takeScreenshots: (progress: ProgressState) => Promise<ScreenshotResults>;
takeScreenshots: () => Promise<ScreenshotResults>;
updateCellOutputs: (request: UpdateCellOutputsRequest) => Promise<null>;
}) {
const { progress, takeScreenshots, updateCellOutputs } = opts;
const { takeScreenshots, updateCellOutputs } = opts;
try {
const cellIdsToOutput = await takeScreenshots(progress);
const cellIdsToOutput = await takeScreenshots();
if (Objects.size(cellIdsToOutput) > 0) {
await updateCellOutputs({ cellIdsToOutput });
}
Expand Down
Loading
Loading