Skip to content

Commit

Permalink
feat: improve collab error notification (excalidraw#7741)
Browse files Browse the repository at this point in the history
* identify cause

* toast after dialog for error messages in collab

* remove comment

* shake tooltip instead for repeating collab errors

* clear collab error

* empty commit

* simplify & fix reset race condition

---------

Co-authored-by: dwelle <[email protected]>
  • Loading branch information
ryan-di and dwelle authored Mar 4, 2024
1 parent f207bd0 commit 160440b
Show file tree
Hide file tree
Showing 8 changed files with 168 additions and 20 deletions.
17 changes: 11 additions & 6 deletions excalidraw-app/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ import { openConfirmModal } from "../packages/excalidraw/components/OverwriteCon
import { OverwriteConfirmDialog } from "../packages/excalidraw/components/OverwriteConfirm/OverwriteConfirm";
import Trans from "../packages/excalidraw/components/Trans";
import { ShareDialog, shareDialogStateAtom } from "./share/ShareDialog";
import CollabError, { collabErrorIndicatorAtom } from "./collab/CollabError";

polyfill();

Expand Down Expand Up @@ -310,6 +311,7 @@ const ExcalidrawWrapper = () => {
const [isCollaborating] = useAtomWithInitialValue(isCollaboratingAtom, () => {
return isCollaborationLink(window.location.href);
});
const collabError = useAtomValue(collabErrorIndicatorAtom);

useHandleLibrary({
excalidrawAPI,
Expand Down Expand Up @@ -748,12 +750,15 @@ const ExcalidrawWrapper = () => {
return null;
}
return (
<LiveCollaborationTrigger
isCollaborating={isCollaborating}
onSelect={() =>
setShareDialogState({ isOpen: true, type: "share" })
}
/>
<div className="top-right-ui">
{collabError.message && <CollabError collabError={collabError} />}
<LiveCollaborationTrigger
isCollaborating={isCollaborating}
onSelect={() =>
setShareDialogState({ isOpen: true, type: "share" })
}
/>
</div>
);
}}
>
Expand Down
64 changes: 52 additions & 12 deletions excalidraw-app/collab/Collab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,16 @@ import { appJotaiStore } from "../app-jotai";
import { Mutable, ValueOf } from "../../packages/excalidraw/utility-types";
import { getVisibleSceneBounds } from "../../packages/excalidraw/element/bounds";
import { withBatchedUpdates } from "../../packages/excalidraw/reactUtils";
import { collabErrorIndicatorAtom } from "./CollabError";

export const collabAPIAtom = atom<CollabAPI | null>(null);
export const isCollaboratingAtom = atom(false);
export const isOfflineAtom = atom(false);

interface CollabState {
errorMessage: string | null;
/** errors related to saving */
dialogNotifiedErrors: Record<string, boolean>;
username: string;
activeRoomLink: string | null;
}
Expand All @@ -107,7 +110,7 @@ export interface CollabAPI {
setUsername: CollabInstance["setUsername"];
getUsername: CollabInstance["getUsername"];
getActiveRoomLink: CollabInstance["getActiveRoomLink"];
setErrorMessage: CollabInstance["setErrorMessage"];
setCollabError: CollabInstance["setErrorDialog"];
}

interface CollabProps {
Expand All @@ -129,6 +132,7 @@ class Collab extends PureComponent<CollabProps, CollabState> {
super(props);
this.state = {
errorMessage: null,
dialogNotifiedErrors: {},
username: importUsernameFromLocalStorage() || "",
activeRoomLink: null,
};
Expand Down Expand Up @@ -197,7 +201,7 @@ class Collab extends PureComponent<CollabProps, CollabState> {
setUsername: this.setUsername,
getUsername: this.getUsername,
getActiveRoomLink: this.getActiveRoomLink,
setErrorMessage: this.setErrorMessage,
setCollabError: this.setErrorDialog,
};

appJotaiStore.set(collabAPIAtom, collabAPI);
Expand Down Expand Up @@ -276,18 +280,35 @@ class Collab extends PureComponent<CollabProps, CollabState> {
this.excalidrawAPI.getAppState(),
);

this.resetErrorIndicator();

if (this.isCollaborating() && savedData && savedData.reconciledElements) {
this.handleRemoteSceneUpdate(
this.reconcileElements(savedData.reconciledElements),
);
}
} catch (error: any) {
this.setState({
// firestore doesn't return a specific error code when size exceeded
errorMessage: /is longer than.*?bytes/.test(error.message)
? t("errors.collabSaveFailed_sizeExceeded")
: t("errors.collabSaveFailed"),
});
const errorMessage = /is longer than.*?bytes/.test(error.message)
? t("errors.collabSaveFailed_sizeExceeded")
: t("errors.collabSaveFailed");

if (
!this.state.dialogNotifiedErrors[errorMessage] ||
!this.isCollaborating()
) {
this.setErrorDialog(errorMessage);
this.setState({
dialogNotifiedErrors: {
...this.state.dialogNotifiedErrors,
[errorMessage]: true,
},
});
}

if (this.isCollaborating()) {
this.setErrorIndicator(errorMessage);
}

console.error(error);
}
};
Expand All @@ -296,6 +317,7 @@ class Collab extends PureComponent<CollabProps, CollabState> {
this.queueBroadcastAllElements.cancel();
this.queueSaveToFirebase.cancel();
this.loadImageFiles.cancel();
this.resetErrorIndicator(true);

this.saveCollabRoomToFirebase(
getSyncableElements(
Expand Down Expand Up @@ -464,7 +486,7 @@ class Collab extends PureComponent<CollabProps, CollabState> {
this.portal.socket.once("connect_error", fallbackInitializationHandler);
} catch (error: any) {
console.error(error);
this.setState({ errorMessage: error.message });
this.setErrorDialog(error.message);
return null;
}

Expand Down Expand Up @@ -923,8 +945,26 @@ class Collab extends PureComponent<CollabProps, CollabState> {

getActiveRoomLink = () => this.state.activeRoomLink;

setErrorMessage = (errorMessage: string | null) => {
this.setState({ errorMessage });
setErrorIndicator = (errorMessage: string | null) => {
appJotaiStore.set(collabErrorIndicatorAtom, {
message: errorMessage,
nonce: Date.now(),
});
};

resetErrorIndicator = (resetDialogNotifiedErrors = false) => {
appJotaiStore.set(collabErrorIndicatorAtom, { message: null, nonce: 0 });
if (resetDialogNotifiedErrors) {
this.setState({
dialogNotifiedErrors: {},
});
}
};

setErrorDialog = (errorMessage: string | null) => {
this.setState({
errorMessage,
});
};

render() {
Expand All @@ -933,7 +973,7 @@ class Collab extends PureComponent<CollabProps, CollabState> {
return (
<>
{errorMessage != null && (
<ErrorDialog onClose={() => this.setState({ errorMessage: null })}>
<ErrorDialog onClose={() => this.setErrorDialog(null)}>
{errorMessage}
</ErrorDialog>
)}
Expand Down
35 changes: 35 additions & 0 deletions excalidraw-app/collab/CollabError.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
@import "../../packages/excalidraw/css/variables.module.scss";

.excalidraw {
.collab-errors-button {
width: 26px;
height: 26px;
margin-inline-end: 1rem;

color: var(--color-danger);

flex-shrink: 0;
}

.collab-errors-button-shake {
animation: strong-shake 0.15s 6;
}

@keyframes strong-shake {
0% {
transform: rotate(0deg);
}
25% {
transform: rotate(10deg);
}
50% {
transform: rotate(0eg);
}
75% {
transform: rotate(-10deg);
}
100% {
transform: rotate(0deg);
}
}
}
54 changes: 54 additions & 0 deletions excalidraw-app/collab/CollabError.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { Tooltip } from "../../packages/excalidraw/components/Tooltip";
import { warning } from "../../packages/excalidraw/components/icons";
import clsx from "clsx";
import { useEffect, useRef, useState } from "react";

import "./CollabError.scss";
import { atom } from "jotai";

type ErrorIndicator = {
message: string | null;
/** used to rerun the useEffect responsible for animation */
nonce: number;
};

export const collabErrorIndicatorAtom = atom<ErrorIndicator>({
message: null,
nonce: 0,
});

const CollabError = ({ collabError }: { collabError: ErrorIndicator }) => {
const [isAnimating, setIsAnimating] = useState(false);
const clearAnimationRef = useRef<string | number | NodeJS.Timeout>();

useEffect(() => {
setIsAnimating(true);
clearAnimationRef.current = setTimeout(() => {
setIsAnimating(false);
}, 1000);

return () => {
clearTimeout(clearAnimationRef.current);
};
}, [collabError.message, collabError.nonce]);

if (!collabError.message) {
return null;
}

return (
<Tooltip label={collabError.message} long={true}>
<div
className={clsx("collab-errors-button", {
"collab-errors-button-shake": isAnimating,
})}
>
{warning}
</div>
</Tooltip>
);
};

CollabError.displayName = "CollabError";

export default CollabError;
7 changes: 7 additions & 0 deletions excalidraw-app/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@
&.theme--dark {
--color-primary-contrast-offset: #726dff; // to offset Chubb illusion
}

.top-right-ui {
display: flex;
justify-content: center;
align-items: center;
}

.footer-center {
justify-content: flex-end;
margin-top: auto;
Expand Down
2 changes: 1 addition & 1 deletion excalidraw-app/share/ShareDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ const ActiveRoomDialog = ({
try {
await copyTextToSystemClipboard(activeRoomLink);
} catch (e) {
collabAPI.setErrorMessage(t("errors.copyToSystemClipboardFailed"));
collabAPI.setCollabError(t("errors.copyToSystemClipboardFailed"));
}

setJustCopied(true);
Expand Down
5 changes: 4 additions & 1 deletion packages/excalidraw/components/Toast.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useCallback, useEffect, useRef } from "react";
import { CSSProperties, useCallback, useEffect, useRef } from "react";
import { CloseIcon } from "./icons";
import "./Toast.scss";
import { ToolButton } from "./ToolButton";
Expand All @@ -11,11 +11,13 @@ export const Toast = ({
closable = false,
// To prevent autoclose, pass duration as Infinity
duration = DEFAULT_TOAST_TIMEOUT,
style,
}: {
message: string;
onClose: () => void;
closable?: boolean;
duration?: number;
style?: CSSProperties;
}) => {
const timerRef = useRef<number>(0);
const shouldAutoClose = duration !== Infinity;
Expand Down Expand Up @@ -43,6 +45,7 @@ export const Toast = ({
className="Toast"
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
style={style}
>
<p className="Toast__message">{message}</p>
{closable && (
Expand Down
4 changes: 4 additions & 0 deletions packages/excalidraw/components/icons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,10 @@ export const share = createIcon(
modifiedTablerIconProps,
);

export const warning = createIcon(
"M256 32c14.2 0 27.3 7.5 34.5 19.8l216 368c7.3 12.4 7.3 27.7 .2 40.1S486.3 480 472 480H40c-14.3 0-27.6-7.7-34.7-20.1s-7-27.8 .2-40.1l216-368C228.7 39.5 241.8 32 256 32zm0 128c-13.3 0-24 10.7-24 24V296c0 13.3 10.7 24 24 24s24-10.7 24-24V184c0-13.3-10.7-24-24-24zm32 224a32 32 0 1 0 -64 0 32 32 0 1 0 64 0z",
);

export const shareIOS = createIcon(
"M16 5l-1.42 1.42-1.59-1.59V16h-1.98V4.83L9.42 6.42 8 5l4-4 4 4zm4 5v11c0 1.1-.9 2-2 2H6c-1.11 0-2-.9-2-2V10c0-1.11.89-2 2-2h3v2H6v11h12V10h-3V8h3c1.1 0 2 .89 2 2z",
{ width: 24, height: 24 },
Expand Down

0 comments on commit 160440b

Please sign in to comment.