merge upstream (#5821)

* fix: hide canvas-modifying UI in view mode (#5815)

* fix: hide canvas-modifying UI in view mode

* add class for better targeting

* fix missing `key`

* fix: useOutsideClick not working in view mode

* fix: Corrected typo in toggle theme shortcut (#5813)

* fix: incorrectly selecting linear elements on creation while tool-locked (#5785)

* fix: syncing 1-point lines to remote clients (#5677)

* feat: stop deleting whole line when no point select in line editor (#5676)

* feat: stop deleting whole line when no point select in line editor

* Comments typo

Co-authored-by: DanielJGeiger <[email protected]>

Co-authored-by: David Luzar <[email protected]>
Co-authored-by: Paul Yi <[email protected]>
Co-authored-by: DanielJGeiger <[email protected]>

src/actions/actionDeleteSelected.tsx

@@ -72,13 +72,22 @@ export const actionDeleteSelected = register({
       if (!element) {
         return false;
       }
-      if (
-        // case: no point selected → delete whole element
-        selectedPointsIndices == null ||
-        // case: deleting last remaining point
-        element.points.length < 2
-      ) {
-        const nextElements = elements.filter((el) => el.id !== element.id);
+      // case: no point selected → do nothing, as deleting the whole element
+      // is most likely a mistake, where you wanted to delete a specific point
+      // but failed to select it (or you thought it's selected, while it was
+      // only in a hover state)
+      if (selectedPointsIndices == null) {
+        return false;
+      }
+
+      // case: deleting last remaining point
+      if (element.points.length < 2) {
+        const nextElements = elements.map((el) => {
+          if (el.id === element.id) {
+            return newElementWith(el, { isDeleted: true });
+          }
+          return el;
+        });
         const nextAppState = handleGroupEditingState(appState, nextElements);
 
         return {

src/actions/shortcuts.ts

@@ -38,7 +38,7 @@ export type ShortcutName =
   | "imageExport";
 
 const shortcutMap: Record<ShortcutName, string[]> = {
-  toggleTheme: [getShortcutKey("Shit+Alt+D")],
+  toggleTheme: [getShortcutKey("Shift+Alt+D")],
   saveScene: [getShortcutKey("CtrlOrCmd+S")],
   loadScene: [getShortcutKey("CtrlOrCmd+O")],
   imageExport: [getShortcutKey("CtrlOrCmd+Shift+E")],

src/components/Actions.tsx

@@ -237,7 +237,7 @@ export const ShapesSwitcher = ({
           keyBindingLabel={`${numberKey}`}
           aria-label={capitalizeString(label)}
           aria-keyshortcuts={shortcut}
-          data-testid={value}
+          data-testid={`toolbar-${value}`}
           onPointerDown={({ pointerType }) => {
             if (!appState.penDetected && pointerType === "pen") {
               setAppState({

src/components/App.tsx

@@ -4836,10 +4836,6 @@ class App extends React.Component<AppProps, AppState> {
           } else {
             this.setState((prevState) => ({
               draggingElement: null,
-              selectedElementIds: {
-                ...prevState.selectedElementIds,
-                [draggingElement.id]: true,
-              },
             }));
           }
         }

src/components/LayerUI.tsx

@@ -213,7 +213,8 @@ const LayerUI = ({
               padding={2}
               style={{ zIndex: 1 }}
             >
-              {actionManager.renderAction("loadScene")}
+              {!appState.viewModeEnabled &&
+                actionManager.renderAction("loadScene")}
               {/* // TODO barnabasmolnar/editor-redesign  */}
               {/* is this fine here? */}
               {UIOptions.canvasActions.saveToActiveFile &&
@@ -237,7 +238,8 @@ const LayerUI = ({
                 />
               )}
               {actionManager.renderAction("toggleShortcuts", undefined, true)}
-              {actionManager.renderAction("clearCanvas")}
+              {!appState.viewModeEnabled &&
+                actionManager.renderAction("clearCanvas")}
               <Separator />
               <MenuLinks />
               <Separator />
@@ -252,14 +254,16 @@ const LayerUI = ({
                 <div style={{ padding: "0 0.625rem" }}>
                   <LanguageList style={{ width: "100%" }} />
                 </div>
-                <div>
-                  <div style={{ fontSize: ".75rem", marginBottom: ".5rem" }}>
-                    {t("labels.canvasBackground")}
-                  </div>
-                  <div style={{ padding: "0 0.625rem" }}>
-                    {actionManager.renderAction("changeViewBackgroundColor")}
+                {!appState.viewModeEnabled && (
+                  <div>
+                    <div style={{ fontSize: ".75rem", marginBottom: ".5rem" }}>
+                      {t("labels.canvasBackground")}
+                    </div>
+                    <div style={{ padding: "0 0.625rem" }}>
+                      {actionManager.renderAction("changeViewBackgroundColor")}
+                    </div>
                   </div>
-                </div>
+                )}
               </div>
             </Island>
           </Section>
@@ -302,12 +306,12 @@ const LayerUI = ({
     return (
       <FixedSideContainer side="top">
         {renderWelcomeScreen && !appState.isLoading && (
-          <WelcomeScreen actionManager={actionManager} />
+          <WelcomeScreen appState={appState} actionManager={actionManager} />
         )}
         <div className="App-menu App-menu_top">
           <Stack.Col
             gap={6}
-            className={clsx({
+            className={clsx("App-menu_top__left", {
               "disable-pointerEvents": appState.zenModeEnabled,
             })}
           >
@@ -408,7 +412,9 @@ const LayerUI = ({
               />
             )}
             {renderTopRightUI?.(device.isMobile, appState)}
-            <LibraryButton appState={appState} setAppState={setAppState} />
+            {!appState.viewModeEnabled && (
+              <LibraryButton appState={appState} setAppState={setAppState} />
+            )}
           </div>
         </div>
       </FixedSideContainer>

src/components/LockButton.tsx

@@ -39,6 +39,7 @@ export const LockButton = (props: LockIconProps) => {
         onChange={props.onChange}
         checked={props.checked}
         aria-label={props.title}
+        data-testid="toolbar-lock"
       />
       <div className="ToolIcon__icon">
         {props.checked ? ICONS.CHECKED : ICONS.UNCHECKED}

src/components/MobileMenu.tsx

@@ -75,7 +75,7 @@ export const MobileMenu = ({
     return (
       <FixedSideContainer side="top" className="App-top-bar">
         {renderWelcomeScreen && !appState.isLoading && (
-          <WelcomeScreen actionManager={actionManager} />
+          <WelcomeScreen appState={appState} actionManager={actionManager} />
         )}
         <Section heading="shapes">
           {(heading: React.ReactNode) => (
@@ -127,11 +127,13 @@ export const MobileMenu = ({
                     title={t("toolBar.lock")}
                     isMobile
                   />
-                  <LibraryButton
-                    appState={appState}
-                    setAppState={setAppState}
-                    isMobile
-                  />
+                  {!appState.viewModeEnabled && (
+                    <LibraryButton
+                      appState={appState}
+                      setAppState={setAppState}
+                      isMobile
+                    />
+                  )}
                 </div>
               </Stack.Row>
             </Stack.Col>
@@ -187,7 +189,7 @@ export const MobileMenu = ({
     }
     return (
       <>
-        {actionManager.renderAction("loadScene")}
+        {!appState.viewModeEnabled && actionManager.renderAction("loadScene")}
         {renderJSONExportDialog()}
         {renderImageExportDialog()}
         <MenuItem
@@ -204,18 +206,20 @@ export const MobileMenu = ({
           />
         )}
         {actionManager.renderAction("toggleShortcuts", undefined, true)}
-        {actionManager.renderAction("clearCanvas")}
+        {!appState.viewModeEnabled && actionManager.renderAction("clearCanvas")}
         <Separator />
         <MenuLinks />
         <Separator />
-        <div style={{ marginBottom: ".5rem" }}>
-          <div style={{ fontSize: ".75rem", marginBottom: ".5rem" }}>
-            {t("labels.canvasBackground")}
-          </div>
-          <div style={{ padding: "0 0.625rem" }}>
-            {actionManager.renderAction("changeViewBackgroundColor")}
+        {!appState.viewModeEnabled && (
+          <div style={{ marginBottom: ".5rem" }}>
+            <div style={{ fontSize: ".75rem", marginBottom: ".5rem" }}>
+              {t("labels.canvasBackground")}
+            </div>
+            <div style={{ padding: "0 0.625rem" }}>
+              {actionManager.renderAction("changeViewBackgroundColor")}
+            </div>
           </div>
-        </div>
+        )}
         {actionManager.renderAction("toggleTheme")}
       </>
     );

src/components/WelcomeScreen.tsx

@@ -5,6 +5,7 @@ import { getShortcutFromShortcutName } from "../actions/shortcuts";
 import { COOKIES } from "../constants";
 import { collabDialogShownAtom } from "../excalidraw-app/collab/Collab";
 import { t } from "../i18n";
+import { AppState } from "../types";
 import {
   ExcalLogo,
   HelpIcon,
@@ -60,7 +61,13 @@ const WelcomeScreenItem = ({
   );
 };
 
-const WelcomeScreen = ({ actionManager }: { actionManager: ActionManager }) => {
+const WelcomeScreen = ({
+  appState,
+  actionManager,
+}: {
+  appState: AppState;
+  actionManager: ActionManager;
+}) => {
   const [, setCollabDialogShown] = useAtom(collabDialogShownAtom);
 
   let subheadingJSX;
@@ -68,12 +75,13 @@ const WelcomeScreen = ({ actionManager }: { actionManager: ActionManager }) => {
   if (isExcalidrawPlusSignedUser) {
     subheadingJSX = t("welcomeScreen.switchToPlusApp")
       .split(/(Excalidraw\+)/)
-      .map((bit) => {
+      .map((bit, idx) => {
         if (bit === "Excalidraw+") {
           return (
             <a
               style={{ pointerEvents: "all" }}
               href={`${process.env.REACT_APP_PLUS_APP}?utm_source=excalidraw&utm_medium=app&utm_content=welcomeScreenSignedInUser`}
+              key={idx}
             >
               Excalidraw+
             </a>
@@ -94,15 +102,17 @@ const WelcomeScreen = ({ actionManager }: { actionManager: ActionManager }) => {
         {subheadingJSX}
       </div>
       <div className="WelcomeScreen-items">
-        <WelcomeScreenItem
-          // TODO barnabasmolnar/editor-redesign
-          // do we want the internationalized labels here that are currently
-          // in use elsewhere or new ones?
-          label={t("buttons.load")}
-          onClick={() => actionManager.executeAction(actionLoadScene)}
-          shortcut={getShortcutFromShortcutName("loadScene")}
-          icon={LoadIcon}
-        />
+        {!appState.viewModeEnabled && (
+          <WelcomeScreenItem
+            // TODO barnabasmolnar/editor-redesign
+            // do we want the internationalized labels here that are currently
+            // in use elsewhere or new ones?
+            label={t("buttons.load")}
+            onClick={() => actionManager.executeAction(actionLoadScene)}
+            shortcut={getShortcutFromShortcutName("loadScene")}
+            icon={LoadIcon}
+          />
+        )}
         <WelcomeScreenItem
           label={t("labels.liveCollaboration")}
           shortcut={null}

src/hooks/useOutsideClick.ts

@@ -21,10 +21,11 @@ export const useOutsideClickHook = (handler: (event: Event) => void) => {
 
         handler(event);
       };
-      document.addEventListener("mousedown", listener);
+
+      document.addEventListener("pointerdown", listener);
       document.addEventListener("touchstart", listener);
       return () => {
-        document.removeEventListener("mousedown", listener);
+        document.removeEventListener("pointerdown", listener);
         document.removeEventListener("touchstart", listener);
       };
     },

src/tests/queries/toolQueries.ts

@@ -1,6 +1,7 @@
 import { queries, buildQueries } from "@testing-library/react";
 
 const toolMap = {
+  lock: "lock",
   selection: "selection",
   rectangle: "rectangle",
   diamond: "diamond",
@@ -15,7 +16,7 @@ export type ToolName = keyof typeof toolMap;
 
 const _getAllByToolName = (container: HTMLElement, tool: string) => {
   const toolTitle = toolMap[tool as ToolName];
-  return queries.getAllByTestId(container, toolTitle);
+  return queries.getAllByTestId(container, `toolbar-${toolTitle}`);
 };
 
 const getMultipleError = (_container: any, tool: any) =>

src/tests/selection.test.tsx

@@ -11,7 +11,8 @@ import * as Renderer from "../renderer/renderScene";
 import { KEYS } from "../keys";
 import { reseed } from "../random";
 import { API } from "./helpers/api";
-import { Keyboard, Pointer } from "./helpers/ui";
+import { Keyboard, Pointer, UI } from "./helpers/ui";
+import { SHAPES } from "../shapes";
 
 // Unmount ReactDOM from root
 ReactDOM.unmountComponentAtNode(document.getElementById("root")!);
@@ -380,3 +381,19 @@ describe("select single element on the scene", () => {
     h.elements.forEach((element) => expect(element).toMatchSnapshot());
   });
 });
+
+describe("tool locking & selection", () => {
+  it("should not select newly created element while tool is locked", async () => {
+    await render(<ExcalidrawApp />);
+
+    UI.clickTool("lock");
+    expect(h.state.activeTool.locked).toBe(true);
+
+    for (const { value } of Object.values(SHAPES)) {
+      if (value !== "image" && value !== "selection") {
+        const element = UI.createElement(value);
+        expect(h.state.selectedElementIds[element.id]).not.toBe(true);
+      }
+    }
+  });
+});