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>
This commit is contained in:
Aakansha Doshi 2024-02-15 11:11:18 +05:30 committed by GitHub
parent 48c3465b19
commit 73bf50e8a8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 101 additions and 81 deletions

View file

@ -709,27 +709,30 @@ const ExcalidrawWrapper = () => {
toggleTheme: true, toggleTheme: true,
export: { export: {
onExportToBackend, onExportToBackend,
renderCustomUI: (elements, appState, files) => { renderCustomUI: excalidrawAPI
return ( ? (elements, appState, files) => {
<ExportToExcalidrawPlus return (
elements={elements} <ExportToExcalidrawPlus
appState={appState} elements={elements}
files={files} appState={appState}
onError={(error) => { files={files}
excalidrawAPI?.updateScene({ name={excalidrawAPI.getName()}
appState: { onError={(error) => {
errorMessage: error.message, excalidrawAPI?.updateScene({
}, appState: {
}); errorMessage: error.message,
}} },
onSuccess={() => { });
excalidrawAPI?.updateScene({ }}
appState: { openDialog: null }, onSuccess={() => {
}); excalidrawAPI.updateScene({
}} appState: { openDialog: null },
/> });
); }}
}, />
);
}
: undefined,
}, },
}, },
}} }}
@ -775,6 +778,7 @@ const ExcalidrawWrapper = () => {
excalidrawAPI.getSceneElements(), excalidrawAPI.getSceneElements(),
excalidrawAPI.getAppState(), excalidrawAPI.getAppState(),
excalidrawAPI.getFiles(), excalidrawAPI.getFiles(),
excalidrawAPI.getName(),
); );
}} }}
> >

View file

