From 73bf50e8a8ebb7f3665c04bace5bd6b5adca56ea Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Thu, 15 Feb 2024 11:11:18 +0530 Subject: [PATCH] fix: remove t from getDefaultAppState and allow name to be nullable (#7666) * fix: remove t and allow name to be nullable * pass name as required prop * remove Unnamed * pass name to excalidrawPlus as well for better type safe * render once we have excalidrawAPI to avoid defaulting * rename `getAppName` -> `getName` (temporary) * stop preventing editing filenames when `props.name` supplied * keep `name` as optional param for export functions * keep `appState.name` on `props.name` state separate * fix lint * assertive first * fix lint * Add TODO --------- Co-authored-by: dwelle <5153846+dwelle@users.noreply.github.com> --- excalidraw-app/App.tsx | 46 ++++++++++--------- .../components/ExportToExcalidrawPlus.tsx | 8 ++-- .../excalidraw/actions/actionClipboard.tsx | 2 + packages/excalidraw/actions/actionExport.tsx | 17 ++++--- packages/excalidraw/appState.ts | 4 +- packages/excalidraw/components/App.tsx | 26 +++++------ .../components/ImageExportDialog.tsx | 13 +++--- packages/excalidraw/components/LayerUI.tsx | 1 + .../excalidraw/components/ProjectName.tsx | 27 ++++------- packages/excalidraw/constants.ts | 6 +++ packages/excalidraw/data/index.ts | 12 +++-- packages/excalidraw/data/json.ts | 5 +- packages/excalidraw/data/resave.ts | 3 +- packages/excalidraw/tests/excalidraw.test.tsx | 5 +- packages/excalidraw/types.ts | 7 ++- 15 files changed, 101 insertions(+), 81 deletions(-) diff --git a/excalidraw-app/App.tsx b/excalidraw-app/App.tsx index e38dd7a94..972737b9d 100644 --- a/excalidraw-app/App.tsx +++ b/excalidraw-app/App.tsx @@ -709,27 +709,30 @@ const ExcalidrawWrapper = () => { toggleTheme: true, export: { onExportToBackend, - renderCustomUI: (elements, appState, files) => { - return ( - { - excalidrawAPI?.updateScene({ - appState: { - errorMessage: error.message, - }, - }); - }} - onSuccess={() => { - excalidrawAPI?.updateScene({ - appState: { openDialog: null }, - }); - }} - /> - ); - }, + renderCustomUI: excalidrawAPI + ? (elements, appState, files) => { + return ( + { + excalidrawAPI?.updateScene({ + appState: { + errorMessage: error.message, + }, + }); + }} + onSuccess={() => { + excalidrawAPI.updateScene({ + appState: { openDialog: null }, + }); + }} + /> + ); + } + : undefined, }, }, }} @@ -775,6 +778,7 @@ const ExcalidrawWrapper = () => { excalidrawAPI.getSceneElements(), excalidrawAPI.getAppState(), excalidrawAPI.getFiles(), + excalidrawAPI.getName(), ); }} > diff --git a/excalidraw-app/components/ExportToExcalidrawPlus.tsx b/excalidraw-app/components/ExportToExcalidrawPlus.tsx index 4c566950b..bfbb4a556 100644 --- a/excalidraw-app/components/ExportToExcalidrawPlus.tsx +++ b/excalidraw-app/components/ExportToExcalidrawPlus.tsx @@ -30,6 +30,7 @@ export const exportToExcalidrawPlus = async ( elements: readonly NonDeletedExcalidrawElement[], appState: Partial, files: BinaryFiles, + name: string, ) => { const firebase = await loadFirebaseStorage(); @@ -53,7 +54,7 @@ export const exportToExcalidrawPlus = async ( .ref(`/migrations/scenes/${id}`) .put(blob, { customMetadata: { - data: JSON.stringify({ version: 2, name: appState.name }), + data: JSON.stringify({ version: 2, name }), created: Date.now().toString(), }, }); @@ -89,9 +90,10 @@ export const ExportToExcalidrawPlus: React.FC<{ elements: readonly NonDeletedExcalidrawElement[]; appState: Partial; files: BinaryFiles; + name: string; onError: (error: Error) => void; onSuccess: () => void; -}> = ({ elements, appState, files, onError, onSuccess }) => { +}> = ({ elements, appState, files, name, onError, onSuccess }) => { const { t } = useI18n(); return ( @@ -117,7 +119,7 @@ export const ExportToExcalidrawPlus: React.FC<{ onClick={async () => { try { trackEvent("export", "eplus", `ui (${getFrame()})`); - await exportToExcalidrawPlus(elements, appState, files); + await exportToExcalidrawPlus(elements, appState, files, name); onSuccess(); } catch (error: any) { console.error(error); diff --git a/packages/excalidraw/actions/actionClipboard.tsx b/packages/excalidraw/actions/actionClipboard.tsx index dadc61013..b2457341d 100644 --- a/packages/excalidraw/actions/actionClipboard.tsx +++ b/packages/excalidraw/actions/actionClipboard.tsx @@ -138,6 +138,7 @@ export const actionCopyAsSvg = register({ { ...appState, exportingFrame, + name: app.getName(), }, ); return { @@ -184,6 +185,7 @@ export const actionCopyAsPng = register({ await exportCanvas("clipboard", exportedElements, appState, app.files, { ...appState, exportingFrame, + name: app.getName(), }); return { appState: { diff --git a/packages/excalidraw/actions/actionExport.tsx b/packages/excalidraw/actions/actionExport.tsx index 74dff34c8..7cb6b1291 100644 --- a/packages/excalidraw/actions/actionExport.tsx +++ b/packages/excalidraw/actions/actionExport.tsx @@ -26,14 +26,11 @@ export const actionChangeProjectName = register({ perform: (_elements, appState, value) => { return { appState: { ...appState, name: value }, commitToHistory: false }; }, - PanelComponent: ({ appState, updateData, appProps, data }) => ( + PanelComponent: ({ appState, updateData, appProps, data, app }) => ( updateData(name)} - isNameEditable={ - typeof appProps.name === "undefined" && !appState.viewModeEnabled - } ignoreFocus={data?.ignoreFocus ?? false} /> ), @@ -144,8 +141,13 @@ export const actionSaveToActiveFile = register({ try { const { fileHandle } = isImageFileHandle(appState.fileHandle) - ? await resaveAsImageWithScene(elements, appState, app.files) - : await saveAsJSON(elements, appState, app.files); + ? await resaveAsImageWithScene( + elements, + appState, + app.files, + app.getName(), + ) + : await saveAsJSON(elements, appState, app.files, app.getName()); return { commitToHistory: false, @@ -190,6 +192,7 @@ export const actionSaveFileToDisk = register({ fileHandle: null, }, app.files, + app.getName(), ); return { commitToHistory: false, diff --git a/packages/excalidraw/appState.ts b/packages/excalidraw/appState.ts index 4dec9a790..a0ab233c9 100644 --- a/packages/excalidraw/appState.ts +++ b/packages/excalidraw/appState.ts @@ -7,9 +7,7 @@ import { EXPORT_SCALES, THEME, } from "./constants"; -import { t } from "./i18n"; import { AppState, NormalizedZoomValue } from "./types"; -import { getDateTime } from "./utils"; const defaultExportScale = EXPORT_SCALES.includes(devicePixelRatio) ? devicePixelRatio @@ -65,7 +63,7 @@ export const getDefaultAppState = (): Omit< isRotating: false, lastPointerDownWith: "mouse", multiElement: null, - name: `${t("labels.untitled")}-${getDateTime()}`, + name: null, contextMenu: null, openMenu: null, openPopup: null, diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 7b9cc8cc2..3d3838afc 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -270,6 +270,7 @@ import { updateStable, addEventListener, normalizeEOL, + getDateTime, } from "../utils"; import { createSrcDoc, @@ -619,7 +620,7 @@ class App extends React.Component { gridModeEnabled = false, objectsSnapModeEnabled = false, theme = defaultAppState.theme, - name = defaultAppState.name, + name = `${t("labels.untitled")}-${getDateTime()}`, } = props; this.state = { ...defaultAppState, @@ -662,6 +663,7 @@ class App extends React.Component { getSceneElements: this.getSceneElements, getAppState: () => this.state, getFiles: () => this.files, + getName: this.getName, registerAction: (action: Action) => { this.actionManager.registerAction(action); }, @@ -1734,7 +1736,7 @@ class App extends React.Component { this.files, { exportBackground: this.state.exportBackground, - name: this.state.name, + name: this.getName(), viewBackgroundColor: this.state.viewBackgroundColor, exportingFrame: opts.exportingFrame, }, @@ -2133,7 +2135,7 @@ class App extends React.Component { let gridSize = actionResult?.appState?.gridSize || null; const theme = actionResult?.appState?.theme || this.props.theme || THEME.LIGHT; - let name = actionResult?.appState?.name ?? this.state.name; + const name = actionResult?.appState?.name ?? this.state.name; const errorMessage = actionResult?.appState?.errorMessage ?? this.state.errorMessage; if (typeof this.props.viewModeEnabled !== "undefined") { @@ -2148,10 +2150,6 @@ class App extends React.Component { gridSize = this.props.gridModeEnabled ? GRID_SIZE : null; } - if (typeof this.props.name !== "undefined") { - name = this.props.name; - } - editingElement = editingElement || actionResult.appState?.editingElement || null; @@ -2709,12 +2707,6 @@ class App extends React.Component { }); } - if (this.props.name && prevProps.name !== this.props.name) { - this.setState({ - name: this.props.name, - }); - } - this.excalidrawContainerRef.current?.classList.toggle( "theme--dark", this.state.theme === "dark", @@ -4122,6 +4114,14 @@ class App extends React.Component { return gesture.pointers.size >= 2; }; + public getName = () => { + return ( + this.state.name || + this.props.name || + `${t("labels.untitled")}-${getDateTime()}` + ); + }; + // fires only on Safari private onGestureStart = withBatchedUpdates((event: GestureEvent) => { event.preventDefault(); diff --git a/packages/excalidraw/components/ImageExportDialog.tsx b/packages/excalidraw/components/ImageExportDialog.tsx index 7ca54e985..cecdfa79a 100644 --- a/packages/excalidraw/components/ImageExportDialog.tsx +++ b/packages/excalidraw/components/ImageExportDialog.tsx @@ -32,7 +32,6 @@ import { Switch } from "./Switch"; import { Tooltip } from "./Tooltip"; import "./ImageExportDialog.scss"; -import { useAppProps } from "./App"; import { FilledButton } from "./FilledButton"; import { cloneJSON } from "../utils"; import { prepareElementsForExport } from "../data"; @@ -58,6 +57,7 @@ type ImageExportModalProps = { files: BinaryFiles; actionManager: ActionManager; onExportImage: AppClassProperties["onExportImage"]; + name: string; }; const ImageExportModal = ({ @@ -66,14 +66,14 @@ const ImageExportModal = ({ files, actionManager, onExportImage, + name, }: ImageExportModalProps) => { const hasSelection = isSomeElementSelected( elementsSnapshot, appStateSnapshot, ); - const appProps = useAppProps(); - const [projectName, setProjectName] = useState(appStateSnapshot.name); + const [projectName, setProjectName] = useState(name); const [exportSelectionOnly, setExportSelectionOnly] = useState(hasSelection); const [exportWithBackground, setExportWithBackground] = useState( appStateSnapshot.exportBackground, @@ -158,10 +158,6 @@ const ImageExportModal = ({ className="TextInput" value={projectName} style={{ width: "30ch" }} - disabled={ - typeof appProps.name !== "undefined" || - appStateSnapshot.viewModeEnabled - } onChange={(event) => { setProjectName(event.target.value); actionManager.executeAction( @@ -347,6 +343,7 @@ export const ImageExportDialog = ({ actionManager, onExportImage, onCloseRequest, + name, }: { appState: UIAppState; elements: readonly NonDeletedExcalidrawElement[]; @@ -354,6 +351,7 @@ export const ImageExportDialog = ({ actionManager: ActionManager; onExportImage: AppClassProperties["onExportImage"]; onCloseRequest: () => void; + name: string; }) => { // we need to take a snapshot so that the exported state can't be modified // while the dialog is open @@ -372,6 +370,7 @@ export const ImageExportDialog = ({ files={files} actionManager={actionManager} onExportImage={onExportImage} + name={name} /> ); diff --git a/packages/excalidraw/components/LayerUI.tsx b/packages/excalidraw/components/LayerUI.tsx index 7247e8bf1..eb8027138 100644 --- a/packages/excalidraw/components/LayerUI.tsx +++ b/packages/excalidraw/components/LayerUI.tsx @@ -195,6 +195,7 @@ const LayerUI = ({ actionManager={actionManager} onExportImage={onExportImage} onCloseRequest={() => setAppState({ openDialog: null })} + name={app.getName()} /> ); }; diff --git a/packages/excalidraw/components/ProjectName.tsx b/packages/excalidraw/components/ProjectName.tsx index 69ff33527..592961793 100644 --- a/packages/excalidraw/components/ProjectName.tsx +++ b/packages/excalidraw/components/ProjectName.tsx @@ -11,7 +11,6 @@ type Props = { value: string; onChange: (value: string) => void; label: string; - isNameEditable: boolean; ignoreFocus?: boolean; }; @@ -42,23 +41,17 @@ export const ProjectName = (props: Props) => { return (
- {props.isNameEditable ? ( - setFileName(event.target.value)} - /> - ) : ( - - {props.value} - - )} + setFileName(event.target.value)} + />
); }; diff --git a/packages/excalidraw/constants.ts b/packages/excalidraw/constants.ts index 021c706a9..09e497564 100644 --- a/packages/excalidraw/constants.ts +++ b/packages/excalidraw/constants.ts @@ -381,3 +381,9 @@ export const EDITOR_LS_KEYS = { MERMAID_TO_EXCALIDRAW: "mermaid-to-excalidraw", PUBLISH_LIBRARY: "publish-library-data", } as const; + +/** + * not translated as this is used only in public, stateless API as default value + * where filename is optional and we can't retrieve name from app state + */ +export const DEFAULT_FILENAME = "Untitled"; diff --git a/packages/excalidraw/data/index.ts b/packages/excalidraw/data/index.ts index fa2ec9de6..51446921f 100644 --- a/packages/excalidraw/data/index.ts +++ b/packages/excalidraw/data/index.ts @@ -2,7 +2,12 @@ import { copyBlobToClipboardAsPng, copyTextToSystemClipboard, } from "../clipboard"; -import { DEFAULT_EXPORT_PADDING, isFirefox, MIME_TYPES } from "../constants"; +import { + DEFAULT_EXPORT_PADDING, + DEFAULT_FILENAME, + isFirefox, + MIME_TYPES, +} from "../constants"; import { getNonDeletedElements } from "../element"; import { isFrameLikeElement } from "../element/typeChecks"; import { @@ -84,14 +89,15 @@ export const exportCanvas = async ( exportBackground, exportPadding = DEFAULT_EXPORT_PADDING, viewBackgroundColor, - name, + name = appState.name || DEFAULT_FILENAME, fileHandle = null, exportingFrame = null, }: { exportBackground: boolean; exportPadding?: number; viewBackgroundColor: string; - name: string; + /** filename, if applicable */ + name?: string; fileHandle?: FileSystemHandle | null; exportingFrame: ExcalidrawFrameLikeElement | null; }, diff --git a/packages/excalidraw/data/json.ts b/packages/excalidraw/data/json.ts index 037c5ca18..94dddf288 100644 --- a/packages/excalidraw/data/json.ts +++ b/packages/excalidraw/data/json.ts @@ -1,6 +1,7 @@ import { fileOpen, fileSave } from "./filesystem"; import { cleanAppStateForExport, clearAppStateForDatabase } from "../appState"; import { + DEFAULT_FILENAME, EXPORT_DATA_TYPES, EXPORT_SOURCE, MIME_TYPES, @@ -71,6 +72,8 @@ export const saveAsJSON = async ( elements: readonly ExcalidrawElement[], appState: AppState, files: BinaryFiles, + /** filename */ + name: string = appState.name || DEFAULT_FILENAME, ) => { const serialized = serializeAsJSON(elements, appState, files, "local"); const blob = new Blob([serialized], { @@ -78,7 +81,7 @@ export const saveAsJSON = async ( }); const fileHandle = await fileSave(blob, { - name: appState.name, + name, extension: "excalidraw", description: "Excalidraw file", fileHandle: isImageFileHandle(appState.fileHandle) diff --git a/packages/excalidraw/data/resave.ts b/packages/excalidraw/data/resave.ts index 0998fd3c7..c73890e22 100644 --- a/packages/excalidraw/data/resave.ts +++ b/packages/excalidraw/data/resave.ts @@ -7,8 +7,9 @@ export const resaveAsImageWithScene = async ( elements: readonly ExcalidrawElement[], appState: AppState, files: BinaryFiles, + name: string, ) => { - const { exportBackground, viewBackgroundColor, name, fileHandle } = appState; + const { exportBackground, viewBackgroundColor, fileHandle } = appState; const fileHandleType = getFileHandleType(fileHandle); diff --git a/packages/excalidraw/tests/excalidraw.test.tsx b/packages/excalidraw/tests/excalidraw.test.tsx index 962dabcc0..98d0e9006 100644 --- a/packages/excalidraw/tests/excalidraw.test.tsx +++ b/packages/excalidraw/tests/excalidraw.test.tsx @@ -303,7 +303,7 @@ describe("", () => { }); describe("Test name prop", () => { - it('should allow editing name when the name prop is "undefined"', async () => { + it("should allow editing name", async () => { const { container } = await render(); //open menu toggleMenu(container); @@ -315,7 +315,7 @@ describe("", () => { expect(textInput?.nodeName).toBe("INPUT"); }); - it('should set the name and not allow editing when the name prop is present"', async () => { + it('should set the name when the name prop is present"', async () => { const name = "test"; const { container } = await render(); //open menu @@ -326,7 +326,6 @@ describe("", () => { ) as HTMLInputElement; expect(textInput?.value).toEqual(name); expect(textInput?.nodeName).toBe("INPUT"); - expect(textInput?.disabled).toBe(true); }); }); diff --git a/packages/excalidraw/types.ts b/packages/excalidraw/types.ts index 89b121b2f..1eaa04449 100644 --- a/packages/excalidraw/types.ts +++ b/packages/excalidraw/types.ts @@ -247,7 +247,7 @@ export interface AppState { scrollY: number; cursorButton: "up" | "down"; scrolledOutside: boolean; - name: string; + name: string | null; isResizing: boolean; isRotating: boolean; zoom: Zoom; @@ -435,6 +435,7 @@ export interface ExcalidrawProps { objectsSnapModeEnabled?: boolean; libraryReturnUrl?: string; theme?: Theme; + // @TODO come with better API before v0.18.0 name?: string; renderCustomStats?: ( elements: readonly NonDeletedExcalidrawElement[], @@ -577,6 +578,7 @@ export type AppClassProperties = { setOpenDialog: App["setOpenDialog"]; insertEmbeddableElement: App["insertEmbeddableElement"]; onMagicframeToolSelect: App["onMagicframeToolSelect"]; + getName: App["getName"]; }; export type PointerDownState = Readonly<{ @@ -651,10 +653,11 @@ export type ExcalidrawImperativeAPI = { history: { clear: InstanceType["resetHistory"]; }; - scrollToContent: InstanceType["scrollToContent"]; getSceneElements: InstanceType["getSceneElements"]; getAppState: () => InstanceType["state"]; getFiles: () => InstanceType["files"]; + getName: InstanceType["getName"]; + scrollToContent: InstanceType["scrollToContent"]; registerAction: (action: Action) => void; refresh: InstanceType["refresh"]; setToast: InstanceType["setToast"];