Browse Source

fix: mobile UI and other fixes (#10177)

* remove legacy openMenu=shape state and unused actions

* close menus/popups in applicable cases when opening a different one

* split ui z-indexes to account prefer different overlap

* make top canvas area clickable on mobile

* make mobile main menu closable by clicking outside and reduce width

* offset picker popups from viewport border on mobile

* reduce items gap in mobile main menu

* show top picks for canvas bg colors in all ui modes

* fix menu separator visibility on mobile

* fix command palette items not being filtered
David Luzar 5 days ago
parent
commit
5fffc4743f

+ 3 - 55
packages/excalidraw/actions/actionMenu.tsx

@@ -1,65 +1,11 @@
 import { KEYS } from "@excalidraw/common";
 
-import { getNonDeletedElements } from "@excalidraw/element";
-
-import { showSelectedShapeActions } from "@excalidraw/element";
-
 import { CaptureUpdateAction } from "@excalidraw/element";
 
-import { ToolButton } from "../components/ToolButton";
-import { HamburgerMenuIcon, HelpIconThin, palette } from "../components/icons";
-import { t } from "../i18n";
+import { HelpIconThin } from "../components/icons";
 
 import { register } from "./register";
 
-export const actionToggleCanvasMenu = register({
-  name: "toggleCanvasMenu",
-  label: "buttons.menu",
-  trackEvent: { category: "menu" },
-  perform: (_, appState) => ({
-    appState: {
-      ...appState,
-      openMenu: appState.openMenu === "canvas" ? null : "canvas",
-    },
-    captureUpdate: CaptureUpdateAction.EVENTUALLY,
-  }),
-  PanelComponent: ({ appState, updateData }) => (
-    <ToolButton
-      type="button"
-      icon={HamburgerMenuIcon}
-      aria-label={t("buttons.menu")}
-      onClick={updateData}
-      selected={appState.openMenu === "canvas"}
-    />
-  ),
-});
-
-export const actionToggleEditMenu = register({
-  name: "toggleEditMenu",
-  label: "buttons.edit",
-  trackEvent: { category: "menu" },
-  perform: (_elements, appState) => ({
-    appState: {
-      ...appState,
-      openMenu: appState.openMenu === "shape" ? null : "shape",
-    },
-    captureUpdate: CaptureUpdateAction.EVENTUALLY,
-  }),
-  PanelComponent: ({ elements, appState, updateData }) => (
-    <ToolButton
-      visible={showSelectedShapeActions(
-        appState,
-        getNonDeletedElements(elements),
-      )}
-      type="button"
-      icon={palette}
-      aria-label={t("buttons.edit")}
-      onClick={updateData}
-      selected={appState.openMenu === "shape"}
-    />
-  ),
-});
-
 export const actionShortcuts = register({
   name: "toggleShortcuts",
   label: "welcomeScreen.defaults.helpHint",
@@ -79,6 +25,8 @@ export const actionShortcuts = register({
             : {
                 name: "help",
               },
+        openMenu: null,
+        openPopup: null,
       },
       captureUpdate: CaptureUpdateAction.EVENTUALLY,
     };

+ 1 - 5
packages/excalidraw/actions/index.ts

@@ -44,11 +44,7 @@ export {
 } from "./actionExport";
 
 export { actionCopyStyles, actionPasteStyles } from "./actionStyles";
-export {
-  actionToggleCanvasMenu,
-  actionToggleEditMenu,
-  actionShortcuts,
-} from "./actionMenu";
+export { actionShortcuts } from "./actionMenu";
 
 export { actionGroup, actionUngroup } from "./actionGroup";
 

+ 0 - 2
packages/excalidraw/actions/types.ts

@@ -72,8 +72,6 @@ export type ActionName =
   | "changeArrowProperties"
   | "changeOpacity"
   | "changeFontSize"
-  | "toggleCanvasMenu"
-  | "toggleEditMenu"
   | "undo"
   | "redo"
   | "finalize"

+ 4 - 1
packages/excalidraw/components/Actions.tsx

@@ -1178,7 +1178,10 @@ export const ShapesSwitcher = ({
               // on top of it
               (laserToolSelected && !app.props.isCollaborating),
           })}
-          onToggle={() => setIsExtraToolsMenuOpen(!isExtraToolsMenuOpen)}
+          onToggle={() => {
+            setIsExtraToolsMenuOpen(!isExtraToolsMenuOpen);
+            setAppState({ openMenu: null, openPopup: null });
+          }}
           title={t("toolBar.extraTools")}
         >
           {frameToolSelected

+ 3 - 2
packages/excalidraw/components/ColorPicker/ColorPicker.tsx

@@ -319,8 +319,9 @@ export const ColorPicker = ({
     openRef.current = appState.openPopup;
   }, [appState.openPopup]);
   const compactMode =
-    appState.stylesPanelMode === "compact" ||
-    appState.stylesPanelMode === "mobile";
+    type !== "canvasBackground" &&
+    (appState.stylesPanelMode === "compact" ||
+      appState.stylesPanelMode === "mobile");
 
   return (
     <div>

+ 6 - 3
packages/excalidraw/components/CommandPalette/CommandPalette.tsx

@@ -476,7 +476,6 @@ function CommandPaletteInner({
           },
           perform: () => {
             setAppState((prevState) => ({
-              openMenu: prevState.openMenu === "shape" ? null : "shape",
               openPopup: "elementStroke",
             }));
           },
@@ -496,7 +495,6 @@ function CommandPaletteInner({
           },
           perform: () => {
             setAppState((prevState) => ({
-              openMenu: prevState.openMenu === "shape" ? null : "shape",
               openPopup: "elementBackground",
             }));
           },
@@ -838,7 +836,12 @@ function CommandPaletteInner({
 
     let matchingCommands =
       commandSearch?.length > 1
-        ? [...allCommands, ...libraryCommands]
+        ? [
+            ...allCommands
+              .filter(isCommandAvailable)
+              .sort((a, b) => a.order - b.order),
+            ...libraryCommands,
+          ]
         : allCommands
             .filter(isCommandAvailable)
             .sort((a, b) => a.order - b.order);

+ 1 - 1
packages/excalidraw/components/IconPicker.tsx

@@ -159,7 +159,7 @@ function Picker<T>({
       side={isMobile ? "right" : "bottom"}
       align="start"
       sideOffset={isMobile ? 8 : 12}
-      style={{ zIndex: "var(--zIndex-popup)" }}
+      style={{ zIndex: "var(--zIndex-ui-styles-popup)" }}
       onKeyDown={handleKeyDown}
     >
       <div

+ 1 - 0
packages/excalidraw/components/LibraryMenu.scss

@@ -133,6 +133,7 @@
   }
 
   .layer-ui__library .library-menu-dropdown-container {
+    z-index: 1;
     position: relative;
     &--in-heading {
       margin-left: auto;

+ 4 - 1
packages/excalidraw/components/MobileToolBar.tsx

@@ -374,7 +374,10 @@ export const MobileToolBar = ({
                 extraToolSelected || isOtherShapesMenuOpen,
             },
           )}
-          onToggle={() => setIsOtherShapesMenuOpen(!isOtherShapesMenuOpen)}
+          onToggle={() => {
+            setIsOtherShapesMenuOpen(!isOtherShapesMenuOpen);
+            setAppState({ openMenu: null, openPopup: null });
+          }}
           title={t("toolBar.extraTools")}
           style={{
             width: WIDTH,

+ 2 - 1
packages/excalidraw/components/PropertiesPopover.tsx

@@ -60,7 +60,8 @@ export const PropertiesPopover = React.forwardRef<
           alignOffset={-16}
           sideOffset={20}
           style={{
-            zIndex: "var(--zIndex-popup)",
+            zIndex: "var(--zIndex-ui-styles-popup)",
+            marginLeft: device.editor.isMobile ? "0.5rem" : undefined,
           }}
           onPointerLeave={onPointerLeave}
           onKeyDown={onKeyDown}

+ 1 - 1
packages/excalidraw/components/Sidebar/Sidebar.scss

@@ -9,7 +9,7 @@
     top: 0;
     bottom: 0;
     right: 0;
-    z-index: 5;
+    z-index: var(--zIndex-ui-library);
     margin: 0;
     padding: 0;
     box-sizing: border-box;

+ 5 - 1
packages/excalidraw/components/Sidebar/SidebarTrigger.tsx

@@ -30,7 +30,11 @@ export const SidebarTrigger = ({
             .querySelector(".layer-ui__wrapper")
             ?.classList.remove("animate");
           const isOpen = event.target.checked;
-          setAppState({ openSidebar: isOpen ? { name, tab } : null });
+          setAppState({
+            openSidebar: isOpen ? { name, tab } : null,
+            openMenu: null,
+            openPopup: null,
+          });
           onToggle?.(isOpen);
         }}
         checked={appState.openSidebar?.name === name}

+ 1 - 2
packages/excalidraw/components/dropdownMenu/DropdownMenu.scss

@@ -5,6 +5,7 @@
     position: absolute;
     top: 2.5rem;
     margin-top: 0.5rem;
+    max-width: 16rem;
 
     &--placement-top {
       top: auto;
@@ -20,10 +21,8 @@
       // When main menu is in the top toolbar, position relative to trigger
       &.main-menu-dropdown {
         min-width: 232px;
-        max-width: calc(100vw - var(--editor-container-padding) * 2);
         margin-top: 0;
         margin-bottom: 0;
-        z-index: var(--zIndex-layerUI);
 
         @media screen and (orientation: landscape) {
           max-width: 232px;

+ 6 - 1
packages/excalidraw/components/dropdownMenu/DropdownMenuContent.tsx

@@ -74,7 +74,12 @@ const MenuContent = ({
         {/* the zIndex ensures this menu has higher stacking order,
     see https://github.com/excalidraw/excalidraw/pull/1445 */}
         {device.editor.isMobile ? (
-          <Stack.Col className="dropdown-menu-container">{children}</Stack.Col>
+          <Stack.Col
+            className="dropdown-menu-container"
+            style={{ ["--gap" as any]: 1.25 }}
+          >
+            {children}
+          </Stack.Col>
         ) : (
           <Island
             className="dropdown-menu-container"

+ 1 - 0
packages/excalidraw/components/dropdownMenu/DropdownMenuSeparator.tsx

@@ -6,6 +6,7 @@ const MenuSeparator = () => (
       height: "1px",
       backgroundColor: "var(--default-border-color)",
       margin: ".5rem 0",
+      flex: "0 0 auto",
     }}
   />
 );

+ 3 - 4
packages/excalidraw/components/main-menu/MainMenu.tsx

@@ -30,9 +30,6 @@ const MainMenu = Object.assign(
       const device = useDevice();
       const appState = useUIAppState();
       const setAppState = useExcalidrawSetAppState();
-      const onClickOutside = device.editor.isMobile
-        ? undefined
-        : () => setAppState({ openMenu: null });
 
       return (
         <MainMenuTunnel.In>
@@ -41,6 +38,8 @@ const MainMenu = Object.assign(
               onToggle={() => {
                 setAppState({
                   openMenu: appState.openMenu === "canvas" ? null : "canvas",
+                  openPopup: null,
+                  openDialog: null,
                 });
               }}
               data-testid="main-menu-trigger"
@@ -49,7 +48,7 @@ const MainMenu = Object.assign(
               {HamburgerMenuIcon}
             </DropdownMenu.Trigger>
             <DropdownMenu.Content
-              onClickOutside={onClickOutside}
+              onClickOutside={() => setAppState({ openMenu: null })}
               onSelect={composeEventHandlers(onSelect, () => {
                 setAppState({ openMenu: null });
               })}

+ 13 - 2
packages/excalidraw/css/styles.scss

@@ -12,6 +12,11 @@
   --zIndex-eyeDropperPreview: 6;
   --zIndex-hyperlinkContainer: 7;
 
+  --zIndex-ui-styles-popup: 40;
+  --zIndex-ui-bottom: 60;
+  --zIndex-ui-library: 80;
+  --zIndex-ui-top: 100;
+
   --zIndex-modal: 1000;
   --zIndex-popup: 1001;
   --zIndex-toast: 999999;
@@ -237,7 +242,7 @@ body.excalidraw-cursor-resize * {
   }
 
   .App-top-bar {
-    z-index: var(--zIndex-layerUI);
+    z-index: var(--zIndex-ui-top);
     display: flex;
     flex-direction: column;
   }
@@ -255,7 +260,7 @@ body.excalidraw-cursor-resize * {
     left: 50%;
     transform: translateX(-50%);
     --bar-padding: calc(4 * var(--space-factor));
-    z-index: var(--zIndex-layerUI);
+    z-index: var(--zIndex-ui-bottom);
     display: flex;
     flex-direction: column;
 
@@ -296,6 +301,12 @@ body.excalidraw-cursor-resize * {
   .App-toolbar-content {
     display: flex;
     flex-direction: column;
+
+    pointer-events: none;
+
+    & > * {
+      pointer-events: var(--ui-pointerEvents);
+    }
   }
 
   .App-mobile-menu {

+ 2 - 2
packages/excalidraw/tests/__snapshots__/excalidraw.test.tsx.snap

@@ -414,7 +414,7 @@ exports[`<Excalidraw/> > Test UIOptions prop > Test canvasActions > should rende
       </div>
     </button>
     <div
-      style="height: 1px; margin: .5rem 0px;"
+      style="height: 1px; margin: .5rem 0px; flex: 0 0 auto;"
     />
     <div
       class="dropdown-menu-group "
@@ -558,7 +558,7 @@ exports[`<Excalidraw/> > Test UIOptions prop > Test canvasActions > should rende
       </a>
     </div>
     <div
-      style="height: 1px; margin: .5rem 0px;"
+      style="height: 1px; margin: .5rem 0px; flex: 0 0 auto;"
     />
     <button
       aria-label="Dark mode"

+ 1 - 1
packages/excalidraw/types.ts

@@ -350,7 +350,7 @@ export interface AppState {
   isResizing: boolean;
   isRotating: boolean;
   zoom: Zoom;
-  openMenu: "canvas" | "shape" | null;
+  openMenu: "canvas" | null;
   openPopup:
     | "canvasBackground"
     | "elementBackground"