@ -30,6 +30,7 @@ export const exportToExcalidrawPlus = async (
elements: readonly NonDeletedExcalidrawElement[], elements: readonly NonDeletedExcalidrawElement[],
appState: Partial<AppState>, appState: Partial<AppState>,
files: BinaryFiles, files: BinaryFiles,
name: string,
) => { ) => {
const firebase = await loadFirebaseStorage(); const firebase = await loadFirebaseStorage();
@ -53,7 +54,7 @@ export const exportToExcalidrawPlus = async (
.ref(`/migrations/scenes/${id}`) .ref(`/migrations/scenes/${id}`)
.put(blob, { .put(blob, {
customMetadata: { customMetadata: {
data: JSON.stringify({ version: 2, name: appState.name }), data: JSON.stringify({ version: 2, name }),
created: Date.now().toString(), created: Date.now().toString(),
}, },
}); });
@ -89,9 +90,10 @@ export const ExportToExcalidrawPlus: React.FC<{
elements: readonly NonDeletedExcalidrawElement[]; elements: readonly NonDeletedExcalidrawElement[];
appState: Partial<AppState>; appState: Partial<AppState>;
files: BinaryFiles; files: BinaryFiles;
name: string;
onError: (error: Error) => void; onError: (error: Error) => void;
onSuccess: () => void; onSuccess: () => void;
}> = ({ elements, appState, files, onError, onSuccess }) => { }> = ({ elements, appState, files, name, onError, onSuccess }) => {
const { t } = useI18n(); const { t } = useI18n();
return ( return (
<Card color="primary"> <Card color="primary">
@ -117,7 +119,7 @@ export const ExportToExcalidrawPlus: React.FC<{
onClick={async () => { onClick={async () => {
try { try {
trackEvent("export", "eplus", `ui (${getFrame()})`); trackEvent("export", "eplus", `ui (${getFrame()})`);
await exportToExcalidrawPlus(elements, appState, files); await exportToExcalidrawPlus(elements, appState, files, name);
onSuccess(); onSuccess();
} catch (error: any) { } catch (error: any) {
console.error(error); console.error(error);

View file

@ -138,6 +138,7 @@ export const actionCopyAsSvg = register({
{ {
...appState, ...appState,
exportingFrame, exportingFrame,
name: app.getName(),
}, },
); );
return { return {
@ -184,6 +185,7 @@ export const actionCopyAsPng = register({
await exportCanvas("clipboard", exportedElements, appState, app.files, { await exportCanvas("clipboard", exportedElements, appState, app.files, {
...appState, ...appState,
exportingFrame, exportingFrame,
name: app.getName(),
}); });
return { return {
appState: { appState: {

View file

@ -26,14 +26,11 @@ export const actionChangeProjectName = register({
perform: (_elements, appState, value) => { perform: (_elements, appState, value) => {
return { appState: { ...appState, name: value }, commitToHistory: false }; return { appState: { ...appState, name: value }, commitToHistory: false };
}, },
PanelComponent: ({ appState, updateData, appProps, data }) => ( PanelComponent: ({ appState, updateData, appProps, data, app }) => (
<ProjectName <ProjectName
label={t("labels.fileTitle")} label={t("labels.fileTitle")}
value={appState.name || "Unnamed"} value={app.getName()}
onChange={(name: string) => updateData(name)} onChange={(name: string) => updateData(name)}
isNameEditable={
typeof appProps.name === "undefined" && !appState.viewModeEnabled
}
ignoreFocus={data?.ignoreFocus ?? false} ignoreFocus={data?.ignoreFocus ?? false}
/> />
), ),
@ -144,8 +141,13 @@ export const actionSaveToActiveFile = register({
try { try {
const { fileHandle } = isImageFileHandle(appState.fileHandle) const { fileHandle } = isImageFileHandle(appState.fileHandle)
? await resaveAsImageWithScene(elements, appState, app.files) ? await resaveAsImageWithScene(
: await saveAsJSON(elements, appState, app.files); elements,
appState,
app.files,
app.getName(),
)
: await saveAsJSON(elements, appState, app.files, app.getName());
return { return {
commitToHistory: false, commitToHistory: false,
@ -190,6 +192,7 @@ export const actionSaveFileToDisk = register({
fileHandle: null, fileHandle: null,
}, },
app.files, app.files,
app.getName(),
); );
return { return {
commitToHistory: false, commitToHistory: false,

View file

@ -7,9 +7,7 @@ import {
EXPORT_SCALES, EXPORT_SCALES,
THEME, THEME,
} from "./constants"; } from "./constants";
import { t } from "./i18n";
import { AppState, NormalizedZoomValue } from "./types"; import { AppState, NormalizedZoomValue } from "./types";
import { getDateTime } from "./utils";
const defaultExportScale = EXPORT_SCALES.includes(devicePixelRatio) const defaultExportScale = EXPORT_SCALES.includes(devicePixelRatio)
? devicePixelRatio ? devicePixelRatio
@ -65,7 +63,7 @@ export const getDefaultAppState = (): Omit<
isRotating: false, isRotating: false,
lastPointerDownWith: "mouse", lastPointerDownWith: "mouse",
multiElement: null, multiElement: null,
name: `${t("labels.untitled")}-${getDateTime()}`, name: null,
contextMenu: null, contextMenu: null,
openMenu: null, openMenu: null,
openPopup: null, openPopup: null,

View file

@ -270,6 +270,7 @@ import {
updateStable, updateStable,
addEventListener, addEventListener,
normalizeEOL, normalizeEOL,
getDateTime,
} from "../utils"; } from "../utils";
import { import {
createSrcDoc, createSrcDoc,
@ -619,7 +620,7 @@ class App extends React.Component<AppProps, AppState> {
gridModeEnabled = false, gridModeEnabled = false,
objectsSnapModeEnabled = false, objectsSnapModeEnabled = false,
theme = defaultAppState.theme, theme = defaultAppState.theme,
name = defaultAppState.name, name = `${t("labels.untitled")}-${getDateTime()}`,
} = props; } = props;
this.state = { this.state = {
...defaultAppState, ...defaultAppState,
@ -662,6 +663,7 @@ class App extends React.Component<AppProps, AppState> {
getSceneElements: this.getSceneElements, getSceneElements: this.getSceneElements,
getAppState: () => this.state, getAppState: () => this.state,
getFiles: () => this.files, getFiles: () => this.files,
getName: this.getName,
registerAction: (action: Action) => { registerAction: (action: Action) => {
this.actionManager.registerAction(action); this.actionManager.registerAction(action);
}, },
@ -1734,7 +1736,7 @@ class App extends React.Component<AppProps, AppState> {
this.files, this.files,
{ {
exportBackground: this.state.exportBackground, exportBackground: this.state.exportBackground,
name: this.state.name, name: this.getName(),
viewBackgroundColor: this.state.viewBackgroundColor, viewBackgroundColor: this.state.viewBackgroundColor,
exportingFrame: opts.exportingFrame, exportingFrame: opts.exportingFrame,
}, },
@ -2133,7 +2135,7 @@ class App extends React.Component<AppProps, AppState> {
let gridSize = actionResult?.appState?.gridSize || null; let gridSize = actionResult?.appState?.gridSize || null;
const theme = const theme =
actionResult?.appState?.theme || this.props.theme || THEME.LIGHT; 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 = const errorMessage =
actionResult?.appState?.errorMessage ?? this.state.errorMessage; actionResult?.appState?.errorMessage ?? this.state.errorMessage;
if (typeof this.props.viewModeEnabled !== "undefined") { if (typeof this.props.viewModeEnabled !== "undefined") {
@ -2148,10 +2150,6 @@ class App extends React.Component<AppProps, AppState> {
gridSize = this.props.gridModeEnabled ? GRID_SIZE : null; gridSize = this.props.gridModeEnabled ? GRID_SIZE : null;
} }
if (typeof this.props.name !== "undefined") {
name = this.props.name;
}
editingElement = editingElement =
editingElement || actionResult.appState?.editingElement || null; editingElement || actionResult.appState?.editingElement || null;
@ -2709,12 +2707,6 @@ class App extends React.Component<AppProps, AppState> {
}); });
} }
if (this.props.name && prevProps.name !== this.props.name) {
this.setState({
name: this.props.name,
});
}
this.excalidrawContainerRef.current?.classList.toggle( this.excalidrawContainerRef.current?.classList.toggle(
"theme--dark", "theme--dark",
this.state.theme === "dark", this.state.theme === "dark",
@ -4122,6 +4114,14 @@ class App extends React.Component<AppProps, AppState> {
return gesture.pointers.size >= 2; return gesture.pointers.size >= 2;
}; };
public getName = () => {
return (
this.state.name ||
this.props.name ||
`${t("labels.untitled")}-${getDateTime()}`
);
};
// fires only on Safari // fires only on Safari
private onGestureStart = withBatchedUpdates((event: GestureEvent) => { private onGestureStart = withBatchedUpdates((event: GestureEvent) => {
event.preventDefault(); event.preventDefault();

View file

@ -32,7 +32,6 @@ import { Switch } from "./Switch";
import { Tooltip } from "./Tooltip"; import { Tooltip } from "./Tooltip";
import "./ImageExportDialog.scss"; import "./ImageExportDialog.scss";
import { useAppProps } from "./App";
import { FilledButton } from "./FilledButton"; import { FilledButton } from "./FilledButton";
import { cloneJSON } from "../utils"; import { cloneJSON } from "../utils";
import { prepareElementsForExport } from "../data"; import { prepareElementsForExport } from "../data";
@ -58,6 +57,7 @@ type ImageExportModalProps = {
files: BinaryFiles; files: BinaryFiles;
actionManager: ActionManager; actionManager: ActionManager;
onExportImage: AppClassProperties["onExportImage"]; onExportImage: AppClassProperties["onExportImage"];
name: string;
}; };
const ImageExportModal = ({ const ImageExportModal = ({
@ -66,14 +66,14 @@ const ImageExportModal = ({
files, files,
actionManager, actionManager,
onExportImage, onExportImage,
name,
}: ImageExportModalProps) => { }: ImageExportModalProps) => {
const hasSelection = isSomeElementSelected( const hasSelection = isSomeElementSelected(
elementsSnapshot, elementsSnapshot,
appStateSnapshot, appStateSnapshot,
); );
const appProps = useAppProps(); const [projectName, setProjectName] = useState(name);
const [projectName, setProjectName] = useState(appStateSnapshot.name);
const [exportSelectionOnly, setExportSelectionOnly] = useState(hasSelection); const [exportSelectionOnly, setExportSelectionOnly] = useState(hasSelection);
const [exportWithBackground, setExportWithBackground] = useState( const [exportWithBackground, setExportWithBackground] = useState(
appStateSnapshot.exportBackground, appStateSnapshot.exportBackground,
@ -158,10 +158,6 @@ const ImageExportModal = ({
className="TextInput" className="TextInput"
value={projectName} value={projectName}
style={{ width: "30ch" }} style={{ width: "30ch" }}
disabled={
typeof appProps.name !== "undefined" ||
appStateSnapshot.viewModeEnabled
}
onChange={(event) => { onChange={(event) => {
setProjectName(event.target.value); setProjectName(event.target.value);
actionManager.executeAction( actionManager.executeAction(
@ -347,6 +343,7 @@ export const ImageExportDialog = ({
actionManager, actionManager,
onExportImage, onExportImage,
onCloseRequest, onCloseRequest,
name,
}: { }: {
appState: UIAppState; appState: UIAppState;
elements: readonly NonDeletedExcalidrawElement[]; elements: readonly NonDeletedExcalidrawElement[];
@ -354,6 +351,7 @@ export const ImageExportDialog = ({
actionManager: ActionManager; actionManager: ActionManager;
onExportImage: AppClassProperties["onExportImage"]; onExportImage: AppClassProperties["onExportImage"];
onCloseRequest: () => void; onCloseRequest: () => void;
name: string;
}) => { }) => {
// we need to take a snapshot so that the exported state can't be modified // we need to take a snapshot so that the exported state can't be modified
// while the dialog is open // while the dialog is open
@ -372,6 +370,7 @@ export const ImageExportDialog = ({
files={files} files={files}
actionManager={actionManager} actionManager={actionManager}
onExportImage={onExportImage} onExportImage={onExportImage}
name={name}
/> />
</Dialog> </Dialog>
); );

View file

@ -195,6 +195,7 @@ const LayerUI = ({
actionManager={actionManager} actionManager={actionManager}
onExportImage={onExportImage} onExportImage={onExportImage}
onCloseRequest={() => setAppState({ openDialog: null })} onCloseRequest={() => setAppState({ openDialog: null })}
name={app.getName()}
/> />
); );
}; };

View file

@ -11,7 +11,6 @@ type Props = {
value: string; value: string;
onChange: (value: string) => void; onChange: (value: string) => void;
label: string; label: string;
isNameEditable: boolean;
ignoreFocus?: boolean; ignoreFocus?: boolean;
}; };
@ -42,23 +41,17 @@ export const ProjectName = (props: Props) => {
return ( return (
<div className="ProjectName"> <div className="ProjectName">
<label className="ProjectName-label" htmlFor="filename"> <label className="ProjectName-label" htmlFor="filename">
{`${props.label}${props.isNameEditable ? "" : ":"}`} {`${props.label}:`}
</label> </label>
{props.isNameEditable ? ( <input
<input type="text"
type="text" className="TextInput"
className="TextInput" onBlur={handleBlur}
onBlur={handleBlur} onKeyDown={handleKeyDown}
onKeyDown={handleKeyDown} id={`${id}-filename`}
id={`${id}-filename`} value={fileName}
value={fileName} onChange={(event) => setFileName(event.target.value)}
onChange={(event) => setFileName(event.target.value)} />
/>
) : (
<span className="TextInput TextInput--readonly" id={`${id}-filename`}>
{props.value}
</span>
)}
</div> </div>
); );
}; };

View file

@ -381,3 +381,9 @@ export const EDITOR_LS_KEYS = {
MERMAID_TO_EXCALIDRAW: "mermaid-to-excalidraw", MERMAID_TO_EXCALIDRAW: "mermaid-to-excalidraw",
PUBLISH_LIBRARY: "publish-library-data", PUBLISH_LIBRARY: "publish-library-data",
} as const; } 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";

View file

@ -2,7 +2,12 @@ import {
copyBlobToClipboardAsPng, copyBlobToClipboardAsPng,
copyTextToSystemClipboard, copyTextToSystemClipboard,
} from "../clipboard"; } 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 { getNonDeletedElements } from "../element";
import { isFrameLikeElement } from "../element/typeChecks"; import { isFrameLikeElement } from "../element/typeChecks";
import { import {
@ -84,14 +89,15 @@ export const exportCanvas = async (
exportBackground, exportBackground,
exportPadding = DEFAULT_EXPORT_PADDING, exportPadding = DEFAULT_EXPORT_PADDING,
viewBackgroundColor, viewBackgroundColor,
name, name = appState.name || DEFAULT_FILENAME,
fileHandle = null, fileHandle = null,
exportingFrame = null, exportingFrame = null,
}: { }: {
exportBackground: boolean; exportBackground: boolean;
exportPadding?: number; exportPadding?: number;
viewBackgroundColor: string; viewBackgroundColor: string;
name: string; /** filename, if applicable */
name?: string;
fileHandle?: FileSystemHandle | null; fileHandle?: FileSystemHandle | null;
exportingFrame: ExcalidrawFrameLikeElement | null; exportingFrame: ExcalidrawFrameLikeElement | null;
}, },

View file

@ -1,6 +1,7 @@
import { fileOpen, fileSave } from "./filesystem"; import { fileOpen, fileSave } from "./filesystem";
import { cleanAppStateForExport, clearAppStateForDatabase } from "../appState"; import { cleanAppStateForExport, clearAppStateForDatabase } from "../appState";
import { import {
DEFAULT_FILENAME,
EXPORT_DATA_TYPES, EXPORT_DATA_TYPES,
EXPORT_SOURCE, EXPORT_SOURCE,
MIME_TYPES, MIME_TYPES,
@ -71,6 +72,8 @@ export const saveAsJSON = async (
elements: readonly ExcalidrawElement[], elements: readonly ExcalidrawElement[],
appState: AppState, appState: AppState,
files: BinaryFiles, files: BinaryFiles,
/** filename */
name: string = appState.name || DEFAULT_FILENAME,
) => { ) => {
const serialized = serializeAsJSON(elements, appState, files, "local"); const serialized = serializeAsJSON(elements, appState, files, "local");
const blob = new Blob([serialized], { const blob = new Blob([serialized], {
@ -78,7 +81,7 @@ export const saveAsJSON = async (
}); });
const fileHandle = await fileSave(blob, { const fileHandle = await fileSave(blob, {
name: appState.name, name,
extension: "excalidraw", extension: "excalidraw",
description: "Excalidraw file", description: "Excalidraw file",
fileHandle: isImageFileHandle(appState.fileHandle) fileHandle: isImageFileHandle(appState.fileHandle)

View file

@ -7,8 +7,9 @@ export const resaveAsImageWithScene = async (
elements: readonly ExcalidrawElement[], elements: readonly ExcalidrawElement[],
appState: AppState, appState: AppState,
files: BinaryFiles, files: BinaryFiles,
name: string,
) => { ) => {
const { exportBackground, viewBackgroundColor, name, fileHandle } = appState; const { exportBackground, viewBackgroundColor, fileHandle } = appState;
const fileHandleType = getFileHandleType(fileHandle); const fileHandleType = getFileHandleType(fileHandle);

View file

@ -303,7 +303,7 @@ describe("<Excalidraw/>", () => {
}); });
describe("Test name prop", () => { 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(<Excalidraw />); const { container } = await render(<Excalidraw />);
//open menu //open menu
toggleMenu(container); toggleMenu(container);
@ -315,7 +315,7 @@ describe("<Excalidraw/>", () => {
expect(textInput?.nodeName).toBe("INPUT"); 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 name = "test";
const { container } = await render(<Excalidraw name={name} />); const { container } = await render(<Excalidraw name={name} />);
//open menu //open menu
@ -326,7 +326,6 @@ describe("<Excalidraw/>", () => {
) as HTMLInputElement; ) as HTMLInputElement;
expect(textInput?.value).toEqual(name); expect(textInput?.value).toEqual(name);
expect(textInput?.nodeName).toBe("INPUT"); expect(textInput?.nodeName).toBe("INPUT");
expect(textInput?.disabled).toBe(true);
}); });
}); });

View file

@ -247,7 +247,7 @@ export interface AppState {
scrollY: number; scrollY: number;
cursorButton: "up" | "down"; cursorButton: "up" | "down";
scrolledOutside: boolean; scrolledOutside: boolean;
name: string; name: string | null;
isResizing: boolean; isResizing: boolean;
isRotating: boolean; isRotating: boolean;
zoom: Zoom; zoom: Zoom;
@ -435,6 +435,7 @@ export interface ExcalidrawProps {
objectsSnapModeEnabled?: boolean; objectsSnapModeEnabled?: boolean;
libraryReturnUrl?: string; libraryReturnUrl?: string;
theme?: Theme; theme?: Theme;
// @TODO come with better API before v0.18.0
name?: string; name?: string;
renderCustomStats?: ( renderCustomStats?: (
elements: readonly NonDeletedExcalidrawElement[], elements: readonly NonDeletedExcalidrawElement[],
@ -577,6 +578,7 @@ export type AppClassProperties = {
setOpenDialog: App["setOpenDialog"]; setOpenDialog: App["setOpenDialog"];
insertEmbeddableElement: App["insertEmbeddableElement"]; insertEmbeddableElement: App["insertEmbeddableElement"];
onMagicframeToolSelect: App["onMagicframeToolSelect"]; onMagicframeToolSelect: App["onMagicframeToolSelect"];
getName: App["getName"];
}; };
export type PointerDownState = Readonly<{ export type PointerDownState = Readonly<{
@ -651,10 +653,11 @@ export type ExcalidrawImperativeAPI = {
history: { history: {
clear: InstanceType<typeof App>["resetHistory"]; clear: InstanceType<typeof App>["resetHistory"];
}; };
scrollToContent: InstanceType<typeof App>["scrollToContent"];
getSceneElements: InstanceType<typeof App>["getSceneElements"]; getSceneElements: InstanceType<typeof App>["getSceneElements"];
getAppState: () => InstanceType<typeof App>["state"]; getAppState: () => InstanceType<typeof App>["state"];
getFiles: () => InstanceType<typeof App>["files"]; getFiles: () => InstanceType<typeof App>["files"];
getName: InstanceType<typeof App>["getName"];
scrollToContent: InstanceType<typeof App>["scrollToContent"];
registerAction: (action: Action) => void; registerAction: (action: Action) => void;
refresh: InstanceType<typeof App>["refresh"]; refresh: InstanceType<typeof App>["refresh"];
setToast: InstanceType<typeof App>["setToast"]; setToast: InstanceType<typeof App>["setToast"];