From 861042c27e0b239ba42ca3f35f362ab52d1e2c5c Mon Sep 17 00:00:00 2001 From: Laurent Nguyen Date: Tue, 11 May 2021 20:24:05 +0200 Subject: [PATCH] Fix bug publish screenshot (#762) [Preview this branch](https://cosmos-explorer-preview.azurewebsites.net/pull/762?feature.someFeatureFlagYouMightNeed=true) The main change in this PR fixes the snapshot functionality in the Publish pane-related components. Because the code cell outputs are now rendered in their own iframes for security reasons, a single snapshot of the notebook is no longer possible: each cell output takes its own snapshot and the snapshots are collated on the main notebook snapshot. - Move the snapshot functionality to notebook components: this removes the reference of the notebook DOM node that we must pass to the Publish pane via explorer. - Add slice in the state and actions in notebook redux for notebook snapshot requests and result - Add post robot message to take snapshots and receive results - Add logic in `NotebookRenderer` to wait for all output snapshots done before taking the main one collating. - Use `zustand` to share snapshot between Redux world and React world. This solves the issue of keeping the `PanelContainer` component generic, while being able to update its children (`PublishPanel` component) with the new snapshot. Additional changes: - Add `local()` in `@font-face` to check if font is already installed before downloading the font (must be done for Safari, but not Edge/Chrome) - Add "Export output to image" menu item in notebook cell, since each cell output can take its own snapshot (which can be downloaded) ![image](https://user-images.githubusercontent.com/21954022/117454706-b5f16600-af46-11eb-8535-6bf99f3d9170.png) --- less/Common/Constants.less | 2 +- package-lock.json | 16 ++ package.json | 5 +- src/CellOutputViewer/CellOutputViewer.tsx | 36 ++++ src/Explorer/Explorer.tsx | 17 +- .../NotebookComponentAdapter.tsx | 17 +- .../NotebookComponentBootstrapper.tsx | 34 ++- .../Notebook/NotebookComponent/actions.ts | 68 +++++- .../Notebook/NotebookComponent/reducers.ts | 37 +++- .../Notebook/NotebookComponent/types.ts | 40 +++- src/Explorer/Notebook/NotebookManager.tsx | 13 +- .../NotebookRenderer/NotebookRenderer.tsx | 94 ++++++--- .../Notebook/NotebookRenderer/Toolbar.tsx | 38 +++- .../outputs/SandboxOutputs.tsx | 107 ++++++++-- src/Explorer/Notebook/NotebookUtil.test.ts | 12 +- src/Explorer/Notebook/NotebookUtil.ts | 193 ++++++++++++++++-- .../Panes/PanelContainerComponent.test.tsx | 2 +- .../PublishNotebookPane.test.tsx | 3 +- .../PublishNotebookPane.tsx | 20 +- .../PublishNotebookPaneComponent.tsx | 98 +++------ .../PublishNotebookPane.test.tsx.snap | 2 +- src/Explorer/Tabs/NotebookV2Tab.ts | 26 ++- src/hooks/useNotebookSnapshotStore.ts | 15 ++ src/hooks/useSidePanel.ts | 10 +- 24 files changed, 683 insertions(+), 222 deletions(-) create mode 100644 src/hooks/useNotebookSnapshotStore.ts diff --git a/less/Common/Constants.less b/less/Common/Constants.less index 8039450b4..50bdd7d91 100644 --- a/less/Common/Constants.less +++ b/less/Common/Constants.less @@ -4,7 +4,7 @@ @font-face { font-family: wf_segoe-ui_normal; - src: url("../../fonts/segoe-ui/west-european/normal/latest.woff"); + src: local("Segoe UI"), url("../../fonts/segoe-ui/west-european/normal/latest.woff"); } @DataExplorerFont: wf_segoe-ui_normal, "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif; diff --git a/package-lock.json b/package-lock.json index 9379dedc1..c9ed3c821 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5255,6 +5255,12 @@ "@types/d3-selection": "*" } }, + "@types/dom-to-image": { + "version": "2.6.2", + "resolved": "https://registry.npmjs.org/@types/dom-to-image/-/dom-to-image-2.6.2.tgz", + "integrity": "sha512-Yxbwmz/glNwRIXfBI8efG2bgIxrFAKV1MdfpqbUDq25ULMot7U7FYXPiso5G8DlBExSP+AakuG0mNus9yw4RZQ==", + "dev": true + }, "@types/dom4": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/@types/dom4/-/dom4-2.0.1.tgz", @@ -9667,6 +9673,11 @@ } } }, + "dom-to-image": { + "version": "2.6.0", + "resolved": "https://registry.npmjs.org/dom-to-image/-/dom-to-image-2.6.0.tgz", + "integrity": "sha1-ilA2CAiMh7HCL5A0rgMuGJiVWGc=" + }, "dom-walk": { "version": "0.1.2", "resolved": "https://registry.npmjs.org/dom-walk/-/dom-walk-0.1.2.tgz", @@ -25918,6 +25929,11 @@ "version": "1.0.46", "resolved": "https://registry.npmjs.org/zalgo-promise/-/zalgo-promise-1.0.46.tgz", "integrity": "sha512-tzPpQRqaQQavxl17TY98nznvmr+judUg3My7ugsUcRDbdqisYOE2z79HNNDgXnyX3eA0mf2bMOJrqHptt00npg==" + }, + "zustand": { + "version": "3.5.0", + "resolved": "https://registry.npmjs.org/zustand/-/zustand-3.5.0.tgz", + "integrity": "sha512-fwZfax2c954pWB+RYXHXG0HWyuoUj8YiNykRjZC/w6L7ay9fPQ7M90mgDVP1KJsRqxLsDILBWSNxuzw5BkCpxA==" } } } diff --git a/package.json b/package.json index dcd1756e8..467ec71a7 100644 --- a/package.json +++ b/package.json @@ -56,6 +56,7 @@ "datatables.net-dt": "1.10.19", "date-fns": "1.29.0", "dayjs": "1.8.19", + "dom-to-image": "2.6.0", "dotenv": "8.2.0", "eslint-plugin-jest": "23.13.2", "eslint-plugin-react": "7.20.0", @@ -97,7 +98,8 @@ "swr": "0.4.0", "terser-webpack-plugin": "3.1.0", "underscore": "1.9.1", - "utility-types": "3.10.0" + "utility-types": "3.10.0", + "zustand": "3.5.0" }, "devDependencies": { "@babel/core": "7.9.0", @@ -109,6 +111,7 @@ "@types/codemirror": "0.0.56", "@types/crossroads": "0.0.30", "@types/d3": "5.9.2", + "@types/dom-to-image": "2.6.2", "@types/enzyme": "3.10.7", "@types/enzyme-adapter-react-16": "1.0.6", "@types/hasher": "0.0.31", diff --git a/src/CellOutputViewer/CellOutputViewer.tsx b/src/CellOutputViewer/CellOutputViewer.tsx index 3ddd72ca8..40563d304 100644 --- a/src/CellOutputViewer/CellOutputViewer.tsx +++ b/src/CellOutputViewer/CellOutputViewer.tsx @@ -9,11 +9,17 @@ import postRobot from "post-robot"; import * as React from "react"; import * as ReactDOM from "react-dom"; import "../../externals/iframeResizer.contentWindow.min.js"; // Required for iFrameResizer to work +import { SnapshotRequest } from "../Explorer/Notebook/NotebookComponent/types"; import "../Explorer/Notebook/NotebookRenderer/base.css"; import "../Explorer/Notebook/NotebookRenderer/default.css"; +import { NotebookUtil } from "../Explorer/Notebook/NotebookUtil"; import "./CellOutputViewer.less"; import { TransformMedia } from "./TransformMedia"; +export interface SnapshotResponse { + imageSrc: string; + requestId: string; +} export interface CellOutputViewerProps { id: string; contentRef: ContentRef; @@ -62,6 +68,36 @@ const onInit = async () => { ReactDOM.render(outputs, document.getElementById("cellOutput")); } ); + + postRobot.on( + "snapshotRequest", + { + window: window.parent, + domain: window.location.origin, + }, + async (event): Promise => { + const topNode = document.getElementById("cellOutput"); + if (!topNode) { + const errorMsg = "No top node to snapshot"; + return Promise.reject(new Error(errorMsg)); + } + + // Typescript definition for event is wrong. So read props by casting to + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const snapshotRequest = (event as any).data as SnapshotRequest; + const result = await NotebookUtil.takeScreenshotDomToImage( + topNode, + snapshotRequest.aspectRatio, + undefined, + snapshotRequest.downloadFilename + ); + + return { + imageSrc: result.imageSrc, + requestId: snapshotRequest.requestId, + }; + } + ); }; // Entry point diff --git a/src/Explorer/Explorer.tsx b/src/Explorer/Explorer.tsx index fe73a7a49..0fa1f5634 100644 --- a/src/Explorer/Explorer.tsx +++ b/src/Explorer/Explorer.tsx @@ -45,6 +45,7 @@ import { GalleryTab as GalleryTabKind } from "./Controls/NotebookGallery/Gallery import { CommandBarComponentAdapter } from "./Menus/CommandBar/CommandBarComponentAdapter"; import { ConsoleData } from "./Menus/NotificationConsole/NotificationConsoleComponent"; import * as FileSystemUtil from "./Notebook/FileSystemUtil"; +import { SnapshotRequest } from "./Notebook/NotebookComponent/types"; import { NotebookContentItem, NotebookContentItemType } from "./Notebook/NotebookContentItem"; import type NotebookManager from "./Notebook/NotebookManager"; import type { NotebookPaneContent } from "./Notebook/NotebookManager"; @@ -91,7 +92,7 @@ export interface ExplorerParams { setIsNotificationConsoleExpanded: (isExpanded: boolean) => void; setNotificationConsoleData: (consoleData: ConsoleData) => void; setInProgressConsoleDataIdToBeDeleted: (id: string) => void; - openSidePanel: (headerText: string, panelContent: JSX.Element) => void; + openSidePanel: (headerText: string, panelContent: JSX.Element, onClose?: () => void) => void; closeSidePanel: () => void; closeDialog: () => void; openDialog: (props: DialogProps) => void; @@ -125,7 +126,7 @@ export default class Explorer { // Panes public contextPanes: ContextualPaneBase[]; - public openSidePanel: (headerText: string, panelContent: JSX.Element) => void; + public openSidePanel: (headerText: string, panelContent: JSX.Element, onClose?: () => void) => void; public closeSidePanel: () => void; // Resource Tree @@ -1270,10 +1271,18 @@ export default class Explorer { public async publishNotebook( name: string, content: NotebookPaneContent, - parentDomElement?: HTMLElement + notebookContentRef?: string, + onTakeSnapshot?: (request: SnapshotRequest) => void, + onClosePanel?: () => void ): Promise { if (this.notebookManager) { - await this.notebookManager.openPublishNotebookPane(name, content, parentDomElement); + await this.notebookManager.openPublishNotebookPane( + name, + content, + notebookContentRef, + onTakeSnapshot, + onClosePanel + ); this.isPublishNotebookPaneEnabled(true); } } diff --git a/src/Explorer/Notebook/NotebookComponent/NotebookComponentAdapter.tsx b/src/Explorer/Notebook/NotebookComponent/NotebookComponentAdapter.tsx index fe44a7b61..24cb9445d 100644 --- a/src/Explorer/Notebook/NotebookComponent/NotebookComponentAdapter.tsx +++ b/src/Explorer/Notebook/NotebookComponent/NotebookComponentAdapter.tsx @@ -1,14 +1,11 @@ -import * as React from "react"; - -import { ReactAdapter } from "../../../Bindings/ReactBindingHandler"; -import { NotebookClientV2 } from "../NotebookClientV2"; - // Vendor modules import { actions, createContentRef, createKernelRef, selectors } from "@nteract/core"; -import VirtualCommandBarComponent from "./VirtualCommandBarComponent"; +import * as React from "react"; +import { ReactAdapter } from "../../../Bindings/ReactBindingHandler"; +import { NotebookClientV2 } from "../NotebookClientV2"; import { NotebookContentItem } from "../NotebookContentItem"; import { NotebookComponentBootstrapper } from "./NotebookComponentBootstrapper"; -import { CdbAppState } from "./types"; +import VirtualCommandBarComponent from "./VirtualCommandBarComponent"; export interface NotebookComponentAdapterOptions { contentItem: NotebookContentItem; @@ -19,7 +16,6 @@ export interface NotebookComponentAdapterOptions { export class NotebookComponentAdapter extends NotebookComponentBootstrapper implements ReactAdapter { private onUpdateKernelInfo: () => void; - public getNotebookParentElement: () => HTMLElement; public parameters: any; constructor(options: NotebookComponentAdapterOptions) { @@ -46,11 +42,6 @@ export class NotebookComponentAdapter extends NotebookComponentBootstrapper impl }) ); } - - this.getNotebookParentElement = () => { - const cdbAppState = this.getStore().getState() as CdbAppState; - return cdbAppState.cdb.currentNotebookParentElements.get(this.contentRef); - }; } protected renderExtraComponent = (): JSX.Element => { diff --git a/src/Explorer/Notebook/NotebookComponent/NotebookComponentBootstrapper.tsx b/src/Explorer/Notebook/NotebookComponent/NotebookComponentBootstrapper.tsx index 3422f6456..28b8b51b3 100644 --- a/src/Explorer/Notebook/NotebookComponent/NotebookComponentBootstrapper.tsx +++ b/src/Explorer/Notebook/NotebookComponent/NotebookComponentBootstrapper.tsx @@ -1,35 +1,29 @@ -import * as React from "react"; - -import { NotebookComponent } from "./NotebookComponent"; -import { NotebookClientV2 } from "../NotebookClientV2"; -import { NotebookUtil } from "../NotebookUtil"; - +import { CellId, CellType, ImmutableNotebook } from "@nteract/commutable"; // Vendor modules import { actions, AppState, - createKernelRef, - DocumentRecordProps, ContentRef, + DocumentRecordProps, KernelRef, NotebookContentRecord, selectors, } from "@nteract/core"; -import * as Immutable from "immutable"; -import { Provider } from "react-redux"; -import { CellType, CellId, ImmutableNotebook } from "@nteract/commutable"; -import { Store, AnyAction } from "redux"; - -import "./NotebookComponent.less"; - -import "codemirror/addon/hint/show-hint.css"; -import "codemirror/lib/codemirror.css"; import "@nteract/styles/editor-overrides.css"; import "@nteract/styles/global-variables.css"; +import "codemirror/addon/hint/show-hint.css"; +import "codemirror/lib/codemirror.css"; +import * as Immutable from "immutable"; +import * as React from "react"; +import { Provider } from "react-redux"; import "react-table/react-table.css"; - -import * as CdbActions from "./actions"; +import { AnyAction, Store } from "redux"; +import { NotebookClientV2 } from "../NotebookClientV2"; +import { NotebookUtil } from "../NotebookUtil"; import * as NteractUtil from "../NTeractUtil"; +import * as CdbActions from "./actions"; +import { NotebookComponent } from "./NotebookComponent"; +import "./NotebookComponent.less"; export interface NotebookComponentBootstrapperOptions { notebookClient: NotebookClientV2; @@ -37,7 +31,7 @@ export interface NotebookComponentBootstrapperOptions { } export class NotebookComponentBootstrapper { - protected contentRef: ContentRef; + public contentRef: ContentRef; protected renderExtraComponent: () => JSX.Element; private notebookClient: NotebookClientV2; diff --git a/src/Explorer/Notebook/NotebookComponent/actions.ts b/src/Explorer/Notebook/NotebookComponent/actions.ts index 607af0c55..31d8ee170 100644 --- a/src/Explorer/Notebook/NotebookComponent/actions.ts +++ b/src/Explorer/Notebook/NotebookComponent/actions.ts @@ -1,6 +1,7 @@ import { CellId } from "@nteract/commutable"; import { ContentRef } from "@nteract/core"; import { Action } from "../../../Shared/Telemetry/TelemetryConstants"; +import { SnapshotFragment, SnapshotRequest } from "./types"; export const CLOSE_NOTEBOOK = "CLOSE_NOTEBOOK"; export interface CloseNotebookAction { @@ -85,21 +86,68 @@ export const traceNotebookTelemetry = (payload: { }; }; -export const UPDATE_NOTEBOOK_PARENT_DOM_ELTS = "UPDATE_NOTEBOOK_PARENT_DOM_ELTS"; -export interface UpdateNotebookParentDomEltAction { - type: "UPDATE_NOTEBOOK_PARENT_DOM_ELTS"; +export const STORE_CELL_OUTPUT_SNAPSHOT = "STORE_CELL_OUTPUT_SNAPSHOT"; +export interface StoreCellOutputSnapshotAction { + type: "STORE_CELL_OUTPUT_SNAPSHOT"; payload: { - contentRef: ContentRef; - parentElt: HTMLElement; + cellId: string; + snapshot: SnapshotFragment; }; } -export const UpdateNotebookParentDomElt = (payload: { - contentRef: ContentRef; - parentElt: HTMLElement; -}): UpdateNotebookParentDomEltAction => { +export const storeCellOutputSnapshot = (payload: { + cellId: string; + snapshot: SnapshotFragment; +}): StoreCellOutputSnapshotAction => { return { - type: UPDATE_NOTEBOOK_PARENT_DOM_ELTS, + type: STORE_CELL_OUTPUT_SNAPSHOT, + payload, + }; +}; + +export const STORE_NOTEBOOK_SNAPSHOT = "STORE_NOTEBOOK_SNAPSHOT"; +export interface StoreNotebookSnapshotAction { + type: "STORE_NOTEBOOK_SNAPSHOT"; + payload: { + imageSrc: string; + requestId: string; + }; +} + +export const storeNotebookSnapshot = (payload: { + imageSrc: string; + requestId: string; +}): StoreNotebookSnapshotAction => { + return { + type: STORE_NOTEBOOK_SNAPSHOT, + payload, + }; +}; + +export const TAKE_NOTEBOOK_SNAPSHOT = "TAKE_NOTEBOOK_SNAPSHOT"; +export interface TakeNotebookSnapshotAction { + type: "TAKE_NOTEBOOK_SNAPSHOT"; + payload: SnapshotRequest; +} + +export const takeNotebookSnapshot = (payload: SnapshotRequest): TakeNotebookSnapshotAction => { + return { + type: TAKE_NOTEBOOK_SNAPSHOT, + payload, + }; +}; + +export const NOTEBOOK_SNAPSHOT_ERROR = "NOTEBOOK_SNAPSHOT_ERROR"; +export interface NotebookSnapshotErrorAction { + type: "NOTEBOOK_SNAPSHOT_ERROR"; + payload: { + error: string; + }; +} + +export const notebookSnapshotError = (payload: { error: string }): NotebookSnapshotErrorAction => { + return { + type: NOTEBOOK_SNAPSHOT_ERROR, payload, }; }; diff --git a/src/Explorer/Notebook/NotebookComponent/reducers.ts b/src/Explorer/Notebook/NotebookComponent/reducers.ts index 6638c87a7..e97a55304 100644 --- a/src/Explorer/Notebook/NotebookComponent/reducers.ts +++ b/src/Explorer/Notebook/NotebookComponent/reducers.ts @@ -70,17 +70,32 @@ export const cdbReducer = (state: CdbRecord, action: Action) => { return state.set("hoveredCellId", typedAction.payload.cellId); } - case cdbActions.UPDATE_NOTEBOOK_PARENT_DOM_ELTS: { - const typedAction = action as cdbActions.UpdateNotebookParentDomEltAction; - var parentEltsMap = state.get("currentNotebookParentElements"); - const contentRef = typedAction.payload.contentRef; - const parentElt = typedAction.payload.parentElt; - if (parentElt) { - parentEltsMap.set(contentRef, parentElt); - } else { - parentEltsMap.delete(contentRef); - } - return state.set("currentNotebookParentElements", parentEltsMap); + case cdbActions.STORE_CELL_OUTPUT_SNAPSHOT: { + const typedAction = action as cdbActions.StoreCellOutputSnapshotAction; + state.cellOutputSnapshots.set(typedAction.payload.cellId, typedAction.payload.snapshot); + // TODO Simpler datastructure to instantiate new Map? + return state.set("cellOutputSnapshots", new Map(state.cellOutputSnapshots)); + } + + case cdbActions.STORE_NOTEBOOK_SNAPSHOT: { + const typedAction = action as cdbActions.StoreNotebookSnapshotAction; + // Clear pending request + return state.set("notebookSnapshot", typedAction.payload).set("pendingSnapshotRequest", undefined); + } + + case cdbActions.TAKE_NOTEBOOK_SNAPSHOT: { + const typedAction = action as cdbActions.TakeNotebookSnapshotAction; + // Clear previous snapshots + return state + .set("cellOutputSnapshots", new Map()) + .set("notebookSnapshot", undefined) + .set("notebookSnapshotError", undefined) + .set("pendingSnapshotRequest", typedAction.payload); + } + + case cdbActions.NOTEBOOK_SNAPSHOT_ERROR: { + const typedAction = action as cdbActions.NotebookSnapshotErrorAction; + return state.set("notebookSnapshotError", typedAction.payload.error); } } return state; diff --git a/src/Explorer/Notebook/NotebookComponent/types.ts b/src/Explorer/Notebook/NotebookComponent/types.ts index 5f413f391..785408208 100644 --- a/src/Explorer/Notebook/NotebookComponent/types.ts +++ b/src/Explorer/Notebook/NotebookComponent/types.ts @@ -1,15 +1,40 @@ -import * as Immutable from "immutable"; -import { AppState, ContentRef } from "@nteract/core"; - -import { Notebook } from "../../../Common/Constants"; import { CellId } from "@nteract/commutable"; +import { AppState } from "@nteract/core"; +import * as Immutable from "immutable"; +import { Notebook } from "../../../Common/Constants"; + +export interface SnapshotFragment { + image: HTMLImageElement; + boundingClientRect: DOMRect; + requestId: string; +} + +export type SnapshotRequest = NotebookSnapshotRequest | CellSnapshotRequest; +interface NotebookSnapshotRequestBase { + requestId: string; + aspectRatio: number; + notebookContentRef: string; // notebook redux contentRef + downloadFilename?: string; // Optional: will download as a file +} + +interface NotebookSnapshotRequest extends NotebookSnapshotRequestBase { + type: "notebook"; +} + +interface CellSnapshotRequest extends NotebookSnapshotRequestBase { + type: "celloutput"; + cellId: string; +} export interface CdbRecordProps { databaseAccountName: string | undefined; defaultExperience: string | undefined; kernelRestartDelayMs: number; hoveredCellId: CellId | undefined; - currentNotebookParentElements: Map; + cellOutputSnapshots: Map; + notebookSnapshot?: { imageSrc: string; requestId: string }; + pendingSnapshotRequest?: SnapshotRequest; + notebookSnapshotError?: string; } export type CdbRecord = Immutable.RecordOf; @@ -23,5 +48,8 @@ export const makeCdbRecord = Immutable.Record({ defaultExperience: undefined, kernelRestartDelayMs: Notebook.kernelRestartInitialDelayMs, hoveredCellId: undefined, - currentNotebookParentElements: new Map(), + cellOutputSnapshots: new Map(), + notebookSnapshot: undefined, + pendingSnapshotRequest: undefined, + notebookSnapshotError: undefined, }); diff --git a/src/Explorer/Notebook/NotebookManager.tsx b/src/Explorer/Notebook/NotebookManager.tsx index c97f14566..7be295b51 100644 --- a/src/Explorer/Notebook/NotebookManager.tsx +++ b/src/Explorer/Notebook/NotebookManager.tsx @@ -26,6 +26,7 @@ import { CopyNotebookPane } from "../Panes/CopyNotebookPane/CopyNotebookPane"; import { PublishNotebookPane } from "../Panes/PublishNotebookPane/PublishNotebookPane"; import { ResourceTreeAdapter } from "../Tree/ResourceTreeAdapter"; import { NotebookContentProvider } from "./NotebookComponent/NotebookContentProvider"; +import { SnapshotRequest } from "./NotebookComponent/types"; import { NotebookContainerClient } from "./NotebookContainerClient"; import { NotebookContentClient } from "./NotebookContentClient"; @@ -112,11 +113,13 @@ export default class NotebookManager { public async openPublishNotebookPane( name: string, content: NotebookPaneContent, - parentDomElement: HTMLElement + notebookContentRef: string, + onTakeSnapshot: (request: SnapshotRequest) => void, + onClosePanel: () => void ): Promise { const explorer = this.params.container; explorer.openSidePanel( - "New Collection", + "Publish Notebook", + notebookContentRef={notebookContentRef} + onTakeSnapshot={onTakeSnapshot} + />, + onClosePanel ); } diff --git a/src/Explorer/Notebook/NotebookRenderer/NotebookRenderer.tsx b/src/Explorer/Notebook/NotebookRenderer/NotebookRenderer.tsx index 876cbf337..db3f8d5ad 100644 --- a/src/Explorer/Notebook/NotebookRenderer/NotebookRenderer.tsx +++ b/src/Explorer/Notebook/NotebookRenderer/NotebookRenderer.tsx @@ -1,6 +1,6 @@ import { CellId } from "@nteract/commutable"; import { CellType } from "@nteract/commutable/src"; -import { actions, ContentRef } from "@nteract/core"; +import { actions, ContentRef, selectors } from "@nteract/core"; import { Cells, CodeCell, RawCell } from "@nteract/stateful-components"; import MonacoEditor from "@nteract/stateful-components/lib/inputs/connected-editors/monacoEditor"; import { PassedEditorProps } from "@nteract/stateful-components/lib/inputs/editor"; @@ -12,6 +12,8 @@ import { Dispatch } from "redux"; import { userContext } from "../../../UserContext"; import * as cdbActions from "../NotebookComponent/actions"; import loadTransform from "../NotebookComponent/loadTransform"; +import { CdbAppState, SnapshotFragment, SnapshotRequest } from "../NotebookComponent/types"; +import { NotebookUtil } from "../NotebookUtil"; import { AzureTheme } from "./AzureTheme"; import "./base.css"; import CellCreator from "./decorators/CellCreator"; @@ -32,10 +34,18 @@ export interface NotebookRendererBaseProps { } interface NotebookRendererDispatchProps { - updateNotebookParentDomElt: (contentRef: ContentRef, parentElt: HTMLElement) => void; + storeNotebookSnapshot: (imageSrc: string, requestId: string) => void; + notebookSnapshotError: (error: string) => void; } -type NotebookRendererProps = NotebookRendererBaseProps & NotebookRendererDispatchProps; +interface StateProps { + pendingSnapshotRequest: SnapshotRequest; + cellOutputSnapshots: Map; + notebookSnapshot: { imageSrc: string; requestId: string }; + nbCodeCells: number; +} + +type NotebookRendererProps = NotebookRendererBaseProps & NotebookRendererDispatchProps & StateProps; const decorate = (id: string, contentRef: ContentRef, cell_type: CellType, children: React.ReactNode) => { const Cell = () => ( @@ -60,27 +70,37 @@ const decorate = (id: string, contentRef: ContentRef, cell_type: CellType, child class BaseNotebookRenderer extends React.Component { private notebookRendererRef = React.createRef(); - constructor(props: NotebookRendererProps) { - super(props); - - this.state = { - hoveredCellId: undefined, - }; - } - componentDidMount() { if (!userContext.features.sandboxNotebookOutputs) { loadTransform(this.props as any); } - this.props.updateNotebookParentDomElt(this.props.contentRef, this.notebookRendererRef.current); } - componentDidUpdate() { - this.props.updateNotebookParentDomElt(this.props.contentRef, this.notebookRendererRef.current); - } - - componentWillUnmount() { - this.props.updateNotebookParentDomElt(this.props.contentRef, undefined); + async componentDidUpdate(): Promise { + // Take a snapshot if there's a pending request and all the outputs are also saved + if ( + this.props.pendingSnapshotRequest && + this.props.pendingSnapshotRequest.type === "notebook" && + this.props.pendingSnapshotRequest.notebookContentRef === this.props.contentRef && + (!this.props.notebookSnapshot || + this.props.pendingSnapshotRequest.requestId !== this.props.notebookSnapshot.requestId) && + this.props.cellOutputSnapshots.size === this.props.nbCodeCells + ) { + try { + // Use Html2Canvas because it is much more reliable and fast than dom-to-file + const result = await NotebookUtil.takeScreenshotHtml2Canvas( + this.notebookRendererRef.current, + this.props.pendingSnapshotRequest.aspectRatio, + [...this.props.cellOutputSnapshots.values()], + this.props.pendingSnapshotRequest.downloadFilename + ); + this.props.storeNotebookSnapshot(result.imageSrc, this.props.pendingSnapshotRequest.requestId); + } catch (error) { + this.props.notebookSnapshotError(error.message); + } finally { + this.setState({ processedSnapshotRequest: undefined }); + } + } } render(): JSX.Element { @@ -156,28 +176,40 @@ class BaseNotebookRenderer extends React.Component { } } +export const makeMapStateToProps = ( + initialState: CdbAppState, + ownProps: NotebookRendererProps +): ((state: CdbAppState) => StateProps) => { + const mapStateToProps = (state: CdbAppState): StateProps => { + const { contentRef } = ownProps; + const model = selectors.model(state, { contentRef }); + + let nbCodeCells; + if (model && model.type === "notebook") { + nbCodeCells = NotebookUtil.findCodeCellWithDisplay(model.notebook).length; + } + const { pendingSnapshotRequest, cellOutputSnapshots, notebookSnapshot } = state.cdb; + return { pendingSnapshotRequest, cellOutputSnapshots, notebookSnapshot, nbCodeCells }; + }; + return mapStateToProps; +}; + const makeMapDispatchToProps = (initialDispatch: Dispatch, initialProps: NotebookRendererBaseProps) => { const mapDispatchToProps = (dispatch: Dispatch) => { return { - addTransform: (transform: React.ComponentType & { MIMETYPE: string }) => { - return dispatch( + addTransform: (transform: React.ComponentType & { MIMETYPE: string }) => + dispatch( actions.addTransform({ mediaType: transform.MIMETYPE, component: transform, }) - ); - }, - updateNotebookParentDomElt: (contentRef: ContentRef, parentElt: HTMLElement) => { - return dispatch( - cdbActions.UpdateNotebookParentDomElt({ - contentRef, - parentElt, - }) - ); - }, + ), + storeNotebookSnapshot: (imageSrc: string, requestId: string) => + dispatch(cdbActions.storeNotebookSnapshot({ imageSrc, requestId })), + notebookSnapshotError: (error: string) => dispatch(cdbActions.notebookSnapshotError({ error })), }; }; return mapDispatchToProps; }; -export default connect(null, makeMapDispatchToProps)(BaseNotebookRenderer); +export default connect(makeMapStateToProps, makeMapDispatchToProps)(BaseNotebookRenderer); diff --git a/src/Explorer/Notebook/NotebookRenderer/Toolbar.tsx b/src/Explorer/Notebook/NotebookRenderer/Toolbar.tsx index 3e9a3f41b..08bf3e24c 100644 --- a/src/Explorer/Notebook/NotebookRenderer/Toolbar.tsx +++ b/src/Explorer/Notebook/NotebookRenderer/Toolbar.tsx @@ -1,5 +1,5 @@ import { ContextualMenuItemType, DirectionalHint, IconButton, IContextualMenuItem } from "@fluentui/react"; -import { CellId, CellType } from "@nteract/commutable"; +import { CellId, CellType, ImmutableCodeCell } from "@nteract/commutable"; import { actions, AppState, DocumentRecordProps } from "@nteract/core"; import * as selectors from "@nteract/selectors"; import { CellToolbarContext } from "@nteract/stateful-components"; @@ -10,6 +10,8 @@ import { connect } from "react-redux"; import { Dispatch } from "redux"; import { Action, ActionModifiers } from "../../../Shared/Telemetry/TelemetryConstants"; import * as cdbActions from "../NotebookComponent/actions"; +import { SnapshotRequest } from "../NotebookComponent/types"; +import { NotebookUtil } from "../NotebookUtil"; export interface ComponentProps { contentRef: ContentRef; @@ -26,12 +28,14 @@ interface DispatchProps { clearOutputs: () => void; deleteCell: () => void; traceNotebookTelemetry: (action: Action, actionModifier?: string, data?: any) => void; + takeNotebookSnapshot: (payload: SnapshotRequest) => void; } interface StateProps { cellType: CellType; cellIdAbove: CellId; cellIdBelow: CellId; + hasCodeOutput: boolean; } class BaseToolbar extends React.PureComponent { @@ -58,11 +62,29 @@ class BaseToolbar extends React.PureComponent { + this.props.takeNotebookSnapshot({ + requestId: new Date().getTime().toString(), + aspectRatio: undefined, + type: "celloutput", + cellId: this.props.id, + notebookContentRef: this.props.contentRef, + downloadFilename: `celloutput-${this.props.contentRef}_${this.props.id}.png`, + }); + }, + }); + } + + items.push({ + key: "Divider", + itemType: ContextualMenuItemType.Divider, + }); } items = items.concat([ @@ -183,12 +205,13 @@ const mapDispatchToProps = ( deleteCell: () => dispatch(actions.deleteCell({ id, contentRef })), traceNotebookTelemetry: (action: Action, actionModifier?: string, data?: any) => dispatch(cdbActions.traceNotebookTelemetry({ action, actionModifier, data })), + takeNotebookSnapshot: (request: SnapshotRequest) => dispatch(cdbActions.takeNotebookSnapshot(request)), }); const makeMapStateToProps = (state: AppState, ownProps: ComponentProps): ((state: AppState) => StateProps) => { const mapStateToProps = (state: AppState) => { - const cellType = selectors.cell.cellFromState(state, { id: ownProps.id, contentRef: ownProps.contentRef }) - .cell_type; + const cell = selectors.cell.cellFromState(state, { id: ownProps.id, contentRef: ownProps.contentRef }); + const cellType = cell.cell_type; const model = selectors.model(state, { contentRef: ownProps.contentRef }); const cellOrder = selectors.notebook.cellOrder(model as RecordOf); const cellIndex = cellOrder.indexOf(ownProps.id); @@ -199,6 +222,7 @@ const makeMapStateToProps = (state: AppState, ownProps: ComponentProps): ((state cellType, cellIdAbove, cellIdBelow, + hasCodeOutput: cellType === "code" && NotebookUtil.hasCodeCellOutput(cell as ImmutableCodeCell), }; }; return mapStateToProps; diff --git a/src/Explorer/Notebook/NotebookRenderer/outputs/SandboxOutputs.tsx b/src/Explorer/Notebook/NotebookRenderer/outputs/SandboxOutputs.tsx index a9cf76757..85beea5b5 100644 --- a/src/Explorer/Notebook/NotebookRenderer/outputs/SandboxOutputs.tsx +++ b/src/Explorer/Notebook/NotebookRenderer/outputs/SandboxOutputs.tsx @@ -7,7 +7,9 @@ import postRobot from "post-robot"; import React from "react"; import { connect } from "react-redux"; import { Dispatch } from "redux"; -import { CellOutputViewerProps } from "../../../../CellOutputViewer/CellOutputViewer"; +import { CellOutputViewerProps, SnapshotResponse } from "../../../../CellOutputViewer/CellOutputViewer"; +import * as cdbActions from "../../NotebookComponent/actions"; +import { CdbAppState, SnapshotFragment, SnapshotRequest } from "../../NotebookComponent/types"; // Adapted from https://github.com/nteract/nteract/blob/main/packages/stateful-components/src/outputs/index.tsx // to add support for sandboxing using