From 9325109836595276cdb97003eeff11086dfbfc52 Mon Sep 17 00:00:00 2001
From: Aakansha Doshi <aakansha1216@gmail.com>
Date: Thu, 10 Jun 2021 02:46:56 +0530
Subject: [PATCH] fix: use excal id so every element has unique id  (#3696)

* fix: use excal id so every element has unique id

* fix

* fix

* fix

* add docs

* fix
---
 src/components/App.tsx                        | 25 ++++++++++++++-----
 src/components/Dialog.tsx                     |  5 ++--
 src/components/ErrorDialog.tsx                |  2 +-
 src/components/LockIcon.tsx                   |  2 --
 src/components/Modal.tsx                      |  2 +-
 src/components/ProjectName.tsx                |  6 +++--
 src/components/Section.tsx                    |  6 +++--
 src/components/ToolButton.tsx                 |  4 ++-
 src/css/styles.scss                           |  5 ++--
 src/packages/excalidraw/CHANGELOG.md          |  4 +++
 src/setupTests.ts                             |  5 ++++
 .../__snapshots__/excalidraw.test.tsx.snap    |  8 +++---
 src/tests/packages/excalidraw.test.tsx        |  6 ++---
 13 files changed, 53 insertions(+), 27 deletions(-)

diff --git a/src/components/App.tsx b/src/components/App.tsx
index 13386e6f..f97a0a8c 100644
--- a/src/components/App.tsx
+++ b/src/components/App.tsx
@@ -197,9 +197,10 @@ import { actionToggleViewMode } from "../actions/actionToggleViewMode";
 
 const IsMobileContext = React.createContext(false);
 export const useIsMobile = () => useContext(IsMobileContext);
-const ExcalidrawContainerContext = React.createContext<HTMLDivElement | null>(
-  null,
-);
+const ExcalidrawContainerContext = React.createContext<{
+  container: HTMLDivElement | null;
+  id: string | null;
+}>({ container: null, id: null });
 export const useExcalidrawContainer = () =>
   useContext(ExcalidrawContainerContext);
 
@@ -244,6 +245,10 @@ class App extends React.Component<AppProps, AppState> {
   public libraryItemsFromStorage: LibraryItems | undefined;
   private id: string;
   private history: History;
+  private excalidrawContainerValue: {
+    container: HTMLDivElement | null;
+    id: string;
+  };
 
   constructor(props: AppProps) {
     super(props);
@@ -300,6 +305,12 @@ class App extends React.Component<AppProps, AppState> {
       }
       readyPromise.resolve(api);
     }
+
+    this.excalidrawContainerValue = {
+      container: this.excalidrawContainerRef.current,
+      id: this.id,
+    };
+
     this.scene = new Scene();
     this.library = new Library(this);
     this.history = new History();
@@ -327,7 +338,7 @@ class App extends React.Component<AppProps, AppState> {
     if (viewModeEnabled) {
       return (
         <canvas
-          id="canvas"
+          className="excalidraw__canvas"
           style={{
             width: canvasDOMWidth,
             height: canvasDOMHeight,
@@ -349,7 +360,7 @@ class App extends React.Component<AppProps, AppState> {
     }
     return (
       <canvas
-        id="canvas"
+        className="excalidraw__canvas"
         style={{
           width: canvasDOMWidth,
           height: canvasDOMHeight,
@@ -394,7 +405,7 @@ class App extends React.Component<AppProps, AppState> {
         }
       >
         <ExcalidrawContainerContext.Provider
-          value={this.excalidrawContainerRef.current}
+          value={this.excalidrawContainerValue}
         >
           <IsMobileContext.Provider value={this.isMobile}>
             <LayerUI
@@ -725,6 +736,8 @@ class App extends React.Component<AppProps, AppState> {
   };
 
   public async componentDidMount() {
+    this.excalidrawContainerValue.container = this.excalidrawContainerRef.current;
+
     if (
       process.env.NODE_ENV === ENV.TEST ||
       process.env.NODE_ENV === ENV.DEVELOPMENT
diff --git a/src/components/Dialog.tsx b/src/components/Dialog.tsx
index ada1bb5a..79cebf7e 100644
--- a/src/components/Dialog.tsx
+++ b/src/components/Dialog.tsx
@@ -2,7 +2,7 @@ import clsx from "clsx";
 import React, { useEffect, useState } from "react";
 import { useCallbackRefState } from "../hooks/useCallbackRefState";
 import { t } from "../i18n";
-import { useIsMobile } from "../components/App";
+import { useExcalidrawContainer, useIsMobile } from "../components/App";
 import { KEYS } from "../keys";
 import "./Dialog.scss";
 import { back, close } from "./icons";
@@ -21,6 +21,7 @@ export const Dialog = (props: {
 }) => {
   const [islandNode, setIslandNode] = useCallbackRefState<HTMLDivElement>();
   const [lastActiveElement] = useState(document.activeElement);
+  const { id } = useExcalidrawContainer();
 
   useEffect(() => {
     if (!islandNode) {
@@ -82,7 +83,7 @@ export const Dialog = (props: {
       theme={props.theme}
     >
       <Island ref={setIslandNode}>
-        <h2 id="dialog-title" className="Dialog__title">
+        <h2 id={`${id}-dialog-title`} className="Dialog__title">
           <span className="Dialog__titleContent">{props.title}</span>
           <button
             className="Modal__close"
diff --git a/src/components/ErrorDialog.tsx b/src/components/ErrorDialog.tsx
index f383a00c..c1c78998 100644
--- a/src/components/ErrorDialog.tsx
+++ b/src/components/ErrorDialog.tsx
@@ -12,7 +12,7 @@ export const ErrorDialog = ({
   onClose?: () => void;
 }) => {
   const [modalIsShown, setModalIsShown] = useState(!!message);
-  const excalidrawContainer = useExcalidrawContainer();
+  const { container: excalidrawContainer } = useExcalidrawContainer();
 
   const handleClose = React.useCallback(() => {
     setModalIsShown(false);
diff --git a/src/components/LockIcon.tsx b/src/components/LockIcon.tsx
index 5fe18d52..eaae9aa9 100644
--- a/src/components/LockIcon.tsx
+++ b/src/components/LockIcon.tsx
@@ -8,7 +8,6 @@ type LockIconSize = "s" | "m";
 type LockIconProps = {
   title?: string;
   name?: string;
-  id?: string;
   checked: boolean;
   onChange?(): void;
   size?: LockIconSize;
@@ -57,7 +56,6 @@ export const LockIcon = (props: LockIconProps) => {
         className="ToolIcon_type_checkbox"
         type="checkbox"
         name={props.name}
-        id={props.id}
         onChange={props.onChange}
         checked={props.checked}
         aria-label={props.title}
diff --git a/src/components/Modal.tsx b/src/components/Modal.tsx
index 50b983c0..bd61f2d3 100644
--- a/src/components/Modal.tsx
+++ b/src/components/Modal.tsx
@@ -58,7 +58,7 @@ const useBodyRoot = (theme: AppState["theme"]) => {
   const isMobileRef = useRef(isMobile);
   isMobileRef.current = isMobile;
 
-  const excalidrawContainer = useExcalidrawContainer();
+  const { container: excalidrawContainer } = useExcalidrawContainer();
 
   useLayoutEffect(() => {
     if (div) {
diff --git a/src/components/ProjectName.tsx b/src/components/ProjectName.tsx
index b14ded79..411cd554 100644
--- a/src/components/ProjectName.tsx
+++ b/src/components/ProjectName.tsx
@@ -4,6 +4,7 @@ import React, { useState } from "react";
 import { focusNearestParent } from "../utils";
 
 import "./ProjectName.scss";
+import { useExcalidrawContainer } from "./App";
 
 type Props = {
   value: string;
@@ -13,6 +14,7 @@ type Props = {
 };
 
 export const ProjectName = (props: Props) => {
+  const { id } = useExcalidrawContainer();
   const [fileName, setFileName] = useState<string>(props.value);
 
   const handleBlur = (event: any) => {
@@ -43,12 +45,12 @@ export const ProjectName = (props: Props) => {
           className="TextInput"
           onBlur={handleBlur}
           onKeyDown={handleKeyDown}
-          id="filename"
+          id={`${id}-filename`}
           value={fileName}
           onChange={(event) => setFileName(event.target.value)}
         />
       ) : (
-        <span className="TextInput TextInput--readonly" id="filename">
+        <span className="TextInput TextInput--readonly" id={`${id}-filename`}>
           {props.value}
         </span>
       )}
diff --git a/src/components/Section.tsx b/src/components/Section.tsx
index 431bb27f..ebe00f8f 100644
--- a/src/components/Section.tsx
+++ b/src/components/Section.tsx
@@ -1,5 +1,6 @@
 import React from "react";
 import { t } from "../i18n";
+import { useExcalidrawContainer } from "./App";
 
 interface SectionProps extends React.HTMLProps<HTMLElement> {
   heading: string;
@@ -7,13 +8,14 @@ interface SectionProps extends React.HTMLProps<HTMLElement> {
 }
 
 export const Section = ({ heading, children, ...props }: SectionProps) => {
+  const { id } = useExcalidrawContainer();
   const header = (
-    <h2 className="visually-hidden" id={`${heading}-title`}>
+    <h2 className="visually-hidden" id={`${id}-${heading}-title`}>
       {t(`headings.${heading}`)}
     </h2>
   );
   return (
-    <section {...props} aria-labelledby={`${heading}-title`}>
+    <section {...props} aria-labelledby={`${id}-${heading}-title`}>
       {typeof children === "function" ? (
         children(header)
       ) : (
diff --git a/src/components/ToolButton.tsx b/src/components/ToolButton.tsx
index c2ed29d6..dd3b10d6 100644
--- a/src/components/ToolButton.tsx
+++ b/src/components/ToolButton.tsx
@@ -2,6 +2,7 @@ import "./ToolIcon.scss";
 
 import React from "react";
 import clsx from "clsx";
+import { useExcalidrawContainer } from "./App";
 
 type ToolIconSize = "s" | "m";
 
@@ -43,6 +44,7 @@ type ToolButtonProps =
 const DEFAULT_SIZE: ToolIconSize = "m";
 
 export const ToolButton = React.forwardRef((props: ToolButtonProps, ref) => {
+  const { id: excalId } = useExcalidrawContainer();
   const innerRef = React.useRef(null);
   React.useImperativeHandle(ref, () => innerRef.current);
   const sizeCn = `ToolIcon_size_${props.size || DEFAULT_SIZE}`;
@@ -98,7 +100,7 @@ export const ToolButton = React.forwardRef((props: ToolButtonProps, ref) => {
         aria-label={props["aria-label"]}
         aria-keyshortcuts={props["aria-keyshortcuts"]}
         data-testid={props["data-testid"]}
-        id={props.id}
+        id={`${excalId}-${props.id}`}
         onChange={props.onChange}
         checked={props.checked}
         ref={innerRef}
diff --git a/src/css/styles.scss b/src/css/styles.scss
index 7aa2e0b5..cb67a574 100644
--- a/src/css/styles.scss
+++ b/src/css/styles.scss
@@ -51,11 +51,12 @@
     image-rendering: -moz-crisp-edges; // FF
 
     z-index: var(--zIndex-canvas);
-  }
 
-  #canvas {
     // Remove the main canvas from document flow to avoid resizeObserver
     // feedback loop (see https://github.com/excalidraw/excalidraw/pull/3379)
+  }
+
+  &__canvas {
     position: absolute;
   }
 
diff --git a/src/packages/excalidraw/CHANGELOG.md b/src/packages/excalidraw/CHANGELOG.md
index 96fb8d76..d0949271 100644
--- a/src/packages/excalidraw/CHANGELOG.md
+++ b/src/packages/excalidraw/CHANGELOG.md
@@ -35,6 +35,10 @@ Please add the latest change on the top under the correct section.
 - `UIOptions.canvasActions.saveAsScene` is now renamed to `UiOptions.canvasActions.export.saveFileToDisk`. Defaults to `true` hence the **save file to disk** button is rendered inside the export dialog.
 - `exportToBackend` is now renamed to `UIOptions.canvasActions.export.exportToBackend`. If this prop is not passed, the **shareable-link** button will not be rendered, same as before.
 
+### Fixes
+
+- Use excalidraw Id in elements so every element has unique id [#3696](https://github.com/excalidraw/excalidraw/pull/3696).
+
 ### Refactor
 
 - #### BREAKING CHANGE
diff --git a/src/setupTests.ts b/src/setupTests.ts
index fe48d2c4..bd9475b5 100644
--- a/src/setupTests.ts
+++ b/src/setupTests.ts
@@ -1,6 +1,11 @@
 import "@testing-library/jest-dom";
 import "jest-canvas-mock";
 
+jest.mock("nanoid", () => {
+  return {
+    nanoid: jest.fn(() => "test-id"),
+  };
+});
 // ReactDOM is located inside index.tsx file
 // as a result, we need a place for it to render into
 const element = document.createElement("div");
diff --git a/src/tests/packages/__snapshots__/excalidraw.test.tsx.snap b/src/tests/packages/__snapshots__/excalidraw.test.tsx.snap
index 0dd76513..97d188b6 100644
--- a/src/tests/packages/__snapshots__/excalidraw.test.tsx.snap
+++ b/src/tests/packages/__snapshots__/excalidraw.test.tsx.snap
@@ -2,12 +2,12 @@
 
 exports[`<Excalidraw/> Test UIOptions prop Test canvasActions should not hide any UI element when canvasActions is "undefined" 1`] = `
 <section
-  aria-labelledby="canvasActions-title"
+  aria-labelledby="test-id-canvasActions-title"
   class="zen-mode-transition"
 >
   <h2
     class="visually-hidden"
-    id="canvasActions-title"
+    id="test-id-canvasActions-title"
   >
     Canvas actions
   </h2>
@@ -201,12 +201,12 @@ exports[`<Excalidraw/> Test UIOptions prop Test canvasActions should not hide an
 
 exports[`<Excalidraw/> Test UIOptions prop should not hide any UI element when the UIOptions prop is "undefined" 1`] = `
 <section
-  aria-labelledby="canvasActions-title"
+  aria-labelledby="test-id-canvasActions-title"
   class="zen-mode-transition"
 >
   <h2
     class="visually-hidden"
-    id="canvasActions-title"
+    id="test-id-canvasActions-title"
   >
     Canvas actions
   </h2>
diff --git a/src/tests/packages/excalidraw.test.tsx b/src/tests/packages/excalidraw.test.tsx
index a1e6927e..947964cf 100644
--- a/src/tests/packages/excalidraw.test.tsx
+++ b/src/tests/packages/excalidraw.test.tsx
@@ -136,7 +136,7 @@ describe("<Excalidraw/>", () => {
       await render(<Excalidraw />);
 
       const canvasActions = document.querySelector(
-        'section[aria-labelledby="canvasActions-title"]',
+        'section[aria-labelledby="test-id-canvasActions-title"]',
       );
 
       expect(canvasActions).toMatchSnapshot();
@@ -145,11 +145,9 @@ describe("<Excalidraw/>", () => {
     describe("Test canvasActions", () => {
       it('should not hide any UI element when canvasActions is "undefined"', async () => {
         await render(<Excalidraw UIOptions={{}} />);
-
         const canvasActions = document.querySelector(
-          'section[aria-labelledby="canvasActions-title"]',
+          'section[aria-labelledby="test-id-canvasActions-title"]',
         );
-
         expect(canvasActions).toMatchSnapshot();
       });