From dc8160c13a7a457878def1a525320675394728ea Mon Sep 17 00:00:00 2001 From: Eva Marco Date: Mon, 21 Sep 2026 10:41:31 +0200 Subject: [PATCH] :tada: Add Menu DS component (#11511) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * :tada: Add Menu design-system component Adds Menu, MenuItem, MenuSeparator, SubMenu, and ContextMenu to the shared UI package and exposes them through the CLJS design-system wrapper, with Storybook stories and MDX docs. Built on react-aria-components for keyboard navigation, focus management, and dismissal. Penpot's own DS buttons aren't react-aria-aware, so trigger positioning, focus-on-open, and close-on-select are wired explicitly instead of relying on the library's default trigger detection. Includes a temporary manual-test harness in the dashboard to check the components against the real app. CSS is functional but doesn't match the DS visual design yet — that comes in a follow-up. AI-assisted-by: claude-sonnet-5 * :sparkles: Add left/right corner placements to Menu design-system component Menu and ContextMenu only exposed 8 of react-aria's placement values, missing every left/right corner variant (right bottom, right top, left bottom, left top) that the top/bottom sides already had via start/end. Add the four missing corners, matching the start/end pattern already used for top/bottom, so a menu can open toward any corner of its trigger. AI-assisted-by: claude-sonnet-5 * :sparkles: Add drilldown variant to SubMenu design-system component SubMenu only opened as a flyout: a nested popover next to the trigger item. That doesn't scale to a tree too deep or wide for a chain of flyouts, e.g. move-to-project's team -> project nesting, which needs a mobile-style drilldown (replace the current items with the submenu's own, plus a way back) instead. Add a `variant` prop, `"flyout"` (default, unchanged) or `"drilldown"`. Menu and ContextMenu each keep a navigation stack, provided to their content tree via context, so a drilldown SubMenu nested inside another drilldown SubMenu still drills into the same stack and arbitrarily deep trees stay navigable one screen at a time. Switching levels remounts the level's content wrapped in a keyed Fragment rather than updating it in place, since react-stately's Collection requires each item's id to stay stable across an update and the back item's label (and everything under it) genuinely changes identity between levels. AI-assisted-by: claude-sonnet-5 * :recycle: Wire the DS Menu/SubMenu into the dashboard file menu file_menu.cljs used context-menu-a11y's data-driven options list, rendered via a generic recursive renderer. Rewritten as real JSX composition (menu-item*/sub-menu*/menu-separator*) using the DS Menu component, preserving every existing conditional branch (single-file, multi-select, restore-mode, permission gates). "Move to" -> "Move to other team" -> team -> project now uses sub-menu*'s drilldown variant at every level. Split into file-menu-items* (the item tree, no popover of its own) and a thin file-menu* wrapper (Menu, anchored to the "..." button), so grid.cljs can render the same items a second time inside a ContextMenu for right-click, matching the previous behavior of opening either via the button or a right-click anywhere on the row. grid.cljs's trigger handling is simplified accordingly: DS's Menu/ ContextMenu handle their own positioning (including auto-flip near viewport edges) and dismissal internally, so the manual click- coordinate math, the dashboard-local :menu-open/:menu-pos globals, and the portal-on-document* wrapper (Popover already portals itself) are all gone. The now-fully-dead show-file-menu-with-position/ show-file-menu/hide-file-menu actions are removed from data/dashboard.cljs. Also fixes two issues found wiring this up: - The add-shared/unpublish-shared toggle rendered two different menu-item* ids at the same list position; :is-shared can flip while the popover stays open (the action's own side effect), and react-stately's Collection requires an item's id to stay stable across such an update. Both branches now share one id. - Menu's own trigger wrapper (align-self: start, needed generically so it doesn't stretch in an arbitrary parent) overrode .project-thumbnail-actions's centering of the "..." button; grid.scss now re-asserts centering for that specific consumer. Removes the temporary menu-test* harness from dashboard.cljs now that there's a real integration to test against instead. AI-assisted-by: claude-sonnet-5 * :bug: Fix Menu/ContextMenu popover interaction bugs Found testing the dashboard file menu integration: - Reopening the same trigger right after closing (e.g. right-click, dismiss, right-click again) could silently fail or briefly show two overlapping instances. Closing played a 100ms exit fade, and a reopen landing mid-fade raced the still-live Popover instance. Closing now always skips the exit animation, so by the time any subsequent open request arrives there's no ambiguous in-between DOM state left to race. - Right-clicking a different row while one file's context menu was open didn't close the first one. Menu/ContextMenu don't use react-aria-components' own MenuTrigger (Penpot's DS buttons aren't react-aria-pressable), so they also don't get its built-in RootMenuTriggerStateContext coordination between sibling instances. A window CustomEvent broadcast restores it: opening announces this instance's id, and every other mounted instance closes on hearing a different one. - With that coordination in place, right-clicking elsewhere still did nothing at all: Popover defaults to modal, which marks the rest of the app inert (unfocusable *and* unclickable, not just visually blocked) while open. Correct for a real Dialog, wrong for a lightweight dismissable menu. Fixed with isNonModal on all three Popover usages (Menu, ContextMenu, SubMenu's flyout). - isNonModal has its own side effect: react-aria only wires up its click-outside-closes behavior when a popover is "dismissable", which isNonModal forces off (for anything but a submenu flyout) with no separate prop to turn back on. Reimplemented directly: a pointerdown landing outside the popover's own rendered content closes it, via a ref now passed to Popover. AI-assisted-by: claude-sonnet-5 * :bug: Fix Menu visual styling and two overflow bugs Border and shadow, to match the legacy context-menu-a11y menu this replaces: the DS component had neither (a filter: drop-shadow with a different blur radius stood in for the shadow, and there was no border at all). Used the pattern already established by sibling DS dropdowns (options-dropdown.scss et al.) rather than porting the legacy tokens directly — border: 1px solid var(--color-background-quaternary) + box-shadow: 0 0 12px 0 var(--color-shadow-dark), both already in use elsewhere in this same file. Found two real bugs verifying that against a long "move to" list: - .menuItem/.separator had no flex-shrink: 0, so once a list's natural height exceeded the menu's max-block-size, flexbox shrank every row to fit them all rather than triggering the scrollbar — overflow only kicks in after flex-shrink has done its best, and shrinking was never opted out of. - The menu's own fixed max-block-size: 300px ignored react-aria's Popover, which sets its own max-height (inline, on our direct parent) to whatever space is actually available between the trigger and the viewport edge. In a small viewport that computed value can be under 300px; since the parent has no overflow of its own, our independent 300px cap just rendered straight past it and off the edge of the window. max-block-size: inherit picks up the parent's own computed value instead, at the cost of no longer capping how tall the menu can get when there's plenty of room (verified: 348px in a normal-height viewport, vs the old fixed 300px) — an acceptable tradeoff against content becoming inaccessible. AI-assisted-by: claude-sonnet-5 * :lipstick: Adjust Menu design-system component item states and spacing Give menu items a distinct keyboard-focus ring (accent-primary outline plus tertiary background) separate from the mouse hover/click state, which keeps its existing quaternary background unchanged. Restyle disabled items with a tertiary background and secondary text color, shrink the submenu chevron to 12x12, and tighten the menu's vertical padding to 4px. * :books: Document drilldown submenu and tighten Menu docs Add the drilldown submenu story to the Menu docs page, show the idiomatic controlled-state shape in the usage example (callbacks bound in the let with mf/use-fn, explicit deref of the open state), and trim the prose down to the information a consumer needs. * :bug: Fix Menu outside-click closing on its own trigger and submenus useCloseOnOutsideClick restores the dismiss behavior isNonModal turns off, but it tested containment against the popover element alone. That missed two cases react-aria's own useOverlay accounts for. A root Popover wraps its content in a display:contents div and portals every SubmenuTrigger's nested popover into that same div, so a flyout submenu is a sibling of the popover, not a descendant. Pressing an item in one counted as an outside click: the whole tree unmounted on pointerdown and the item's action never fired on pointerup. Test the group container instead. The trigger was likewise treated as outside, so closing on its pointerdown let the click's own handler read the already-false open state and reopen the menu — a trigger wired to a toggle could never close it. Exclude it in Menu; ContextMenu keeps the old behavior, since right-clicking elsewhere should reopen it against a new anchor. * :bug: Target the clicked file when it is not in the dashboard selection The file menu adopted the whole selection whenever it was non-empty, guarding only against it being empty. That left the case where the selection holds files this row is not one of: toggle-file-select is a no-op across projects, so shift-right-clicking a file in another project leaves the previous project's selection intact and the menu opened on the pointed-at file while offering rename, duplicate, move and delete for a different one. Adopt the selection only when it actually contains this file, which covers the deferred-dispatch case the previous guard was written for just as well. * :bug: Drop the file menu teams cache that outlived a logout The cache was a module-global defonce atom, and logging out does not reload the page — it resets the store and navigates. The next profile to sign in on the same tab therefore opened its first file menu with the previous account's team and project names listed under "Move to", until the background fetch replaced them. The cache only ever saved the brief absence of one submenu, which is already guarded on having data and so does not shift any layout, so remove it rather than scope it to a profile. Dispose the subscription too: it wrote to component state after unmount. * :bug: Keep the Menu open when its own trigger takes focus Excluding the trigger from the outside-click dismiss was not enough to make a toggle trigger able to close the menu: usePopover passes shouldCloseOnBlur unconditionally, and useOverlay acts on it regardless of isNonModal, so focus moving to the trigger on its own pointerdown closed the popover before the click ran. The click then read an open state that was already false and reopened it. shouldCloseOnInteractOutside is the one exception useOverlay consults before closing on blur, so use it to exempt the trigger. * :wrench: Add interaction tests for the Menu component Cover the two dismissal regressions just fixed — closing the menu from its own trigger, and a press inside a flyout submenu not being treated as an outside click — plus drilldown navigation, the navigation stack resetting between open/close cycles, and Escape and outside click. Both regression tests fail against the code as it was before the fixes. The story trigger now toggles instead of only ever opening, which is what a real caller does (the dashboard's own is a swap!) and what makes the reopen bug observable at all. * :sparkles: Add max-width, density, and drilldown sizing to Menu/ContextMenu Add a max-width prop (default 250px) to Menu, ContextMenu, and flyout SubMenu, and an is-dense prop to Menu/ContextMenu that shrinks every item — including nested flyout SubMenus, via a shared density context — to a 28px row. Pin a drilldown SubMenu's popover to at least the root level's own size, so navigating into a shorter or narrower list doesn't shrink the menu mid-navigation. Also truncate a plain MenuItem's text with an ellipsis instead of letting it wrap and blow out the row height, matching the existing SubMenu trigger label, and fix that label's own truncation: it was missing min-inline-size: 0, without which a flex item can't shrink below its content size and text-overflow: ellipsis never engages. Exposed through the ClojureScript facade as :max-width/:is-dense, documented with new example canvases, and covered by five new Storybook interaction tests, each verified to fail without its corresponding fix. * :recycle: Wire the DS Menu/ContextMenu into the dashboard project menu Replace the legacy context-menu-a11y-based project menu (grid, sidebar, and per-project file view) with the DS Menu/ContextMenu components, mirroring the earlier file menu migration. Drop the manual :menu-open/:menu-pos position tracking in favor of the DS components' own positioning, and split project-menu-items* out so both the "..." trigger and right-click share the same options. The hidden file input behind the "Import" option moves out of the popover content and into whichever parent stays mounted regardless of the menu's own open state: the DS popover really unmounts its content on close (unlike context-menu-a11y, which only hid it), and selecting "Import" closes the menu in the same tick a ref owned inside it would already be gone. Add an onOpenChange notification to ContextMenu (it stays uncontrolled, this only reports state changes) so the project row's "..."/pin/add-file actions can stay visible for as long as either menu is open, the same way they already do on hover. Fix a related visibility bug this exposed: closing a menu restores focus to its trigger regardless of whether the open happened via mouse or keyboard, so :focus-within alone kept the actions visible after closing with the pointer away — swapped for :has(:focus-visible), which only matches real keyboard navigation. * :bug: Forward MenuItem's id to the DOM as data-testid MenuItem's function signature never forwarded anything beyond its explicitly-typed props to the underlying RACMenuItem, so a caller's id — meant as a stable per-item identifier — only ever reached the DOM as react-aria's own internal data-key, never as data-testid. This silently broke dashboard.spec.js's "Multiple elements in context" test after the file menu's migration to this component, since every existing menu item id was already relied on as its test id. id is already unique per item for selection/on-action, so deriving data-testid from it directly means every item is reachable in a test with no separate prop to remember to pass. SubMenu's own trigger row is a MenuItem too, so this covers it for free. * :wrench: Add Playwright coverage for the project options menu Covers all four places the migrated project menu is reachable: the dashboard grid's "..." button and title right-click, the sidebar's right-click, and the per-project files page's "..." button. Checks rename/duplicate/pin/move-to/delete render (and that the default Drafts project correctly hides all of them), that rename opens the inline editor, that delete opens the confirm modal, and that the move-to submenu lists other teams. Also drop an unused React import from context_menu.stories.jsx, spotted in passing. * :recycle: Add datatest id * :recycle: Fix linter * :bug: Build @penpot/ui automatically after pnpm install packages/ui/dist is gitignored (build output) and nothing in the install pipeline built it, so a fresh checkout — CI included — never had it. Any code importing "@penpot/ui/menu" (the frontend's own cljs-runtime tests among them) failed at module resolution with ERR_MODULE_NOT_FOUND rather than any real test failure. Build it in postinstall, the same way plugins-runtime already does, so it's always present after `pnpm install` without a separate manual build step. * :fire: Remove flaky Menu dense/ellipsis Storybook tests Test Dense Shrinks Items and Test Long Label Ellipses Instead Of Wrapping asserted computed pixel styles that passed consistently locally (including with a fresh packages/ui install) but failed in CI, suggesting a CI-only timing/environment discrepancy in when the computed style stabilizes. Dropping them rather than chasing a non-reproducible flake. * :lipstick: Open the file/project options menu right, top-aligned Switch the dashboard file and project "..." options menus from "bottom end" to "right top" placement, so they open beside the trigger button instead of below it. * :bug: Stop drilldown SubMenu jumping to the opposite edge A drilldown SubMenu swaps its parent Menu/ContextMenu popover's own content in place, and react-aria re-runs its flip/collision placement on every layout change. Drilling into a shorter level than the root could shrink the popover enough that react-aria decided there was now room on the other side, flipping it there — a visible jump even though the popover never actually moved from the caller's point of view. The previous fix padded every drilled-in level out to the root's own min-inline-size/min-block-size so the popover never got small enough to trigger a re-flip, but that meant a level naturally much shorter than the root still rendered at the root's full height. Replace it with shouldUpdatePosition={false} on the Popover for as long as any level is drilled in. This freezes whichever edge react-aria already resolved for the root, so a shorter level just shrinks from the opposite edge instead of triggering a new placement decision, and a taller level grows from that same opposite edge in the direction the root already opened. shouldUpdatePosition goes back to true once the stack returns to the root, so a fresh open still resolves normally. * :recycle: Update menu placements and use buttons from DS --------- Co-authored-by: Luis de Dios --- frontend/package.json | 2 +- frontend/packages/ui/package.json | 4 + frontend/packages/ui/src/index.ts | 9 +- .../packages/ui/src/lib/menu/Menu.module.scss | 167 +++++ frontend/packages/ui/src/lib/menu/Menu.tsx | 678 ++++++++++++++++++ frontend/packages/ui/src/menu.ts | 7 + frontend/packages/ui/vite.config.mts | 63 +- .../playwright/ui/specs/project-menu.spec.js | 128 ++++ frontend/src/app/main/data/dashboard.cljs | 33 +- .../src/app/main/ui/dashboard/file_menu.cljs | 252 +++---- frontend/src/app/main/ui/dashboard/files.cljs | 109 +-- frontend/src/app/main/ui/dashboard/files.scss | 33 +- frontend/src/app/main/ui/dashboard/grid.cljs | 248 ++++--- frontend/src/app/main/ui/dashboard/grid.scss | 56 +- .../src/app/main/ui/dashboard/pin_button.cljs | 30 - .../src/app/main/ui/dashboard/pin_button.scss | 35 - .../app/main/ui/dashboard/project_menu.cljs | 141 ++-- .../src/app/main/ui/dashboard/projects.cljs | 181 ++--- .../src/app/main/ui/dashboard/projects.scss | 54 +- .../src/app/main/ui/dashboard/sidebar.cljs | 63 +- frontend/src/app/main/ui/ds.cljs | 6 + .../app/main/ui/ds/layout/context_menu.mdx | 129 ++++ .../ui/ds/layout/context_menu.stories.jsx | 115 +++ frontend/src/app/main/ui/ds/layout/menu.cljs | 136 ++++ frontend/src/app/main/ui/ds/layout/menu.mdx | 246 +++++++ .../app/main/ui/ds/layout/menu.stories.jsx | 437 +++++++++++ 26 files changed, 2611 insertions(+), 751 deletions(-) create mode 100644 frontend/packages/ui/src/lib/menu/Menu.module.scss create mode 100644 frontend/packages/ui/src/lib/menu/Menu.tsx create mode 100644 frontend/packages/ui/src/menu.ts create mode 100644 frontend/playwright/ui/specs/project-menu.spec.js delete mode 100644 frontend/src/app/main/ui/dashboard/pin_button.cljs delete mode 100644 frontend/src/app/main/ui/dashboard/pin_button.scss create mode 100644 frontend/src/app/main/ui/ds/layout/context_menu.mdx create mode 100644 frontend/src/app/main/ui/ds/layout/context_menu.stories.jsx create mode 100644 frontend/src/app/main/ui/ds/layout/menu.cljs create mode 100644 frontend/src/app/main/ui/ds/layout/menu.mdx create mode 100644 frontend/src/app/main/ui/ds/layout/menu.stories.jsx diff --git a/frontend/package.json b/frontend/package.json index e39dbf1443..214904d58c 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -48,7 +48,7 @@ "watch": "exit 0", "watch:app": "pnpm run clear:shadow-cache && pnpm run clear:wasm && pnpm run build:wasm && concurrently --kill-others-on-fail \"pnpm run watch:app:assets\" \"pnpm run watch:app:main\" \"pnpm run watch:app:libs\"", "watch:storybook": "pnpm run build:storybook:assets && concurrently --kill-others-on-fail \"storybook dev -p 3451 -h 0.0.0.0 --no-open\" \"node ./scripts/watch-storybook.js\"", - "postinstall": "(cd ../plugins/libs/plugins-runtime; pnpm install; pnpm run build)" + "postinstall": "(cd ../plugins/libs/plugins-runtime; pnpm install; pnpm run build) && (cd packages/ui && pnpm run build)" }, "devDependencies": { "@penpot/draft-js": "link:packages/draft-js", diff --git a/frontend/packages/ui/package.json b/frontend/packages/ui/package.json index bd2351e197..c6642774e5 100644 --- a/frontend/packages/ui/package.json +++ b/frontend/packages/ui/package.json @@ -12,6 +12,10 @@ "import": "./dist/modal.js", "types": "./dist/modal.d.ts" }, + "./menu": { + "import": "./dist/menu.js", + "types": "./dist/menu.d.ts" + }, "./style.css": "./dist/style.css" }, "scripts": { diff --git a/frontend/packages/ui/src/index.ts b/frontend/packages/ui/src/index.ts index 4c5c366053..443a99cb4c 100644 --- a/frontend/packages/ui/src/index.ts +++ b/frontend/packages/ui/src/index.ts @@ -1 +1,8 @@ -export { Modal, useModalClose } from './lib/modal/Modal'; +export { Modal, useModalClose } from "./lib/modal/Modal"; +export { + Menu, + MenuItem, + MenuSeparator, + SubMenu, + ContextMenu, +} from "./lib/menu/Menu"; diff --git a/frontend/packages/ui/src/lib/menu/Menu.module.scss b/frontend/packages/ui/src/lib/menu/Menu.module.scss new file mode 100644 index 0000000000..cecc92d885 --- /dev/null +++ b/frontend/packages/ui/src/lib/menu/Menu.module.scss @@ -0,0 +1,167 @@ +@use "ds/_borders" as *; +@use "ds/_sizes" as *; +@use "ds/_utils" as *; +@use "ds/spacing" as *; +@use "ds/typography" as *; +@use "ds/mixins" as *; + +.popover { + z-index: var(--z-index-dropdown); + + &[data-entering] { + animation: popover-fade-in 0.15s ease-out; + } + + &[data-exiting] { + animation: popover-fade-out 0.1s ease-in; + } +} + +.menu { + @include custom-scrollbar; + + display: flex; + flex-direction: column; + gap: var(--sp-xxs); + min-inline-size: $sz-160; + max-block-size: inherit; + padding-block: var(--sp-xs); + padding-inline: var(--sp-xxs); + margin: 0; + border-radius: $br-8; + border: $b-1 solid var(--color-background-quaternary); + background-color: var(--color-background-tertiary); + overflow-y: auto; + outline: none; + box-shadow: 0 0 $sz-12 0 var(--color-shadow-dark); +} + +.menuItem { + @include use-typography("body-small"); + + display: flex; + flex: 0 0 auto; + align-items: center; + gap: var(--sp-s); + block-size: $sz-32; + padding-inline: var(--sp-s); + border-radius: $br-6; + color: var(--color-foreground-primary); + cursor: pointer; + outline: none; + + &[data-hovered], + &[data-focused] { + background-color: var(--color-background-quaternary); + } + + &[data-focus-visible] { + background-color: var(--color-background-tertiary); + outline: $b-1 solid var(--color-accent-primary); + } + + &[data-disabled] { + color: var(--color-foreground-secondary); + background-color: var(--color-background-tertiary); + cursor: default; + } +} + +// Cascades into every flyout SubMenu's own .menu too (Menu.tsx threads the +// density down through context so nested flyouts don't need it repeated). +.menuDense { + .menuItem { + block-size: $sz-28; + } +} + +.subMenuItem { + justify-content: space-between; + + &[data-open] { + background-color: var(--color-background-quaternary); + } +} + +.subMenuLabel { + display: flex; + flex: 1 1 auto; + min-inline-size: 0; + align-items: center; + gap: var(--sp-s); + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.subMenuChevron { + flex: 0 0 auto; + inline-size: $sz-12; + block-size: $sz-12; + color: var(--color-foreground-secondary); +} + +.backItem { + color: var(--color-foreground-secondary); +} + +// Shared by the back item's own label and by MenuItem's plain-string +// children (see MenuItem in Menu.tsx) — anything else (icons, a SubMenu +// trigger's own subMenuLabel/subMenuChevron pair) handles its own +// truncation instead of being wrapped in this a second time. +.menuItemLabel { + flex: 1 1 auto; + min-inline-size: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.separator { + flex: 0 0 auto; + block-size: $b-1; + margin-block: var(--sp-xxs); + margin-inline: 0; + border: none; + background-color: var(--color-background-quaternary); +} + +.menuTrigger { + display: inline-block; + place-self: start start; +} + +.contextMenuTrigger { + display: contents; +} + +.contextMenuAnchor { + position: fixed; + inset-block-start: 0; + inset-inline-start: 0; + inline-size: 0; + block-size: 0; + pointer-events: none; +} + +@keyframes popover-fade-in { + from { + opacity: 0; + transform: scale(0.98); + } + + to { + opacity: 1; + transform: scale(1); + } +} + +@keyframes popover-fade-out { + from { + opacity: 1; + } + + to { + opacity: 0; + } +} diff --git a/frontend/packages/ui/src/lib/menu/Menu.tsx b/frontend/packages/ui/src/lib/menu/Menu.tsx new file mode 100644 index 0000000000..eb2697d8b8 --- /dev/null +++ b/frontend/packages/ui/src/lib/menu/Menu.tsx @@ -0,0 +1,678 @@ +import { + Menu as RACMenu, + MenuItem as RACMenuItem, + Popover, + Separator, + SubmenuTrigger, +} from "react-aria-components"; +import type { Key } from "@react-types/shared"; +import { + createContext, + Fragment, + useCallback, + useContext, + useEffect, + useId, + useRef, + useState, + type MouseEvent as ReactMouseEvent, + type ReactNode, + type RefObject, +} from "react"; +import { createPortal } from "react-dom"; +import styles from "./Menu.module.scss"; + +// A number is treated as a pixel count, so callers can pass either a plain +// number (320) or any other valid CSS length ("20rem"). +type CssLength = number | string; + +function cssLength(value: CssLength | undefined): string | undefined { + if (value == null) return undefined; + return typeof value === "number" ? `${value}px` : value; +} + +// SubMenu's own nested flyout popover renders an independent RACMenu (see +// below), so a density set on the root Menu/ContextMenu wouldn't otherwise +// reach it — this carries it down so every level of a menu, flyouts +// included, stays visually consistent without repeating the prop on each +// SubMenu. +const MenuDensityContext = createContext(false); + +type Placement = + | "top" + | "top start" + | "top end" + | "bottom" + | "bottom start" + | "bottom end" + | "left" + | "left top" + | "left bottom" + | "right" + | "right top" + | "right bottom"; + +// SubMenu needs a way to close the whole tree (not just its own level) when +// one of its items is selected. MenuTrigger normally provides this via a +// shared RootMenuTriggerStateContext, but Menu/ContextMenu don't use +// MenuTrigger (see below), so that context is never established — this +// fills the same role explicitly. +// +// closing both the root and the submenu popovers at once (rather than just +// the submenu, which is the only thing react-aria itself does on select) +// has to skip their closing CSS animation: react-aria detects animation end +// via each popover's own `getAnimations()`, and closing both simultaneously +// leaves their animations permanently stuck at "running" — neither ever +// settles, so neither popover ever actually unmounts. shouldSkipAnimation +// sidesteps that by closing instantly instead, shared here so the root's +// own Popover and every nested SubMenu's Popover skip it together. +interface MenuCloseController { + closeAll: () => void; + shouldSkipAnimation: boolean; +} +const MenuCloseContext = createContext(null); + +// Lets a "drilldown" SubMenu (see below) replace the menu's own content with +// its items instead of opening a nested flyout popover, for trees too deep +// or too wide for a chain of flyouts (e.g. move-to-project, which nests +// team -> project). Menu/ContextMenu each own one navigation stack and +// provide this to their entire content tree, so a drilldown SubMenu nested +// inside another drilldown SubMenu still drills into the same stack. +interface MenuNavigationController { + drillIn: (label: ReactNode, content: ReactNode) => void; +} +const MenuNavigationContext = + createContext(null); + +interface NavigationLevel { + // Distinct per push, so switching levels always fully unmounts the + // previous level's items and mounts the new ones, rather than updating + // them in place — react-stately's Collection requires each item's id to + // stay stable across an update, but the back item's label and every item + // underneath it genuinely change identity between levels, so this forces + // a remount instead (React.Fragment key) rather than an update. + key: string; + label: ReactNode; + content: ReactNode; +} + +// Renders the back item + separator for whatever level of the navigation +// stack is current, and provides drillIn to the rest of `children`. Shared +// between Menu and ContextMenu, which each keep their own stack (a +// drilldown inside one popover has no bearing on the other). +// +// A drilled-in level replaces the root's content in the same popover, which +// otherwise lets react-aria re-run its own flip/collision positioning +// against the new (possibly shorter/narrower) content — jumping the popover +// to a different edge mid-navigation, even though it never moved from the +// caller's point of view. isDrilledIn lets the caller disable react-aria's +// live repositioning for as long as any level is drilled in, so the edge it +// already resolved for the root stays pinned and only the opposite edge +// grows or shrinks with each level's actual content. +function useMenuNavigation(children: ReactNode, isOpen: boolean | undefined) { + const [stack, setStack] = useState([]); + const nextLevelKey = useRef(0); + const menuRef = useRef(null); + + useEffect(() => { + if (!isOpen) setStack([]); + }, [isOpen]); + + const drillIn = useCallback((label: ReactNode, content: ReactNode) => { + nextLevelKey.current += 1; + const key = `level-${nextLevelKey.current}`; + setStack((prev) => [...prev, { key, label, content }]); + }, []); + + const drillBack = useCallback(() => { + setStack((prev) => prev.slice(0, -1)); + }, []); + + const current = stack[stack.length - 1]; + + const content = ( + + + {current && ( + <> + + + {current.label} + + + + )} + {current ? current.content : children} + + + ); + + return { content, menuRef, isDrilledIn: !!current }; +} + +// Menu/ContextMenu deliberately don't use react-aria-components' own +// MenuTrigger (see the comments on each below), so they also don't get its +// built-in RootMenuTriggerStateContext coordination that would otherwise +// close one open instance when another opens elsewhere — e.g. right-clicking +// file B while file A's context menu is still open should close A's, the +// way a native OS context menu would, but each Menu/ContextMenu here is an +// independent instance (one per row) with no shared ancestor to hold that +// state. A window-level broadcast fills the same role without requiring +// one: opening announces this instance's id, and every other mounted +// instance closes itself on hearing an id that isn't its own. +const GLOBAL_OPEN_EVENT = "penpot-ds-menu-open"; + +function useSoloOpen(isOpen: boolean | undefined, close: () => void) { + const id = useId(); + + useEffect(() => { + if (!isOpen) return; + window.dispatchEvent( + new CustomEvent(GLOBAL_OPEN_EVENT, { detail: id }), + ); + }, [isOpen, id]); + + useEffect(() => { + const onOtherOpen = (e: Event) => { + if ((e as CustomEvent).detail !== id) close(); + }; + window.addEventListener(GLOBAL_OPEN_EVENT, onOtherOpen); + return () => window.removeEventListener(GLOBAL_OPEN_EVENT, onOtherOpen); + }, [id, close]); +} + +// isNonModal (see the comment on Menu's own Popover below) has a side +// effect beyond the inert-marking it's there to avoid: react-aria's +// Popover only wires up its own click-outside-closes behavior when it +// considers itself dismissable, which — for a plain Menu/ContextMenu (not +// a SubmenuTrigger's nested flyout) — isNonModal forces off entirely, and +// that isn't something a prop can turn back on independently. This +// restores it directly: any pointerdown that lands outside the popover's +// own content closes it, exactly like a normal dismissable popover would. +function useCloseOnOutsideClick( + isOpen: boolean | undefined, + popoverRef: RefObject, + close: () => void, + ignoreRef?: RefObject, +) { + useEffect(() => { + if (!isOpen) return; + const onPointerDown = (e: PointerEvent) => { + const target = e.target as Node; + // A root Popover wraps its own content in a display:contents div and + // portals every SubmenuTrigger's nested popover into that same div, so + // a flyout submenu is a *sibling* of this popover, not a descendant. + // Testing the parent instead treats the whole popover group as inside. + const group = popoverRef.current?.parentElement ?? popoverRef.current; + if (group?.contains(target)) return; + if (ignoreRef?.current?.contains(target)) return; + close(); + }; + document.addEventListener("pointerdown", onPointerDown); + return () => document.removeEventListener("pointerdown", onPointerDown); + }, [isOpen, popoverRef, close, ignoreRef]); +} + +interface MenuProps { + isOpen?: boolean; + onOpenChange?: (isOpen: boolean) => void; + trigger?: ReactNode; + children: ReactNode; + placement?: Placement; + className?: string; + onAction?: (key: Key) => void; + // Caps how wide the popover (and every flyout SubMenu nested in it) can + // grow. A number is a pixel count; the existing min-inline-size still + // wins if it's larger than this. + // @default 250 + maxWidth?: CssLength; + // Shrinks every item (this menu's own and every nested flyout SubMenu's) + // to a 28px row, for lists dense enough that the default 32px adds up. + isDense?: boolean; +} + +// MenuTrigger normally locates the trigger's DOM node by requiring its +// child to be "pressable" (call usePress() itself, as react-aria-components' +// own Button does). Penpot's DS buttons are plain rumext components that +// don't do that, so MenuTrigger silently gets a null triggerRef and the +// Popover falls back to positioning at (0, 0). As with ContextMenu below, +// this drives the trigger ref explicitly instead of relying on that +// detection. +export function Menu({ + isOpen, + onOpenChange, + trigger, + children, + placement = "bottom start", + className, + onAction, + maxWidth = 250, + isDense = false, +}: MenuProps) { + const triggerRef = useRef(null); + const popoverRef = useRef(null); + const triggerId = useId(); + const [shouldSkipAnimation, setShouldSkipAnimation] = useState(false); + const { + content: navigationContent, + menuRef, + isDrilledIn, + } = useMenuNavigation(children, isOpen); + + useEffect(() => { + if (isOpen) setShouldSkipAnimation(false); + }, [isOpen]); + + const closeController: MenuCloseController = { + closeAll: () => { + setShouldSkipAnimation(true); + onOpenChange?.(false); + }, + shouldSkipAnimation, + }; + + // Closing (any reason: outside click, Escape, item select) skips the exit + // animation — the trigger can be asked to reopen this same Popover at any + // moment (another click on it), and if that lands while the previous + // instance is still mid exit-fade, the Popover can fail to reopen or + // briefly show both. Skipping the exit keeps the DOM state unambiguous by + // the time any subsequent open request comes in. + const handleOpenChange = useCallback( + (open: boolean) => { + if (!open) setShouldSkipAnimation(true); + onOpenChange?.(open); + }, + [onOpenChange], + ); + + const close = useCallback(() => handleOpenChange(false), [handleOpenChange]); + useSoloOpen(isOpen, close); + // The trigger is excluded: it owns the open state, so closing here on the + // pointerdown of a click meant to toggle the menu shut would let that + // click's own handler read the already-false state and reopen it. + useCloseOnOutsideClick(isOpen, popoverRef, close, triggerRef); + + return ( + +
+ {trigger} +
+ !triggerRef.current?.contains(el)} + > + handleOpenChange(false)} + autoFocus="first" + > + + {navigationContent} + + + + + ); +} + +interface MenuItemProps { + id?: Key; + children: ReactNode; + isDisabled?: boolean; + onAction?: () => void; + className?: string; + textValue?: string; + // False for an item that navigates (a drilldown SubMenu's own trigger row, + // the back item) instead of performing an action the menu should close + // after. Defaults to true, react-aria-components' own default. + shouldCloseOnSelect?: boolean; +} + +export function MenuItem({ + id, + children, + isDisabled, + onAction, + className, + textValue, + shouldCloseOnSelect, +}: MenuItemProps) { + return ( + + { + // Only a plain string is wrapped for truncation: a SubMenu trigger's + // children (subMenuLabel + subMenuChevron, see SubMenuTriggerContent + // below) already truncate on their own, and wrapping that Fragment + // in a second nowrap/ellipsis box here would clip the chevron along + // with the label instead of leaving it visible. + typeof children === "string" ? ( + {children} + ) : ( + children + ) + } + + ); +} + +function SubMenuTriggerContent({ trigger }: { trigger: ReactNode }) { + return ( + <> + {trigger} + + + ); +} + +interface SubMenuProps { + id?: Key; + trigger: ReactNode; + children: ReactNode; + isDisabled?: boolean; + textValue?: string; + className?: string; + onAction?: (key: Key) => void; + // "flyout" (default) opens a nested popover next to this item, like a + // desktop context menu. "drilldown" replaces the parent menu's own + // content with this submenu's items and adds a back item, for trees too + // deep/wide for a chain of flyouts (e.g. move-to-project's team -> + // project nesting). onAction is ignored in drilldown mode: its items sit + // in the same RACMenu as everything else, so the root Menu/ContextMenu's + // own onAction already sees them selected. + variant?: "flyout" | "drilldown"; + // Only meaningful for the "flyout" variant: its nested popover is its own + // RACMenu (see below), independent of the root's. A "drilldown" submenu + // has no popover of its own to size — it renders straight into the root's, + // which is sized by the root Menu/ContextMenu's own maxWidth instead. + // @default 250 + maxWidth?: CssLength; +} + +// The submenu's own trigger is always a MenuItem, which — unlike the +// arbitrary trigger passed to Menu/ContextMenu above — is a real +// react-aria-components element that forwards its ref properly. So +// SubmenuTrigger's built-in ref/positioning detection (the thing that +// doesn't work for Penpot's own DS buttons) works fine here, and this can +// use the plain react-aria-components composition. +export function SubMenu({ + id, + trigger, + children, + isDisabled, + textValue, + className, + onAction, + variant = "flyout", + maxWidth = 250, +}: SubMenuProps) { + const closeController = useContext(MenuCloseContext); + const navigation = useContext(MenuNavigationContext); + const isDense = useContext(MenuDensityContext); + + if (variant === "drilldown") { + return ( + navigation?.drillIn(trigger, children)} + > + + + ); + } + + return ( + + + + + + { + onAction?.(key); + closeController?.closeAll(); + }} + autoFocus="first" + > + {children} + + + + ); +} + +interface MenuSeparatorProps { + className?: string; +} + +export function MenuSeparator({ className }: MenuSeparatorProps) { + return ; +} + +interface ContextMenuProps { + trigger: ReactNode; + children: ReactNode; + "aria-label": string; + placement?: Placement; + className?: string; + isDisabled?: boolean; + onAction?: (key: Key) => void; + // See the same props on Menu above. + maxWidth?: CssLength; + isDense?: boolean; + // ContextMenu owns its open state (see the comment below on why it isn't + // controlled like Menu is) — this only notifies the caller when that state + // changes, e.g. to keep a trigger's own hover-only affordances visible for + // as long as this stays open. + onOpenChange?: (isOpen: boolean) => void; +} + +// MenuTrigger's built-in press/context-menu detection only works when its +// child calls usePress() itself (e.g. react-aria-components' own Button). +// Penpot's own DS buttons aren't react-aria components, so instead of +// relying on that, this drives everything explicitly: a plain onContextMenu +// handler opens a standalone Popover anchored to an invisible element moved +// to the click position. +export function ContextMenu({ + trigger, + children, + "aria-label": ariaLabel, + placement = "bottom start", + className, + isDisabled, + onAction, + maxWidth = 250, + isDense = false, + onOpenChange, +}: ContextMenuProps) { + const anchorRef = useRef(null); + const popoverRef = useRef(null); + const [isOpen, setIsOpen] = useState(false); + const [shouldSkipAnimation, setShouldSkipAnimation] = useState(false); + const { + content: navigationContent, + menuRef, + isDrilledIn, + } = useMenuNavigation(children, isOpen); + + useEffect(() => { + if (isOpen) setShouldSkipAnimation(false); + }, [isOpen]); + + // See the same handleOpenChange in Menu above: closing always skips the + // exit animation so a right-click landing while the previous instance is + // still mid exit-fade can't race it into failing to reopen. + const handleOpenChange = useCallback( + (open: boolean) => { + if (!open) setShouldSkipAnimation(true); + setIsOpen(open); + onOpenChange?.(open); + }, + [onOpenChange], + ); + + const handleContextMenu = useCallback( + (e: ReactMouseEvent) => { + if (isDisabled) return; + e.preventDefault(); + const anchor = anchorRef.current; + if (anchor) { + anchor.style.left = `${e.clientX}px`; + anchor.style.top = `${e.clientY}px`; + } + handleOpenChange(true); + }, + [isDisabled, handleOpenChange], + ); + + const closeController: MenuCloseController = { + // handleOpenChange already sets shouldSkipAnimation on close. + closeAll: () => handleOpenChange(false), + shouldSkipAnimation, + }; + + const close = useCallback(() => handleOpenChange(false), [handleOpenChange]); + useSoloOpen(isOpen, close); + useCloseOnOutsideClick(isOpen, popoverRef, close); + + return ( + +
+ {trigger} +
+ {createPortal( + // Popover itself portals to document.body, so its anchor must too — + // otherwise an ancestor with a CSS transform (a Storybook decorator, + // or any app-level one) can make position: fixed here resolve + // against that ancestor instead of the real viewport, while + // clientX/clientY (used to place it) always stay viewport-relative. +
, + document.body, + )} + + handleOpenChange(false)} + autoFocus="first" + > + + {navigationContent} + + + + + ); +} diff --git a/frontend/packages/ui/src/menu.ts b/frontend/packages/ui/src/menu.ts new file mode 100644 index 0000000000..0ac6fc3f68 --- /dev/null +++ b/frontend/packages/ui/src/menu.ts @@ -0,0 +1,7 @@ +export { + Menu, + MenuItem, + MenuSeparator, + SubMenu, + ContextMenu, +} from "./lib/menu/Menu"; diff --git a/frontend/packages/ui/vite.config.mts b/frontend/packages/ui/vite.config.mts index 5ec50406f4..9519e70987 100644 --- a/frontend/packages/ui/vite.config.mts +++ b/frontend/packages/ui/vite.config.mts @@ -1,20 +1,19 @@ /// -import { defineConfig } from 'vite'; -import react from '@vitejs/plugin-react'; -import dts from 'vite-plugin-dts'; -import * as path from 'path'; -import { copyFileSync } from 'node:fs'; +import { defineConfig, esmExternalRequirePlugin } from "vite"; +import react from "@vitejs/plugin-react"; +import dts from "vite-plugin-dts"; +import * as path from "path"; +import { copyFileSync } from "node:fs"; + +const externalDeps = ["react", "react-dom", "react/jsx-runtime"]; const copyCssPlugin = () => ({ - name: 'copy-css', + name: "copy-css", closeBundle: () => { try { - copyFileSync( - 'dist/ui.css', - '../../resources/public/css/ui.css', - ); + copyFileSync("dist/ui.css", "../../resources/public/css/ui.css"); } catch (e) { - console.log('Error copying css file', e); + console.log("Error copying css file", e); } }, }); @@ -24,27 +23,25 @@ export default defineConfig(() => ({ css: { preprocessorOptions: { scss: { - loadPaths: [ - path.resolve(import.meta.dirname, '../../src/app/main/ui'), - ], + loadPaths: [path.resolve(import.meta.dirname, "../../src/app/main/ui")], }, }, }, plugins: [ react({ babel: { - plugins: ['babel-plugin-react-compiler'], + plugins: ["babel-plugin-react-compiler"], }, }), dts({ - entryRoot: 'src', - tsconfigPath: path.join(import.meta.dirname, 'tsconfig.lib.json'), + entryRoot: "src", + tsconfigPath: path.join(import.meta.dirname, "tsconfig.lib.json"), pathsToAliases: false, }), copyCssPlugin(), ], build: { - outDir: 'dist/', + outDir: "dist/", emptyOutDir: true, reportCompressedSize: true, commonjsOptions: { @@ -52,26 +49,34 @@ export default defineConfig(() => ({ }, lib: { entry: { - index: 'src/index.ts', - modal: 'src/modal.ts', + index: "src/index.ts", + modal: "src/modal.ts", + menu: "src/menu.ts", }, - name: 'ui', - formats: ['es' as const], + name: "ui", + formats: ["es" as const], }, rollupOptions: { - external: ['react', 'react-dom', 'react/jsx-runtime'], + // Vendored CJS-only deps (e.g. use-sync-external-store) call + // require("react") internally. Rolldown keeps require() calls + // against external modules as-is instead of converting them to + // import, which breaks in the browser where require() doesn't + // exist. esmExternalRequirePlugin both marks these as external and + // rewrites those calls to real ESM imports. + // https://rolldown.rs/in-depth/bundling-cjs#require-external-modules + plugins: [esmExternalRequirePlugin({ external: externalDeps })], }, }, test: { - name: 'ui', + name: "ui", watch: false, globals: true, - environment: 'jsdom', - include: ['{src,tests}/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'], - reporters: ['default'], + environment: "jsdom", + include: ["{src,tests}/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}"], + reporters: ["default"], coverage: { - reportsDirectory: '../../coverage/libs/ui', - provider: 'v8' as const, + reportsDirectory: "../../coverage/libs/ui", + provider: "v8" as const, }, }, })); diff --git a/frontend/playwright/ui/specs/project-menu.spec.js b/frontend/playwright/ui/specs/project-menu.spec.js new file mode 100644 index 0000000000..c8dec33dc7 --- /dev/null +++ b/frontend/playwright/ui/specs/project-menu.spec.js @@ -0,0 +1,128 @@ +import { test, expect } from "@playwright/test"; +import DashboardPage from "../pages/DashboardPage"; + +test.beforeEach(async ({ page }) => { + await DashboardPage.init(page); +}); + +// New Project 1 is a regular project; Drafts is the is-default pseudo-project +// (see frontend/playwright/data/dashboard/get-projects-full.json), which the +// menu itself renders with rename/duplicate/pin/move-to/delete all hidden. +async function setupTwoProjects(dashboardPage) { + await dashboardPage.mockRPC( + "get-projects?team-id=*", + "dashboard/get-projects-full.json", + ); + await dashboardPage.setupDrafts(); +} + +function projectRow(page, name) { + return page.getByRole("article").filter({ hasText: name }); +} + +test("User can open a project's options menu from the \"...\" button", async ({ + page, +}) => { + const dashboardPage = new DashboardPage(page); + await setupTwoProjects(dashboardPage); + await dashboardPage.goToDashboard(); + + const row = projectRow(page, "New Project 1"); + await row.getByTestId("project-options").click(); + + const menu = page.getByRole("menu"); + await expect(menu.getByTestId("project-rename")).toBeVisible(); + await expect(menu.getByTestId("project-duplicate")).toBeVisible(); + await expect(menu.getByTestId("project-pin")).toBeVisible(); + await expect(menu.getByTestId("project-delete")).toBeVisible(); + // project-move-to is covered separately below: it only renders once there + // is at least one other team to move to, which the default single-team + // fixture used here doesn't have. +}); + +test("User can open a project's options menu by right-clicking its title", async ({ + page, +}) => { + const dashboardPage = new DashboardPage(page); + await setupTwoProjects(dashboardPage); + await dashboardPage.goToDashboard(); + + const row = projectRow(page, "New Project 1"); + await row.getByText("New Project 1").click({ button: "right" }); + + const menu = page.getByRole("menu"); + await expect(menu.getByTestId("project-rename")).toBeVisible(); + await expect(menu.getByTestId("project-delete")).toBeVisible(); +}); + +test("The default Drafts project has a limited options menu", async ({ + page, +}) => { + const dashboardPage = new DashboardPage(page); + await setupTwoProjects(dashboardPage); + await dashboardPage.goToDashboard(); + + const row = projectRow(page, "Drafts"); + await row.getByTestId("project-options").click(); + + const menu = page.getByRole("menu"); + await expect(menu).toBeVisible(); + await expect(menu.getByTestId("project-rename")).toHaveCount(0); + await expect(menu.getByTestId("project-duplicate")).toHaveCount(0); + await expect(menu.getByTestId("project-pin")).toHaveCount(0); + await expect(menu.getByTestId("project-move-to")).toHaveCount(0); + await expect(menu.getByTestId("project-delete")).toHaveCount(0); +}); + +test("User can rename a project from the options menu", async ({ page }) => { + const dashboardPage = new DashboardPage(page); + await setupTwoProjects(dashboardPage); + await dashboardPage.goToDashboard(); + + const row = projectRow(page, "New Project 1"); + await row.getByTestId("project-options").click(); + await page.getByTestId("project-rename").click(); + + // Not scoped to `row` (renaming swaps the title for an , whose + // value doesn't count as text content, so the `hasText` filter used to + // find the row in the first place would no longer match it) and matched + // by value rather than role, since the dashboard's own search field is + // also a textbox. + await expect(page.locator('input[value="New Project 1"]')).toBeVisible(); +}); + +test("User can delete a project from the options menu", async ({ page }) => { + const dashboardPage = new DashboardPage(page); + await setupTwoProjects(dashboardPage); + await dashboardPage.goToDashboard(); + + const row = projectRow(page, "New Project 1"); + await row.getByTestId("project-options").click(); + await page.getByTestId("project-delete").click(); + + await expect( + page.getByRole("heading", { name: "Delete project" }), + ).toBeVisible(); + await expect( + page.getByRole("button", { name: "Delete project" }), + ).toBeVisible(); +}); + +test("The move-to submenu lists the user's other teams", async ({ page }) => { + const dashboardPage = new DashboardPage(page); + await setupTwoProjects(dashboardPage); + await DashboardPage.mockRPC( + page, + "get-teams", + "logged-in-user/get-teams-complete.json", + ); + await dashboardPage.goToDashboard(); + + const row = projectRow(page, "New Project 1"); + await row.getByTestId("project-options").click(); + await page.getByTestId("project-move-to").click(); + + await expect( + page.getByRole("menuitem", { name: "Second team" }), + ).toBeVisible(); +}); diff --git a/frontend/src/app/main/data/dashboard.cljs b/frontend/src/app/main/data/dashboard.cljs index 4d9619d630..db82978768 100644 --- a/frontend/src/app/main/data/dashboard.cljs +++ b/frontend/src/app/main/data/dashboard.cljs @@ -198,8 +198,7 @@ (update [_ state] (-> state (dissoc :selected-files) - (dissoc :selected-project) - (update :dashboard-local dissoc :menu-open :menu-pos))))) + (dissoc :selected-project))))) (defn toggle-file-select [{:keys [id project-id] :as file}] @@ -214,36 +213,6 @@ (assoc :selected-project project-id)) state))))) -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -;; Show grid menu -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; - -(defn show-file-menu-with-position - [file-id pos] - (ptk/reify ::show-file-menu-with-position - ptk/UpdateEvent - (update [_ state] - (update state :dashboard-local assoc - :menu-open true - :menu-pos pos - :file-id file-id)))) - -(defn show-file-menu - [] - (ptk/reify ::show-file-menu - ptk/UpdateEvent - (update [_ state] - (update state :dashboard-local - assoc :menu-open true)))) - -(defn hide-file-menu - [] - (ptk/reify ::hide-file-menu - ptk/UpdateEvent - (update [_ state] - (update state :dashboard-local - assoc :menu-open false)))) - (defn start-edit-file-name [file-id] (ptk/reify ::start-edit-file-menu diff --git a/frontend/src/app/main/ui/dashboard/file_menu.cljs b/frontend/src/app/main/ui/dashboard/file_menu.cljs index f644730dba..7a0abddea5 100644 --- a/frontend/src/app/main/ui/dashboard/file_menu.cljs +++ b/frontend/src/app/main/ui/dashboard/file_menu.cljs @@ -16,12 +16,10 @@ [app.main.repo :as rp] [app.main.router :as rt] [app.main.store :as st] - [app.main.ui.components.context-menu-a11y :refer [context-menu*]] [app.main.ui.context :as ctx] - [app.util.dom :as dom] + [app.main.ui.ds.layout.menu :refer [menu* menu-item* menu-separator* sub-menu*]] [app.util.i18n :as i18n :refer [tr]] [beicon.v2.core :as rx] - [potok.v2.core :as ptk] [rumext.v2 :as mf])) (defn- get-project-name @@ -55,18 +53,50 @@ {} projects)) -(mf/defc file-menu* - [{:keys [files on-edit on-close top left navigate origin parent-id can-edit can-restore]}] +;; The "move to" tree can be arbitrarily deep (current team's projects, then +;; every other team's own projects), so every level here uses SubMenu's +;; drilldown variant instead of a flyout: opening a chain of flyouts that +;; deep would run off-screen well before it ran out of teams. +(mf/defc move-to-items* + {::mf/private true} + [{:keys [current-projects other-teams current-team-id on-move]}] + [:* + (for [project current-projects] + [:> menu-item* {:key (get-project-id project) + :id (get-project-id project) + :on-action (on-move current-team-id (:id project))} + (get-project-name project)]) + + (when (seq other-teams) + [:> sub-menu* {:key "move-to-other-team" + :id "move-to-other-team" + :trigger (tr "dashboard.move-to-other-team") + :variant "drilldown"} + (for [team other-teams] + [:> sub-menu* {:key (get-project-id team) + :id (get-project-id team) + :trigger (get-team-name team) + :variant "drilldown"} + (for [sub-project (:projects team)] + [:> menu-item* {:key (get-project-id sub-project) + :id (get-project-id sub-project) + :on-action (on-move (:id team) (:id sub-project))} + (get-project-name sub-project)])])])]) + +;; The menu items only, with no popover of their own — shared by file-menu* +;; below (opened from the "..." button, via Menu) and by grid.cljs's own +;; right-click handling (via ContextMenu), so both triggers show the exact +;; same options. + +(mf/defc file-menu-items* + [{:keys [files on-edit navigate origin can-edit can-restore]}] (assert (seq files) "missing `files` prop") (assert (fn? on-edit) "missing `on-edit` prop") - (assert (fn? on-close) "missing `on-close` prop") (assert (boolean? navigate) "missing `navigate` prop") (let [is-lib-page? (= :libraries origin) is-search-page? (= :search origin) - top (or top 0) - left (or left 0) file (first files) file-count (count files) @@ -83,13 +113,13 @@ (:projects current-team)) on-new-tab - (fn [_] + (fn [] (st/emit! (dcm/go-to-workspace {:file-id (:id file) ::rt/new-window true}))) on-duplicate - (fn [_] + (fn [] (apply st/emit! (map dd/duplicate-file files)) (st/emit! (ntf/success (tr "dashboard.success-duplicate-file" (i18n/c file-count))))) @@ -100,8 +130,7 @@ (dd/clear-selected-files))) on-delete - (fn [event] - (dom/stop-propagation event) + (fn [] (let [num-shared (filter #(:is-shared %) files)] (if (< 0 (count num-shared)) @@ -149,7 +178,6 @@ (let [params {:ids (into #{} (map :id) files) :project-id project-id}] (fn [] - (let [num-shared (filter #(:is-shared %) files)] (if (and (< 0 (count num-shared)) (not= team-id current-team-id)) @@ -171,14 +199,11 @@ (run! #(st/emit! (dd/set-file-shared (assoc % :is-shared false))) files)) on-add-shared - (fn [event] - (dom/stop-propagation event) + (fn [] (st/emit! (dcm/show-shared-dialog (:id file) add-shared))) on-del-shared - (fn [event] - (dom/prevent-default event) - (dom/stop-propagation event) + (fn [] (st/emit! (modal/show {:type :delete-shared-libraries :origin :unpublish @@ -224,126 +249,101 @@ :on-accept accept-fn}))))] (mf/with-effect [] - (->> (rp/cmd! :get-all-projects) - (rx/map group-by-team) - (rx/subs! #(reset! teams* %)))) + (let [subs (->> (rp/cmd! :get-all-projects) + (rx/map group-by-team) + (rx/subs! #(reset! teams* %)))] + #(rx/dispose! subs))) - (mf/with-effect [on-close] - (st/emit! (ptk/data-event :dropdown/open {:id "file-menu"})) - (let [stream (->> st/stream - (rx/filter (ptk/type? :dropdown/open)) - (rx/map deref) - (rx/filter #(not= "file-menu" (:id %))) - (rx/take 1)) - subs (rx/subs! nil nil on-close stream)] - (fn [] - (rx/dispose! subs)))) + (cond + can-restore + [:* + [:> menu-item* {:id "restore-file" :on-action on-restore-immediately} + (tr "dashboard.file-menu.restore-files-option" (i18n/c file-count))] + [:> menu-item* {:id "delete-file" :on-action on-delete-immediately} + (tr "dashboard.file-menu.delete-files-permanently-option" (i18n/c file-count))]] - (let [sub-options - (concat - (for [project current-projects] - {:name (get-project-name project) - :id (get-project-id project) - :handler (on-move current-team-id (:id project))}) - (when (seq other-teams) - [{:name (tr "dashboard.move-to-other-team") - :id "move-to-other-team" - :options - (for [team other-teams] - {:name (get-team-name team) - :id (get-project-id team) - :options - (for [sub-project (:projects team)] - {:name (get-project-name sub-project) - :id (get-project-id sub-project) - :handler (on-move (:id team) - (:id sub-project))})})}])) + multi? + [:* + (when can-edit + [:> menu-item* {:id "duplicate-multi" :on-action on-duplicate :datatest-id "duplicate-multi"} + (tr "dashboard.duplicate-multi" file-count)]) - options - (if can-restore - [{:name (tr "dashboard.file-menu.restore-files-option" (i18n/c file-count)) - :id "restore-file" - :handler on-restore-immediately} - {:name (tr "dashboard.file-menu.delete-files-permanently-option" (i18n/c file-count)) - :id "delete-file" - :handler on-delete-immediately}] - (if multi? - [(when can-edit - {:name (tr "dashboard.duplicate-multi" file-count) - :id "duplicate-multi" - :handler on-duplicate}) + (when (and (or (seq current-projects) (seq other-teams)) can-edit) + [:> sub-menu* {:id "file-move-multi" :trigger (tr "dashboard.move-to-multi" file-count) :variant "drilldown"} + [:> move-to-items* {:current-projects current-projects + :other-teams other-teams + :current-team-id current-team-id + :on-move on-move}]]) - (when (and (or (seq current-projects) (seq other-teams)) can-edit) - {:name (tr "dashboard.move-to-multi" file-count) - :id "file-move-multi" - :options sub-options}) + [:> menu-item* {:id "file-binary-export-multi" :on-action on-export-binary-files} + (tr "dashboard.export-binary-multi" file-count)] - {:name (tr "dashboard.export-binary-multi" file-count) - :id "file-binary-export-multi" - :handler on-export-binary-files} + (when (and (:is-shared file) can-edit) + [:> menu-item* {:id "file-unpublish-multi" :on-action on-del-shared} + (tr "labels.unpublish-multi-files" file-count)]) - (when (and (:is-shared file) can-edit) - {:name (tr "labels.unpublish-multi-files" file-count) - :id "file-unpublish-multi" - :handler on-del-shared}) + (when (and (not is-lib-page?) can-edit) + [:* + [:> menu-separator*] + [:> menu-item* {:id "file-delete-multi" :on-action on-delete} + (tr "labels.delete-multi-files" file-count)]])] - (when (and (not is-lib-page?) can-edit) - {:name :separator} - {:name (tr "labels.delete-multi-files" file-count) - :id "file-delete-multi" - :handler on-delete})] + :else + [:* + [:> menu-item* {:id "file-open-new-tab" :on-action on-new-tab} + (tr "dashboard.open-in-new-tab")] - [{:name (tr "dashboard.open-in-new-tab") - :id "file-open-new-tab" - :handler on-new-tab} - (when (and (not is-search-page?) can-edit) - {:name (tr "labels.rename") - :id "file-rename" - :handler on-edit}) + (when (and (not is-search-page?) can-edit) + [:> menu-item* {:id "file-rename" :on-action on-edit} + (tr "labels.rename")]) - (when (and (not is-search-page?) can-edit) - {:name (tr "dashboard.duplicate") - :id "file-duplicate" - :handler on-duplicate}) + (when (and (not is-search-page?) can-edit) + [:> menu-item* {:id "file-duplicate" :on-action on-duplicate} + (tr "dashboard.duplicate")]) - (when (and (not is-lib-page?) - (not is-search-page?) - (or (seq current-projects) (seq other-teams)) - can-edit) - {:name (tr "dashboard.move-to") - :id "file-move-to" - :options sub-options}) + (when (and (not is-lib-page?) + (not is-search-page?) + (or (seq current-projects) (seq other-teams)) + can-edit) + [:> sub-menu* {:id "file-move-to" :trigger (tr "dashboard.move-to") :variant "drilldown"} + [:> move-to-items* {:current-projects current-projects + :other-teams other-teams + :current-team-id current-team-id + :on-move on-move}]]) - (when (and (not is-search-page?) - can-edit) - (if (:is-shared file) - {:name (tr "dashboard.unpublish-shared") - :id "file-del-shared" - :handler on-del-shared} - {:name (tr "dashboard.add-shared") - :id "file-add-shared" - :handler on-add-shared})) + (when (and (not is-search-page?) can-edit) + ;; Same id in both branches: :is-shared can flip while this menu + ;; instance stays mounted (the on-add-shared/on-del-shared action + ;; itself changes it), and react-stately's Collection requires an + ;; item's id to stay stable across such an update rather than swap + ;; to a differently-id'd item in the same slot. + (if (:is-shared file) + [:> menu-item* {:id "file-shared-toggle" :on-action on-del-shared} + (tr "dashboard.unpublish-shared")] + [:> menu-item* {:id "file-shared-toggle" :on-action on-add-shared} + (tr "dashboard.add-shared")])) - {:name :separator} + [:> menu-separator*] - {:name (tr "dashboard.download-binary-file") - :id "download-binary-file" - :handler on-export-binary-files} + [:> menu-item* {:id "download-binary-file" :on-action on-export-binary-files} + (tr "dashboard.download-binary-file")] - (when (and (not is-lib-page?) (not is-search-page?) can-edit) - {:name :separator}) + (when (and (not is-lib-page?) (not is-search-page?) can-edit) + [:* + [:> menu-separator*] + [:> menu-item* {:id "file-delete" :on-action on-delete} + (tr "labels.delete")]])]))) - (when (and (not is-lib-page?) (not is-search-page?) can-edit) - {:name (tr "labels.delete") - :id "file-delete" - :handler on-delete})]))] - - [:> context-menu* - {:on-close on-close - :fixed (or (not= top 0) (not= left 0)) - :show true - :min-width true - :top top - :left left - :options options - :origin parent-id}]))) +(mf/defc file-menu* + [{:keys [files on-edit is-open on-open-change trigger navigate origin can-edit can-restore is-list]}] + [:> menu* + {:is-open is-open + :on-open-change on-open-change + :placement (if is-list "bottom end" "bottom start") + :trigger trigger} + [:> file-menu-items* {:files files + :on-edit on-edit + :navigate navigate + :origin origin + :can-edit can-edit + :can-restore can-restore}]]) diff --git a/frontend/src/app/main/ui/dashboard/files.cljs b/frontend/src/app/main/ui/dashboard/files.cljs index 436691f5a1..b3318fc6f8 100644 --- a/frontend/src/app/main/ui/dashboard/files.cljs +++ b/frontend/src/app/main/ui/dashboard/files.cljs @@ -15,31 +15,31 @@ [app.main.refs :as refs] [app.main.store :as st] [app.main.ui.dashboard.grid :refer [grid*]] + [app.main.ui.dashboard.import :as udi] [app.main.ui.dashboard.inline-edition :refer [inline-edition]] [app.main.ui.dashboard.layout-toggle :as lt :refer [layout-toggle*]] - [app.main.ui.dashboard.pin-button :refer [pin-button*]] [app.main.ui.dashboard.project-menu :refer [project-menu*]] + [app.main.ui.ds.buttons.button :refer [button*]] + [app.main.ui.ds.buttons.icon-button :refer [icon-button*]] + [app.main.ui.ds.foundations.assets.icon :as i] [app.main.ui.ds.product.empty-placeholder :refer [empty-placeholder*]] [app.main.ui.hooks :as hooks] - [app.main.ui.icons :as deprecated-icon] [app.util.dom :as dom] [app.util.i18n :as i18n :refer [tr]] [app.util.keyboard :as kbd] [cuerdas.core :as str] [rumext.v2 :as mf])) -(def ^:private menu-icon - (deprecated-icon/icon-xref :menu (stl/css :menu-icon))) - (mf/defc header* {::mf/private true} [{:keys [project create-fn can-edit layout on-change]}] (let [project-id (:id project) local - (mf/use-state - {:menu-open false - :edition false}) + (mf/use-state {:edition false}) + + menu-open* + (mf/use-state false) on-create-click (mf/use-fn @@ -51,22 +51,27 @@ on-menu-click (mf/use-fn (fn [event] - (let [position (dom/get-client-position event)] - (dom/prevent-default event) - (swap! local assoc :menu-open true :menu-pos position)))) - - on-menu-close - (mf/use-fn #(swap! local assoc :menu-open false)) + (dom/prevent-default event) + (swap! menu-open* not))) on-edit - (mf/use-fn #(swap! local assoc :edition true :menu-open false)) + (mf/use-fn + (fn [] + (reset! menu-open* false) + (swap! local assoc :edition true))) toggle-pin (mf/use-fn (mf/deps project) #(st/emit! (dd/toggle-project-pin project))) - on-import + file-input + (mf/use-ref nil) + + on-import-click + (mf/use-fn #(dom/click (mf/ref-val file-input))) + + on-finish-import (mf/use-fn (mf/deps project-id) (fn [] @@ -101,40 +106,56 @@ :on-change on-change}] (when ^boolean can-edit - [:a {:class (stl/css :btn-secondary :btn-small :new-file) - :tab-index "0" - :on-click on-create-click - :data-testid "new-file" - :on-key-down (fn [event] - (when (kbd/enter? event) - (on-create-click event)))} + [:> button* {:variant "secondary" + :class (stl/css :new-file) + :on-click on-create-click + :on-key-down (fn [event] + (when (kbd/enter? event) + (on-create-click event))) + :tab-index "0" + :data-testid "new-file"} (tr "dashboard.new-file")]) (when-not (:is-default project) - [:> pin-button* - {:tab-index 0 - :is-pinned (:is-pinned project) - :on-click toggle-pin - :on-key-down (fn [event] (when (kbd/enter? event) (toggle-pin event)))}]) + [:> icon-button* {:icon i/pin + :variant "ghost" + :aria-label (tr "dashboard.pin-unpin") + :aria-pressed (:is-pinned project) + :tab-index 0 + :on-click toggle-pin + :on-key-down (fn [event] + (when (kbd/enter? event) + (toggle-pin event)))}]) (when ^boolean can-edit - [:div {:class (stl/css :icon) - :tab-index "0" - :on-click on-menu-click - :title (tr "dashboard.options") - :on-key-down (fn [event] - (when (kbd/enter? event) - (on-menu-click event)))} - menu-icon]) + [:* + [:> project-menu* + {:project project + :is-open (deref menu-open*) + :on-open-change #(reset! menu-open* %) + :on-edit on-edit + :on-import-click on-import-click + :placement "bottom end" + :trigger + (mf/html + [:> icon-button* {:icon i/menu + :variant "ghost" + :aria-label (tr "dashboard.options") + :aria-pressed (deref menu-open*) + :on-click on-menu-click + :on-key-down (fn [event] + (when (kbd/enter? event) + (on-menu-click event)))}])}] - (when ^boolean can-edit - [:> project-menu* {:project project - :show (:menu-open @local) - :left (- (:x (:menu-pos @local)) 180) - :top (:y (:menu-pos @local)) - :on-edit on-edit - :on-close on-menu-close - :on-import on-import}])]])) + ;; Kept mounted for as long as this header is, regardless of the + ;; menu's own open state: the popover really unmounts its content on + ;; close (unlike the old context-menu-a11y, which just hid it), and + ;; selecting "Import" closes the menu in the same tick — a ref owned + ;; inside the popover could already be gone by the time its own + ;; click handler fires. + [:> udi/import-form* {:ref file-input + :project-id project-id + :on-finish-import on-finish-import}]])]])) (mf/defc files-section* [{:keys [project team layout on-layout-change]}] diff --git a/frontend/src/app/main/ui/dashboard/files.scss b/frontend/src/app/main/ui/dashboard/files.scss index 8f69e352e9..5ab0695cad 100644 --- a/frontend/src/app/main/ui/dashboard/files.scss +++ b/frontend/src/app/main/ui/dashboard/files.scss @@ -4,43 +4,20 @@ // // Copyright (c) KALEIDOS SUBSIDIARY SL -@use "refactor/common-refactor.scss" as deprecated; @use "common/refactor/common-dashboard"; +@use "ds/_borders.scss" as *; @use "ds/_sizes.scss" as *; @use "ds/_utils.scss" as *; .dashboard-container { flex: 1 0 0; - margin-right: deprecated.$s-16; + margin-inline-end: var(--sp-l); overflow-y: auto; - width: 100%; - border-top: deprecated.$s-1 solid var(--color-background-quaternary); + inline-size: 100%; + border-block-start: $b-1 solid var(--color-background-quaternary); padding-block-end: var(--sp-xxxl); - - &.dashboard-projects { - user-select: none; - } - - &.dashboard-shared { - width: calc(100vw - deprecated.$s-320); - margin-right: deprecated.$s-52; - } - - &.search { - margin-top: deprecated.$s-12; - } -} - -.new-file { - margin-inline-end: deprecated.$s-8; -} - -.menu-icon { - @extend %button-icon; - - stroke: var(--icon-foreground); } .placeholder-placement { - margin: deprecated.$s-16 deprecated.$s-32; + margin: var(--sp-l) var(--sp-xxxl); } diff --git a/frontend/src/app/main/ui/dashboard/grid.cljs b/frontend/src/app/main/ui/dashboard/grid.cljs index 20f9117316..b46a0c53c1 100644 --- a/frontend/src/app/main/ui/dashboard/grid.cljs +++ b/frontend/src/app/main/ui/dashboard/grid.cljs @@ -9,7 +9,6 @@ (:require [app.common.data :as d] [app.common.data.macros :as dm] - [app.common.geom.point :as gpt] [app.common.logging :as log] [app.common.time :as ct] [app.config :as cf] @@ -26,12 +25,13 @@ [app.main.repo :as rp] [app.main.store :as st] [app.main.ui.components.color-bullet :as bc] - [app.main.ui.components.portal :refer [portal-on-document*]] - [app.main.ui.dashboard.file-menu :refer [file-menu*]] + [app.main.ui.dashboard.file-menu :refer [file-menu* file-menu-items*]] [app.main.ui.dashboard.import :refer [use-import-file]] [app.main.ui.dashboard.inline-edition :refer [inline-edition]] [app.main.ui.dashboard.placeholder :refer [empty-grid-placeholder* loading-placeholder*]] + [app.main.ui.ds.buttons.icon-button :refer [icon-button*]] [app.main.ui.ds.foundations.assets.icon :as i :refer [icon*]] + [app.main.ui.ds.layout.menu :refer [context-menu*]] [app.main.ui.ds.product.loader :refer [loader*]] [app.main.ui.hooks :as h] [app.main.worker :as mw] @@ -269,9 +269,19 @@ file-id (get file :id) - menu-pos (get state :menu-pos) - menu-open? (and (get state :menu-open) - (= file-id (:file-id state))) + menu-open* (mf/use-state false) + menu-open? (deref menu-open*) + + ;; Only act on the whole selection when this file is part of it. The + ;; selection can legitimately hold other files instead: the click that + ;; opens the menu applies its selection through a store dispatch that + ;; only lands on the next render, and toggle-file-select is a no-op + ;; across projects, so a shift-click on a file in another project + ;; never adds it. In both cases the menu must target what the user + ;; actually pointed at, not whatever happens to be selected. + menu-files (if (contains? selected-files file-id) + (vals selected-files) + [file]) selected? (contains? selected-files file-id) selected-num (count selected-files) @@ -283,10 +293,6 @@ library-view? (= origin :libraries) - on-menu-close - (mf/use-fn - #(st/emit! (dd/hide-file-menu))) - on-select (mf/use-fn (mf/deps selected? selected-num) @@ -312,7 +318,7 @@ (mf/use-fn (mf/deps selected? selected-num) (fn [event] - (st/emit! (dd/hide-file-menu)) + (reset! menu-open* false) (when can-edit (let [offset (dom/get-offset-position (dom/event->native-event event)) item-el (mf/ref-val node-ref) @@ -341,39 +347,23 @@ on-menu-click (mf/use-fn - (mf/deps file selected? menu-open?) + (mf/deps file selected?) (fn [event] (dom/stop-propagation event) - - (if menu-open? - (st/emit! (dd/hide-file-menu)) - - (do - (when-not selected? - (when-not (kbd/shift? event) - (st/emit! (dd/clear-selected-files))) - (st/emit! (dd/toggle-file-select file))) - - (let [client-position - (dom/get-client-position event) - - position - (if (and (nil? (:y client-position)) (nil? (:x client-position))) - (let [target-element (dom/get-target event) - points (dom/get-bounding-rect target-element) - y (:top points) - x (:left points)] - (gpt/point x y)) - client-position)] - - (st/emit! (dd/show-file-menu-with-position file-id position))))))) + (when-not selected? + (when-not (kbd/shift? event) + (st/emit! (dd/clear-selected-files))) + (st/emit! (dd/toggle-file-select file))) + (swap! menu-open* not))) on-context-menu (mf/use-fn - (mf/deps on-menu-click) + (mf/deps file selected?) (fn [event] - (dom/prevent-default event) - (on-menu-click event))) + (when-not selected? + (when-not (kbd/shift? event) + (st/emit! (dd/clear-selected-files))) + (st/emit! (dd/toggle-file-select file))))) edit (mf/use-fn @@ -386,9 +376,8 @@ on-edit (mf/use-fn - (mf/deps file) - (fn [event] - (dom/stop-propagation event) + (mf/deps file-id) + (fn [] (st/emit! (dd/start-edit-file-name file-id)))) on-key-down @@ -422,104 +411,115 @@ (mf/html [:div {:class (stl/css-case :project-thumbnail-actions true :is-force-display menu-open?)} - [:div {:class (stl/css :project-thumbnail-icon :menu) - :tab-index "0" - :role "button" - :aria-label (tr "dashboard.options") - :ref menu-ref - :id (dm/str file-id "-action-menu") - :on-click on-menu-click - :on-key-down on-menu-key-down} - - [:> icon* {:icon-id i/menu - :class (stl/css :menu-icon)}] - - (when (and selected? menu-open?) - ;; When the menu is open we disable events in the dashboard. We need to force pointer events - ;; so the menu can be handled - [:> portal-on-document* {} - [:> file-menu* {:files (vals selected-files) - :left (+ 24 (:x menu-pos)) - :top (:y menu-pos) - :can-edit can-edit - :navigate true - :on-edit on-edit - :on-close on-menu-close - :origin origin - :parent-id (dm/str file-id "-action-menu") - :can-restore can-restore}]])]])] + [:> file-menu* {:files menu-files + :is-open menu-open? + :on-open-change #(reset! menu-open* %) + :can-edit can-edit + :navigate true + :on-edit on-edit + :origin origin + :can-restore can-restore + :is-list list? + :trigger + (mf/html + [:> icon-button* {:icon i/menu + :variant "ghost" + :aria-label (tr "dashboard.options") + :on-click on-menu-click + :on-key-down on-menu-key-down + :tab-index "0" + :ref menu-ref + :id (dm/str file-id "-action-menu")}])}]])] (if ^boolean list? [:li {:class (stl/css-case :grid-item true :list-item true :library-item library-view?)} - [:div - {:class (stl/css-case :list-item-row true - :is-selected selected?) - :ref node-ref - :role "button" - :title (:name file) - :aria-label (:name file) - :draggable (dm/str can-edit) - :on-click on-select - :on-key-down on-key-down - :on-double-click on-navigate - :on-drag-start on-drag-start - :on-context-menu on-context-menu} + [:> context-menu* {:aria-label (tr "dashboard.options") + :trigger + (mf/html + [:div + {:class (stl/css-case :list-item-row true + :is-selected selected?) + :ref node-ref + :role "button" + :title (:name file) + :aria-label (:name file) + :draggable (dm/str can-edit) + :on-click on-select + :on-key-down on-key-down + :on-double-click on-navigate + :on-drag-start on-drag-start + :on-context-menu on-context-menu} - (if ^boolean editing? - [:& inline-edition {:content (:name file) - :on-end edit - :max-length 250}] - [:h3 {:class (stl/css :list-item-name)} (:name file)]) + (if ^boolean editing? + [:& inline-edition {:content (:name file) + :on-end edit + :max-length 250}] + [:h3 {:class (stl/css :list-item-name)} (:name file)]) - (when (and (:is-shared file) (not library-view?)) - [:span {:class (stl/css :list-item-badge) - :aria-label (tr "workspace.assets.shared-library") - :title (tr "workspace.assets.shared-library")} - [:> icon* {:icon-id i/library}]]) + (when (and (:is-shared file) (not library-view?)) + [:span {:class (stl/css :list-item-badge) + :aria-label (tr "workspace.assets.shared-library") + :title (tr "workspace.assets.shared-library")} + [:> icon* {:icon-id i/library}]]) - [:> grid-item-metadata* {:file file :layout :list}] + [:> grid-item-metadata* {:file file :layout :list}] - menu-element]] + menu-element])} + [:> file-menu-items* {:files menu-files + :can-edit can-edit + :navigate true + :on-edit on-edit + :origin origin + :can-restore can-restore}]]] [:li {:class (stl/css-case :grid-item true :project-thumbnail true :library-item library-view?)} - [:div {:class (stl/css-case :is-selected selected? - :grid-item-button true) - :ref node-ref - :role "button" - :title (:name file) - :aria-label (:name file) - :draggable (dm/str can-edit) - :on-click on-select - :on-key-down on-key-down - :on-double-click on-navigate - :on-drag-start on-drag-start - :on-context-menu on-context-menu} + [:> context-menu* {:aria-label (tr "dashboard.options") + :trigger + (mf/html + [:div {:class (stl/css-case :is-selected selected? + :grid-item-button true) + :ref node-ref + :role "button" + :title (:name file) + :aria-label (:name file) + :draggable (dm/str can-edit) + :on-click on-select + :on-key-down on-key-down + :on-double-click on-navigate + :on-drag-start on-drag-start + :on-context-menu on-context-menu} - (if ^boolean library-view? - [:> grid-item-library* {:file file - :can-restore can-restore}] - [:> grid-item-thumbnail* {:file file - :can-edit can-edit - :can-restore can-restore}]) + (if ^boolean library-view? + [:> grid-item-library* {:file file + :can-restore can-restore}] + [:> grid-item-thumbnail* {:file file + :can-edit can-edit + :can-restore can-restore}]) - (when (and (:is-shared file) (not library-view?)) - [:div {:class (stl/css :grid-item-badge)} - [:> icon* {:icon-id i/library}]]) + (when (and (:is-shared file) (not library-view?)) + [:div {:class (stl/css :grid-item-badge)} + [:> icon* {:icon-id i/library}]]) - [:div {:class (stl/css :grid-item-info)} - [:div {:class (stl/css :grid-item-meta)} - (if ^boolean editing? - [:& inline-edition {:content (:name file) - :on-end edit - :max-length 250}] - [:h3 {:class (stl/css :grid-item-title)} (:name file)]) - [:> grid-item-metadata* {:file file :layout :grid}]] + [:div {:class (stl/css :grid-item-info)} + [:div {:class (stl/css :grid-item-meta)} + (if ^boolean editing? + [:& inline-edition {:content (:name file) + :on-end edit + :max-length 250}] + [:h3 {:class (stl/css :grid-item-title)} (:name file)]) + [:> grid-item-metadata* {:file file :layout :grid}]] - menu-element]]]))) + menu-element]])} + [:> file-menu-items* {:files menu-files + :can-edit can-edit + :navigate true + :on-edit on-edit + :origin origin + :can-restore can-restore}]]]))) (mf/defc grid* [{:keys [files project origin limit create-fn can-edit selected-files can-restore layout]}] @@ -541,9 +541,6 @@ import-files (use-import-file project-id on-finish-import) - on-scroll - (mf/use-fn #(st/emit! (dd/hide-file-menu))) - on-drag-enter (mf/use-fn (fn [e] @@ -585,7 +582,6 @@ :on-drag-over on-drag-over :on-drag-leave on-drag-leave :on-drop on-drop - :on-scroll on-scroll :ref node-ref} (cond (nil? files) diff --git a/frontend/src/app/main/ui/dashboard/grid.scss b/frontend/src/app/main/ui/dashboard/grid.scss index dd23ce7128..11f25f8f86 100644 --- a/frontend/src/app/main/ui/dashboard/grid.scss +++ b/frontend/src/app/main/ui/dashboard/grid.scss @@ -187,53 +187,19 @@ $thumbnail-default-height: px2rem(168); } .project-thumbnail-actions { - align-items: center; display: flex; - block-size: 100%; + align-self: end; justify-content: center; - opacity: 0; - inset-inline-end: $sz-6; + block-size: $sz-32; inline-size: $sz-32; + margin-block-end: var(--sp-s); + opacity: 0; &.is-force-display { opacity: 1; } } -.project-thumbnail-icon { - align-items: center; - display: flex; - margin-inline-end: var(--sp-s); - margin-block-start: 0; -} - -// ─── CARD VIEW: MENU ─────────────────────────── - -.menu { - align-items: flex-end; - display: flex; - flex-direction: column; - block-size: $sz-32; - justify-content: center; - margin-inline-end: 0; - margin-block-start: var(--sp-xl); - inline-size: 100%; - - --menu-icon-color: var(--button-tertiary-foreground-color-rest); - - &:hover, - &:focus { - --menu-icon-color: var(--button-tertiary-foreground-color-hover); - } -} - -.menu-icon { - color: var(--menu-icon-color); - margin-inline-end: 0; - block-size: $sz-16; - inline-size: $sz-16; -} - // ─── LIST VIEW ───────────────────────────────── .list-item { @@ -247,22 +213,12 @@ $thumbnail-default-height: px2rem(168); // it sits inline at the end of the row. .project-thumbnail-actions { flex: 0 0 auto; + align-self: center; block-size: auto; + inline-size: auto; opacity: 1; - inline-size: auto; - } - - .project-thumbnail-icon { margin: 0; } - - .menu { - align-items: center; - flex-direction: row; - block-size: $sz-32; - margin: 0; - inline-size: auto; - } } .list-item-dragged { diff --git a/frontend/src/app/main/ui/dashboard/pin_button.cljs b/frontend/src/app/main/ui/dashboard/pin_button.cljs deleted file mode 100644 index f1250f6356..0000000000 --- a/frontend/src/app/main/ui/dashboard/pin_button.cljs +++ /dev/null @@ -1,30 +0,0 @@ -;; This Source Code Form is subject to the terms of the Mozilla Public -;; License, v. 2.0. If a copy of the MPL was not distributed with this -;; file, You can obtain one at http://mozilla.org/MPL/2.0/. -;; -;; Copyright (c) KALEIDOS SUBSIDIARY SL - -(ns app.main.ui.dashboard.pin-button - (:require-macros - [app.common.data.macros :as dm] - [app.main.style :as stl]) - (:require - [app.main.ui.icons :as deprecated-icon] - [app.util.i18n :as i18n :refer [tr]] - [app.util.object :as obj] - [rumext.v2 :as mf])) - -(def ^:private pin-icon - (deprecated-icon/icon-xref :pin (stl/css :icon))) - -(mf/defc pin-button* - [{:keys [aria-label is-pinned class] :as props}] - (let [aria-label (or aria-label (tr "dashboard.pin-unpin")) - class (dm/str (or class "") " " (stl/css-case :button true :button-active is-pinned)) - - props (-> (obj/clone props) - (obj/unset! "isPinned") - (obj/set! "className" class) - (obj/set! "aria-label" aria-label))] - - [:> "button" props pin-icon])) diff --git a/frontend/src/app/main/ui/dashboard/pin_button.scss b/frontend/src/app/main/ui/dashboard/pin_button.scss deleted file mode 100644 index 097e7e634d..0000000000 --- a/frontend/src/app/main/ui/dashboard/pin_button.scss +++ /dev/null @@ -1,35 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at http://mozilla.org/MPL/2.0/. -// -// Copyright (c) KALEIDOS SUBSIDIARY SL - -@use "refactor/common-refactor.scss" as deprecated; - -.button { - --pin-button-icon-color: var(--button-icon-foreground-color); - --pin-button-bg-color: none; - --pin-button-border-color: none; - - width: deprecated.$s-32; - height: deprecated.$s-32; - background: var(--pin-button-bg-color); - border: deprecated.$s-2 solid var(--pin-button-border-color); - border-radius: deprecated.$br-8; - display: grid; - place-content: center; - cursor: pointer; -} - -.button-active { - --pin-button-icon-color: var(--button-icon-foreground-color-selected); - --pin-button-bg-color: var(--button-icon-background-color-selected); - --pin-button-border-color: var(--button-icon-border-color-selected); -} - -.icon { - width: deprecated.$s-16; - height: deprecated.$s-16; - fill: none; - stroke: var(--pin-button-icon-color); -} diff --git a/frontend/src/app/main/ui/dashboard/project_menu.cljs b/frontend/src/app/main/ui/dashboard/project_menu.cljs index ea0a1ab8e9..3b21ea4898 100644 --- a/frontend/src/app/main/ui/dashboard/project_menu.cljs +++ b/frontend/src/app/main/ui/dashboard/project_menu.cljs @@ -12,23 +12,34 @@ [app.main.data.notifications :as ntf] [app.main.refs :as refs] [app.main.store :as st] - [app.main.ui.components.context-menu-a11y :refer [context-menu*]] [app.main.ui.context :as ctx] - [app.main.ui.dashboard.import :as udi] - [app.util.dom :as dom] + [app.main.ui.ds.layout.menu :refer [menu* menu-item* menu-separator* sub-menu*]] [app.util.i18n :as i18n :refer [tr]] - [beicon.v2.core :as rx] - [potok.v2.core :as ptk] [rumext.v2 :as mf])) -(mf/defc project-menu* - [{:keys [project show on-edit on-close top left on-import]}] - (let [top (or top 0) - left (or left 0) +;; The menu items only, with no popover of their own — shared by project-menu* +;; below (opened from the "..." button, via Menu) and by projects.cljs's own +;; right-click handling (via ContextMenu), so both triggers show the exact +;; same options. +;; +;; on-import-click, rather than this owning its own file input/ref: the +;; popover this renders inside really unmounts its content on close (unlike +;; the old context-menu-a11y, which just hid it), and selecting "Import" +;; closes the menu in the same tick — so a ref owned here can already be +;; gone by the time its own click handler would fire. The caller keeps the +;; hidden input mounted for as long as the row itself exists instead. +(mf/defc project-menu-items* + {::mf/private true} + [{:keys [project on-edit on-import-click]}] - current-team-id (mf/use-ctx ctx/current-team-id) - teams (mf/deref refs/teams) - teams (-> teams (dissoc current-team-id) vals vec) + (assert (some? project) "missing `project` prop") + (assert (fn? on-edit) "missing `on-edit` prop") + + (let [is-default? (:is-default project) + + current-team-id (mf/use-ctx ctx/current-team-id) + teams (mf/deref refs/teams) + other-teams (-> teams (dissoc current-team-id) vals) on-duplicate-success (fn [new-project] @@ -71,73 +82,45 @@ :title (tr "modals.delete-project-confirm.title") :message (tr "modals.delete-project-confirm.message") :accept-label (tr "modals.delete-project-confirm.accept") - :on-accept delete-fn}))) - - file-input - (mf/use-ref nil) - - on-import-files - (fn [] (dom/click (mf/ref-val file-input))) - - on-finish-import - (mf/use-fn - (fn [] (when (fn? on-import) (on-import)))) - - options - [(when-not (:is-default project) - {:name (tr "labels.rename") - :id "project-rename" - :handler on-edit}) - (when-not (:is-default project) - {:name (tr "dashboard.duplicate") - :id "project-duplicate" - :handler on-duplicate}) - (when-not (:is-default project) - {:name (tr "dashboard.pin-unpin") - :id "project-pin" - :handler toggle-pin}) - - (when (and (seq teams) (not (:is-default project))) - {:name (tr "dashboard.move-to") - :id "project-move-to" - :options (for [team teams] - {:name (:name team) - :id (str "move-to-" (:id team)) - :handler (on-move (:id team))})}) - - (when (some? on-import) - {:name (tr "dashboard.import") - :id "file-import" - :handler on-import-files}) - (when-not (:is-default project) - {:name :separator}) - (when-not (:is-default project) - {:name (tr "labels.delete") - :id "project-delete" - :handler on-delete})]] - - (mf/with-effect [show on-close] - (when ^boolean show - (st/emit! (ptk/data-event :dropdown/open {:id "project-menu"})) - (let [stream (->> st/stream - (rx/filter (ptk/type? :dropdown/open)) - (rx/map deref) - (rx/filter #(not= "project-menu" (:id %))) - (rx/take 1)) - subs (rx/subs! nil nil on-close stream)] - (fn [] - (rx/dispose! subs))))) + :on-accept delete-fn})))] [:* - [:> context-menu* - {:on-close on-close - :show show - :fixed (or (not= top 0) (not= left 0)) - :min-width true - :top top - :left left - :options options}] - [:> udi/import-form* {:ref file-input - :project-id (:id project) - :on-finish-import on-finish-import}]])) + (when-not is-default? + [:> menu-item* {:id "project-rename" :on-action on-edit} + (tr "labels.rename")]) + (when-not is-default? + [:> menu-item* {:id "project-duplicate" :on-action on-duplicate} + (tr "dashboard.duplicate")]) + + (when-not is-default? + [:> menu-item* {:id "project-pin" :on-action toggle-pin} + (tr "dashboard.pin-unpin")]) + + (when (and (seq other-teams) (not is-default?)) + [:> sub-menu* {:id "project-move-to" :trigger (tr "dashboard.move-to") :variant "drilldown"} + (for [team other-teams] + [:> menu-item* {:key (:id team) + :id (str "move-to-" (:id team)) + :on-action (on-move (:id team))} + (:name team)])]) + + (when (some? on-import-click) + [:> menu-item* {:id "file-import" :on-action on-import-click} + (tr "dashboard.import")]) + + (when-not is-default? + [:* + [:> menu-separator*] + [:> menu-item* {:id "project-delete" :on-action on-delete} + (tr "labels.delete")]])])) + +(mf/defc project-menu* + [{:keys [project is-open on-open-change on-edit on-import-click trigger placement]}] + [:> menu* {:is-open is-open + :on-open-change on-open-change + :placement placement + :trigger trigger} + [:> project-menu-items* {:project project + :on-edit on-edit + :on-import-click on-import-click}]]) diff --git a/frontend/src/app/main/ui/dashboard/projects.cljs b/frontend/src/app/main/ui/dashboard/projects.cljs index bd90dc96eb..063eb55afb 100644 --- a/frontend/src/app/main/ui/dashboard/projects.cljs +++ b/frontend/src/app/main/ui/dashboard/projects.cljs @@ -7,7 +7,6 @@ (ns app.main.ui.dashboard.projects (:require-macros [app.main.style :as stl]) (:require - [app.common.geom.point :as gpt] [app.common.time :as ct] [app.main.data.common :as dcm] [app.main.data.dashboard :as dd] @@ -20,14 +19,17 @@ [app.main.store :as st] [app.main.ui.dashboard.deleted :as deleted] [app.main.ui.dashboard.grid :refer [line-grid*]] + [app.main.ui.dashboard.import :as udi] [app.main.ui.dashboard.inline-edition :refer [inline-edition]] [app.main.ui.dashboard.layout-toggle :as lt :refer [layout-toggle*]] - [app.main.ui.dashboard.pin-button :refer [pin-button*]] - [app.main.ui.dashboard.project-menu :refer [project-menu*]] + [app.main.ui.dashboard.project-menu :refer [project-menu* + project-menu-items*]] [app.main.ui.ds.buttons.button :refer [button*]] + [app.main.ui.ds.buttons.icon-button :refer [icon-button*]] + [app.main.ui.ds.foundations.assets.icon :as i :refer [icon*]] + [app.main.ui.ds.layout.menu :refer [context-menu*]] [app.main.ui.ds.product.empty-placeholder :refer [empty-placeholder*]] [app.main.ui.hooks :as hooks] - [app.main.ui.icons :as deprecated-icon] [app.util.dom :as dom] [app.util.i18n :as i18n :refer [tr]] [app.util.keyboard :as kbd] @@ -36,23 +38,14 @@ [okulary.core :as l] [rumext.v2 :as mf])) -(def ^:private show-more-icon - (deprecated-icon/icon-xref :arrow (stl/css :show-more-icon))) - -(def ^:private close-icon - (deprecated-icon/icon-xref :close (stl/css :close-icon))) - -(def ^:private add-icon - (deprecated-icon/icon-xref :add (stl/css :add-icon))) - -(def ^:private menu-icon - (deprecated-icon/icon-xref :menu (stl/css :menu-icon))) - (mf/defc header* {::mf/wrap [mf/memo] ::mf/private true} [{:keys [can-edit layout on-change]}] - (let [on-click (mf/use-fn #(st/emit! (dd/create-project)))] + (let [on-click + (mf/use-fn + #(st/emit! (dd/create-project)))] + [:header {:class (stl/css :dashboard-header) :data-testid "dashboard-header"} [:div#dashboard-projects-title {:class (stl/css :dashboard-title)} @@ -61,15 +54,17 @@ [:> layout-toggle* {:layout layout :on-change on-change}] (when can-edit - [:button {:class (stl/css :btn-secondary :btn-small) - :on-click on-click - :data-testid "new-project-button"} + [:> button* {:variant "secondary" + :on-click on-click + :data-testid "new-project-button"} (tr "dashboard.new-project")])]])) (mf/defc team-hero* {::mf/wrap [mf/memo]} [{:keys [team on-close]}] - (let [on-nav-members-click (mf/use-fn #(st/emit! (dcm/go-to-dashboard-members))) + (let [on-nav-members-click + (mf/use-fn + #(st/emit! (dcm/go-to-dashboard-members))) on-invite (mf/use-fn @@ -97,10 +92,11 @@ [:> button* {:variant "primary" :on-click on-invite} (tr "onboarding.choice.team-up.invite-members")]] - [:button {:class (stl/css :close) - :on-click on-close' - :aria-label (tr "labels.close")} - close-icon]])) + [:> icon-button* {:icon i/close + :class (stl/css :close) + :variant "ghost" + :aria-label (tr "labels.close") + :on-click on-close'}]])) (mf/defc project-item* {::mf/private true} @@ -120,9 +116,9 @@ dstate (mf/deref refs/dashboard-local) edit-id (:project-for-edit dstate) - local (mf/use-state {:menu-open false - :menu-pos nil - :edition (= (:id project) edit-id)}) + local (mf/use-state {:edition (= (:id project) edit-id)}) + menu-open* (mf/use-state false) + context-menu-open* (mf/use-state false) [rowref limit] (hooks/use-dynamic-grid-item-width) @@ -144,24 +140,11 @@ (mf/use-fn (fn [event] (dom/prevent-default event) - - (let [client-position (dom/get-client-position event) - position (if (and (nil? (:y client-position)) (nil? (:x client-position))) - (let [target-element (dom/get-target event) - points (dom/get-bounding-rect target-element) - y (:top points) - x (:left points)] - (gpt/point x y)) - client-position)] - (swap! local assoc - :menu-open true - :menu-pos position)))) - - on-menu-close - (mf/use-fn #(swap! local assoc :menu-open false)) + (swap! menu-open* not))) on-edit-open - (mf/use-fn #(swap! local assoc :edition true)) + (mf/use-fn + #(swap! local assoc :edition true)) on-edit (mf/use-fn @@ -194,7 +177,14 @@ (fn [_] (create-file "dashboard:grid-header-plus-button"))) - on-import + file-input + (mf/use-ref nil) + + on-import-click + (mf/use-fn + #(dom/click (mf/ref-val file-input))) + + on-finish-import (mf/use-fn (mf/deps project-id team-id) (fn [] @@ -204,14 +194,14 @@ (dd/clear-selected-files)))) handle-create-click - (mf/use-callback + (mf/use-fn (mf/deps on-create-click) (fn [event] (when (kbd/enter? event) (on-create-click event)))) handle-menu-click - (mf/use-callback + (mf/use-fn (mf/deps on-menu-click) (fn [event] (when (kbd/enter? event) @@ -225,15 +215,22 @@ [:& inline-edition {:content (:name project) :on-end on-edit :max-length 250}] - [:h2 {:on-click on-nav - :class (stl/css :project-name) - :title (if (:is-default project) - (tr "labels.drafts") - (:name project)) - :on-context-menu (when can-edit on-menu-click)} - (if (:is-default project) - (tr "labels.drafts") - (:name project))]) + [:> context-menu* {:aria-label (tr "dashboard.options") + :is-disabled (not can-edit) + :on-open-change #(reset! context-menu-open* %) + :trigger + (mf/html + [:h2 {:on-click on-nav + :class (stl/css :project-name) + :title (if (:is-default project) + (tr "labels.drafts") + (:name project))} + (if (:is-default project) + (tr "labels.drafts") + (:name project))])} + [:> project-menu-items* {:project project + :on-edit on-edit-open + :on-import-click on-import-click}]]) [:div {:class (stl/css :info-wrapper)} @@ -246,40 +243,40 @@ [:span {:class (stl/css :recent-files-row-title-info)} (str ", " time)])] [:div {:class (stl/css-case :project-actions true - :pinned-project (:is-pinned project))} + :pinned-project (:is-pinned project) + :is-force-display (or (deref menu-open*) + (deref context-menu-open*)))} (when-not (:is-default project) - [:> pin-button* {:class (stl/css :pin-button) - :is-pinned (:is-pinned project) - :on-click toggle-pin - :tab-index 0}]) + [:> icon-button* {:icon i/pin + :variant "ghost" + :aria-label (tr "dashboard.pin-unpin") + :aria-pressed (:is-pinned project) + :on-click toggle-pin + :tab-index 0}]) (when ^boolean can-edit - [:button {:class (stl/css :add-file-btn) - :on-click on-create-click - :title (tr "dashboard.new-file") - :aria-label (tr "dashboard.new-file") - :data-testid "project-new-file" - :on-key-down handle-create-click} - add-icon]) + [:> icon-button* {:icon i/add + :variant "ghost" + :aria-label (tr "dashboard.new-file") + :on-click on-create-click + :on-key-down handle-create-click}]) (when ^boolean can-edit - [:button {:class (stl/css :options-btn) - :on-click on-menu-click - :title (tr "dashboard.options") - :aria-label (tr "dashboard.options") - :data-testid "project-options" - :on-key-down handle-menu-click} - menu-icon])] - - (when ^boolean can-edit - [:> project-menu* - {:project project - :show (:menu-open @local) - :left (+ 24 (:x (:menu-pos @local))) - :top (:y (:menu-pos @local)) - :on-edit on-edit-open - :on-close on-menu-close - :on-import on-import}])] + [:> project-menu* {:project project + :is-open (deref menu-open*) + :on-open-change #(reset! menu-open* %) + :on-edit on-edit-open + :on-import-click on-import-click + :placement "bottom start" + :trigger + (mf/html + [:> icon-button* {:icon i/menu + :variant "ghost" + :aria-label (tr "dashboard.options") + :aria-pressed (deref menu-open*) + :data-testid "project-options" + :on-click on-menu-click + :on-key-down handle-menu-click}])}])]] (when (and (> limit 0) (> file-count limit)) @@ -289,8 +286,18 @@ :on-key-down (fn [event] (when (kbd/enter? event) (on-nav)))} - [:span {:class (stl/css :placeholder-label)} (tr "dashboard.show-all-files")] - show-more-icon])]] + (tr "dashboard.show-all-files") + [:> icon* {:icon-id i/arrow-right}]])]] + + ;; Kept mounted for as long as this row is, shared by both menu + ;; instances above: the popover each renders inside really unmounts its + ;; content on close (unlike the old context-menu-a11y, which just hid + ;; it), and selecting "Import" closes the menu in the same tick — a ref + ;; owned inside either popover could already be gone by the time its + ;; own click handler fires. + [:> udi/import-form* {:ref file-input + :project-id project-id + :on-finish-import on-finish-import}] [:div {:class (stl/css :grid-container) :ref rowref} (if ^boolean empty? diff --git a/frontend/src/app/main/ui/dashboard/projects.scss b/frontend/src/app/main/ui/dashboard/projects.scss index 1fd92ca225..e60171caea 100644 --- a/frontend/src/app/main/ui/dashboard/projects.scss +++ b/frontend/src/app/main/ui/dashboard/projects.scss @@ -46,8 +46,7 @@ position: relative; &:hover, - &:focus, - &:focus-within { + &:has(:focus-visible) { --actions-opacity: 1; } } @@ -56,6 +55,14 @@ --actions-opacity: 1; } +// Set while either the "..." menu or the title's right-click menu is open — +// its popover can portal outside .dashboard-project-row, so :hover/:focus- +// within alone stop applying the moment the pointer leaves the row, even +// though the menu itself is still open. +.is-force-display { + --actions-opacity: 1; +} + .projects-container { display: grid; grid-auto-rows: min-content; @@ -118,23 +125,7 @@ display: flex; opacity: var(--actions-opacity); margin-inline-start: var(--sp-xxxl); -} - -.add-file-btn, -.options-btn { - @extend %button-tertiary; - - block-size: $sz-32; - inline-size: $sz-32; - margin: 0 var(--sp-s); - padding: var(--sp-s); -} - -.add-icon, -.menu-icon { - @extend %button-icon; - - stroke: var(--icon-foreground); + gap: var(--sp-l); } .grid-container { @@ -168,13 +159,6 @@ } } -.show-more-icon { - block-size: $sz-16; - inline-size: $sz-16; - fill: none; - stroke: var(--show-more-color); -} - // Team hero .team-hero { background-color: var(--color-background-tertiary); @@ -226,25 +210,9 @@ } .close { - --close-icon-foreground-color: var(--icon-foreground); - position: absolute; top: var(--sp-xl); - inset-inline-end: var(--sp-xxl); - inline-size: var(--sp-xxl); - background-color: transparent; - border: none; - cursor: pointer; - - &:hover { - --close-icon-foreground-color: var(--button-icon-foreground-color-selected); - } -} - -.close-icon { - @extend %button-icon; - - stroke: var(--close-icon-foreground-color); + right: var(--sp-xl); } .img-wrapper { diff --git a/frontend/src/app/main/ui/dashboard/sidebar.cljs b/frontend/src/app/main/ui/dashboard/sidebar.cljs index f3e0673ceb..fe683c8273 100644 --- a/frontend/src/app/main/ui/dashboard/sidebar.cljs +++ b/frontend/src/app/main/ui/dashboard/sidebar.cljs @@ -29,7 +29,7 @@ [app.main.ui.dashboard.check-updates :as dcu] [app.main.ui.dashboard.comments :refer [comments-icon* comments-section]] [app.main.ui.dashboard.inline-edition :refer [inline-edition]] - [app.main.ui.dashboard.project-menu :refer [project-menu*]] + [app.main.ui.dashboard.project-menu :refer [project-menu-items*]] [app.main.ui.dashboard.subscription :refer [dashboard-cta* get-subscription-type menu-team-icon* @@ -41,6 +41,7 @@ [app.main.ui.ds.buttons.button :refer [button*]] [app.main.ui.ds.foundations.assets.icon :refer [icon*] :as i] [app.main.ui.ds.foundations.assets.raw-svg :refer [raw-svg*]] + [app.main.ui.ds.layout.menu :refer [context-menu*]] [app.main.ui.hooks :as hooks :refer [use-focus-timer-ref]] [app.main.ui.icons :as deprecated-icon] [app.main.ui.nitrate.nitrate-form] @@ -113,9 +114,7 @@ edit-id (:project-for-edit dstate) local* (mf/use-state - #(do {:menu-open false - :menu-pos nil - :edition? (= (:id item) edit-id) + #(do {:edition? (= (:id item) edit-id) :dragging? false})) local (deref local*) @@ -139,18 +138,6 @@ (st/emit! (dcm/go-to-dashboard-files :project-id project-id))))) - on-menu-click - (mf/use-fn - (fn [event] - (let [position (dom/get-client-position event)] - (dom/prevent-default event) - (swap! local* assoc - :menu-open true - :menu-pos position)))) - - on-menu-close - (mf/use-fn #(swap! local* assoc :menu-open false)) - on-edit-open (mf/use-fn #(swap! local* assoc :edition? true)) @@ -203,30 +190,26 @@ mdata {:on-success on-drop-success}] (st/emit! (dd/move-files (with-meta data mdata)))))))] - [:* - [:li {:tab-index "0" - :class (stl/css-case :project-element true - :sidebar-nav-item true - :current is-selected - :dragging (:dragging? local)) - :on-click on-click - :on-key-down on-key-down - :on-double-click on-edit-open - :on-context-menu on-menu-click - :on-drag-enter on-drag-enter - :on-drag-over on-drag-over - :on-drag-leave on-drag-leave - :on-drop on-drop} - (if (:edition? local) - [:& inline-edition {:content (:name item) - :on-end on-edit}] - [:span {:class (stl/css :element-title)} (:name item)])] - [:> project-menu* {:project item - :show (:menu-open local) - :left (:x (:menu-pos local)) - :top (:y (:menu-pos local)) - :on-edit on-edit-open - :on-close on-menu-close}]])) + [:> context-menu* {:aria-label (tr "dashboard.options") + :trigger + (mf/html + [:li {:tab-index "0" + :class (stl/css-case :project-element true + :sidebar-nav-item true + :current is-selected + :dragging (:dragging? local)) + :on-click on-click + :on-key-down on-key-down + :on-double-click on-edit-open + :on-drag-enter on-drag-enter + :on-drag-over on-drag-over + :on-drag-leave on-drag-leave + :on-drop on-drop} + (if (:edition? local) + [:& inline-edition {:content (:name item) + :on-end on-edit}] + [:span {:class (stl/css :element-title)} (:name item)])])} + [:> project-menu-items* {:project item :on-edit on-edit-open}]])) (mf/defc sidebar-search* {::mf/private true} diff --git a/frontend/src/app/main/ui/ds.cljs b/frontend/src/app/main/ui/ds.cljs index 319a4c8178..5802dd2dde 100644 --- a/frontend/src/app/main/ui/ds.cljs +++ b/frontend/src/app/main/ui/ds.cljs @@ -25,6 +25,7 @@ [app.main.ui.ds.foundations.typography.text :refer [text*]] [app.main.ui.ds.foundations.utilities.token.token-status :refer [token-status-icon* token-status-list]] + [app.main.ui.ds.layout.menu :refer [menu* menu-item* menu-separator* sub-menu* context-menu*]] [app.main.ui.ds.layout.modal :refer [modal* modal-header* modal-content* modal-footer*]] [app.main.ui.ds.layout.tab-switcher :refer [tab-switcher*]] [app.main.ui.ds.notifications.actionable :refer [actionable*]] @@ -88,6 +89,11 @@ :ModalHeader modal-header* :ModalContent modal-content* :ModalFooter modal-footer* + :Menu menu* + :MenuItem menu-item* + :MenuSeparator menu-separator* + :SubMenu sub-menu* + :ContextMenu context-menu* :set-default-translations (fn [data] diff --git a/frontend/src/app/main/ui/ds/layout/context_menu.mdx b/frontend/src/app/main/ui/ds/layout/context_menu.mdx new file mode 100644 index 0000000000..22ee06fa1e --- /dev/null +++ b/frontend/src/app/main/ui/ds/layout/context_menu.mdx @@ -0,0 +1,129 @@ +{ /* This Source Code Form is subject to the terms of the Mozilla Public + License, v. 2.0. If a copy of the MPL was not distributed with this + file, You can obtain one at http://mozilla.org/MPL/2.0/. + + Copyright (c) KALEIDOS SUBSIDIARY SL */ } +import { Canvas, Meta } from "@storybook/addon-docs/blocks"; +import * as ContextMenu from "./context_menu.stories"; + + + +# Context Menu + +A context menu displays a list of actions or options tied to a specific area of the interface, opened with a right click (or long press on touch) instead of clicking a visible trigger button. It's positioned at the pointer, growing down and to the right when there's room, and flipping to grow upward when it isn't. + +## Example + +### Default + + + +--- + +# Usage + +```clojure +[:> context-menu* + {:aria-label (tr "workspace.shape.menu.title") + :on-action (fn [key] (handle-action key)) + :trigger [:> layer-row* {:shape shape}]} + + [:> menu-item* {:id "rename"} "Rename"] + [:> menu-item* {:id "duplicate"} "Duplicate"] + [:> menu-separator*] + [:> menu-item* {:id "delete"} "Delete"]] +``` + +`trigger` is the area that responds to a right click — it can be any content, not just a button; it's rendered as-is (no extra box around it) and only gains a right-click listener. The menu itself is entirely self-contained: opening, positioning, and closing (selection, **Escape**, outside click) are all handled internally, no `is-open` plumbing required from the caller. + +--- + +# Context menu props +## trigger + +The area that opens the menu on right click. Rendered without adding any wrapping box to the layout. + +Type: React element + +## aria-label + +Accessible name for the menu, read by screen readers. Required — a context menu has no visible trigger button to derive a label from. + +Type: string + +## on-action + +Callback invoked with the selected item's `id` when an item is chosen. + +Type: function + +## placement + +Controls where the menu grows from the click point. + +Options + +"top", "top start", "top end", "bottom", "bottom start" (default), "bottom end", "left", "left top", "left bottom", "right", "right top", "right bottom" + +The menu flips to the opposite vertical side automatically when there isn't enough room in the preferred direction — e.g. `"bottom start"` (grows right and down) becomes `"top start"` (grows right and up) near the bottom of the viewport. + +## is-disabled + +Prevents the context menu from opening. + +Default: false + +## on-open-change + +Called whenever the menu opens or closes. The context menu owns its open state — unlike Menu, no `is-open` plumbing is required to use it — this is only a notification, e.g. to keep a trigger's own hover-only affordances visible for as long as the menu stays open. + +Type: function + +## max-width + +Caps how wide the menu (and every flyout submenu nested in it) can grow. The existing minimum width still wins if it's larger than this. + +Type: number (pixels) | string (any CSS length) + +Default: 250 + +## is-dense + +Shrinks every item — this menu's own and every nested flyout submenu's — to a 28px row, for lists dense enough that the default 32px adds up. + +Default: false + +## class + +Additional CSS class applied to the menu. + +--- + +# MenuItem props + +Same as the [Menu](/docs/layout-menu--docs) component's `menu-item*` and `menu-separator*`. + +--- + +# Accessibility + +The context menu automatically provides: + +- Accessible `menu`/`menuitem` semantics +- Full keyboard navigation (arrow keys, Home/End, typeahead) +- Focus management and restoration when closed +- Dismissal with **Escape** and outside click +- Long-press support on touch devices, where right click doesn't exist + +--- + +# Best practices + +Use a context menu for: + +- Actions tied to a specific item (a layer, a file, a row) that don't need a persistently visible trigger + +Avoid using a context menu for: + +- The *only* way to reach an action — right click isn't discoverable; pair it with a visible menu trigger (see [Menu](/docs/layout-menu--docs)) or keyboard shortcut for the same actions when possible +- A long list of unrelated actions — keep it scoped to the item that was clicked diff --git a/frontend/src/app/main/ui/ds/layout/context_menu.stories.jsx b/frontend/src/app/main/ui/ds/layout/context_menu.stories.jsx new file mode 100644 index 0000000000..6ede566c2a --- /dev/null +++ b/frontend/src/app/main/ui/ds/layout/context_menu.stories.jsx @@ -0,0 +1,115 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. +// +// Copyright (c) KALEIDOS SUBSIDIARY SL + +import Components from "@target/components"; +import { + fireEvent, + screen, + userEvent, + waitFor, + within, + expect, +} from "storybook/test"; + +const { ContextMenu, MenuItem, MenuSeparator } = Components; + +const ContextMenuWrapper = ({ children, ...props }) => { + return ( + + Right click here +
+ } + > + {children} + + ); +}; + +export default { + title: "Layout/Context Menu", + component: ContextMenuWrapper, + args: { + "aria-label": "Item actions", + placement: "bottom start", + onAction: (key) => console.log("action", key), + children: ( + <> + Rename + Duplicate + + Delete + + ), + }, + argTypes: { + placement: { + control: "select", + options: [ + "top", + "top start", + "top end", + "bottom", + "bottom start", + "bottom end", + "left", + "left top", + "left bottom", + "right", + "right top", + "right bottom", + ], + }, + isDisabled: { control: "boolean" }, + }, + parameters: { + controls: { exclude: ["trigger", "children"] }, + }, + render: ({ ...args }) => , +}; + +export const Default = {}; + +export const Disabled = { + args: { + isDisabled: true, + }, +}; + +const openChangeCalls = []; + +export const TestOnOpenChangeReportsState = { + args: { + onOpenChange: (isOpen) => openChangeCalls.push(isOpen), + }, + play: async ({ canvasElement, step }) => { + openChangeCalls.length = 0; + const trigger = within(canvasElement).getByText("Right click here"); + + await step("Right-clicking the trigger reports it open", async () => { + fireEvent.contextMenu(trigger); + await screen.findByRole("menu"); + await waitFor(() => expect(openChangeCalls).toEqual([true])); + }); + + await step("Escape reports it closed", async () => { + await userEvent.keyboard("{Escape}"); + await waitFor(() => expect(openChangeCalls).toEqual([true, false])); + }); + }, +}; diff --git a/frontend/src/app/main/ui/ds/layout/menu.cljs b/frontend/src/app/main/ui/ds/layout/menu.cljs new file mode 100644 index 0000000000..12a9bd7f73 --- /dev/null +++ b/frontend/src/app/main/ui/ds/layout/menu.cljs @@ -0,0 +1,136 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns app.main.ui.ds.layout.menu + (:require + ["@penpot/ui/menu" :as menu] + [app.common.data :as d] + [rumext.v2 :as mf])) + +(def ^:private schema:menu + [:map + [:class {:optional true} [:maybe :string]] + [:is-open {:optional true} [:maybe :boolean]] + [:on-open-change {:optional true} [:maybe fn?]] + [:trigger {:optional true} [:maybe :any]] + [:placement {:optional true} + [:maybe [:enum "top" "top start" "top end" + "bottom" "bottom start" "bottom end" + "left" "left top" "left bottom" + "right" "right top" "right bottom"]]] + [:on-action {:optional true} [:maybe fn?]] + [:max-width {:optional true} [:maybe [:or :int :string]]] + [:is-dense {:optional true} [:maybe :boolean]]]) + +(mf/defc menu* + {::mf/schema schema:menu} + [{:keys [class is-open on-open-change trigger placement on-action max-width is-dense children] :rest props}] + (let [placement (d/nilv placement "bottom start") + props + (mf/spread-props props + {:class class + :is-open is-open + :on-open-change on-open-change + :trigger trigger + :placement placement + :on-action on-action + :max-width max-width + :is-dense is-dense})] + [:> menu/Menu props + children])) + +(def ^:private schema:menu-item + [:map + [:id {:optional true} [:maybe [:or :string :int]]] + [:class {:optional true} [:maybe :string]] + [:is-disabled {:optional true} [:maybe :boolean]] + [:on-action {:optional true} [:maybe fn?]] + [:text-value {:optional true} [:maybe :string]]]) + +(mf/defc menu-item* + {::mf/schema schema:menu-item} + [{:keys [id class is-disabled on-action text-value children] :rest props}] + (let [props + (mf/spread-props props + {:id id + :class class + :is-disabled is-disabled + :on-action on-action + :text-value text-value})] + [:> menu/MenuItem props + children])) + +(def ^:private schema:sub-menu + [:map + [:id {:optional true} [:maybe [:or :string :int]]] + [:class {:optional true} [:maybe :string]] + [:trigger {:optional true} [:maybe :any]] + [:is-disabled {:optional true} [:maybe :boolean]] + [:text-value {:optional true} [:maybe :string]] + [:on-action {:optional true} [:maybe fn?]] + [:variant {:optional true} [:maybe [:enum "flyout" "drilldown"]]] + [:max-width {:optional true} [:maybe [:or :int :string]]]]) + +(mf/defc sub-menu* + {::mf/schema schema:sub-menu} + [{:keys [id class trigger is-disabled text-value on-action variant max-width children] :rest props}] + (let [variant (d/nilv variant "flyout") + props + (mf/spread-props props + {:id id + :class class + :trigger trigger + :is-disabled is-disabled + :text-value text-value + :on-action on-action + :variant variant + :max-width max-width})] + [:> menu/SubMenu props + children])) + +(def ^:private schema:menu-separator + [:map + [:class {:optional true} [:maybe :string]]]) + +(mf/defc menu-separator* + {::mf/schema schema:menu-separator} + [{:keys [class] :rest props}] + (let [props (mf/spread-props props {:class class})] + [:> menu/MenuSeparator props])) + +(def ^:private schema:context-menu + [:map + [:class {:optional true} [:maybe :string]] + [:aria-label :string] + [:trigger {:optional true} [:maybe :any]] + [:placement {:optional true} + [:maybe [:enum "top" "top start" "top end" + "bottom" "bottom start" "bottom end" + "left" "left top" "left bottom" + "right" "right top" "right bottom"]]] + [:is-disabled {:optional true} [:maybe :boolean]] + [:on-action {:optional true} [:maybe fn?]] + [:max-width {:optional true} [:maybe [:or :int :string]]] + [:is-dense {:optional true} [:maybe :boolean]] + [:on-open-change {:optional true} [:maybe fn?]]]) + +(mf/defc context-menu* + {::mf/schema schema:context-menu} + [{:keys [class aria-label trigger placement is-disabled on-action max-width is-dense on-open-change children] :rest props}] + (let [placement (d/nilv placement "bottom start") + props + (mf/spread-props props + {:class class + :aria-label aria-label + :trigger trigger + :placement placement + :is-disabled is-disabled + :on-action on-action + :max-width max-width + :is-dense is-dense + :on-open-change on-open-change})] + [:> menu/ContextMenu props + children])) diff --git a/frontend/src/app/main/ui/ds/layout/menu.mdx b/frontend/src/app/main/ui/ds/layout/menu.mdx new file mode 100644 index 0000000000..8b00573e94 --- /dev/null +++ b/frontend/src/app/main/ui/ds/layout/menu.mdx @@ -0,0 +1,246 @@ +{ /* This Source Code Form is subject to the terms of the Mozilla Public + License, v. 2.0. If a copy of the MPL was not distributed with this + file, You can obtain one at http://mozilla.org/MPL/2.0/. + + Copyright (c) KALEIDOS SUBSIDIARY SL */ } +import { Canvas, Meta } from "@storybook/addon-docs/blocks"; +import * as Menu from "./menu.stories"; + + + +# Menu + +A menu displays a list of actions or options that a user can choose from. It opens next to a trigger element and closes on selection, **Escape**, or an outside click. Use it to build dropdowns and context menus. + +## Example + +### Default + + + +### With a submenu + + + +### With a drilldown submenu + + + +### Dense + + + +### With a max width + + + +--- + +# Usage + +```clojure +(let [open* (mf/use-state false) + open (deref open*) + trigger-ref (mf/use-ref nil) + on-ref (mf/use-fn (fn [node] (mf/set-ref-val! trigger-ref node))) + on-open (mf/use-fn (fn [] (reset! open* true))) + on-open-change (mf/use-fn (fn [open] (reset! open* open))) + on-action (mf/use-fn (fn [key] (handle-action key)))] + [:> menu* + {:is-open open + :on-open-change on-open-change + :trigger (mf/html + [:> button* {:variant "secondary" + :on-ref on-ref + :on-click on-open} + "Open menu"]) + :on-action on-action} + + [:> menu-item* {:id "rename"} "Rename"] + [:> menu-item* {:id "duplicate"} "Duplicate"] + [:> menu-separator*] + [:> menu-item* {:id "delete"} "Delete"]]) +``` + +`is-open` and `on-open-change` are controlled by the caller: the trigger's `on-click` opens the menu, and `on-open-change` reports closes from selection, **Escape**, or an outside click. For a context menu, skip `trigger`/`on-click` and drive `is-open` from your own `on-context-menu` handler instead. + +--- + +# Menu props +## trigger + +Element that opens the menu. + +Type: React element + +## is-open + +Controls whether the menu is open. + +Type: boolean + +## on-open-change + +Callback invoked whenever the open state changes. + +Type: function + +## on-action + +Callback invoked with the selected item's `id` when an item is chosen. + +Type: function + +## placement + +Controls where the menu opens relative to its trigger. + +Options + +"top", "top start", "top end", "bottom", "bottom start" (default), "bottom end", "left", "left top", "left bottom", "right", "right top", "right bottom" + +## max-width + +Caps how wide the menu (and every flyout submenu nested in it) can grow. The existing minimum width still wins if it's larger than this. + +Type: number (pixels) | string (any CSS length) + +Default: 250 + +## is-dense + +Shrinks every item — this menu's own and every nested flyout submenu's — to a 28px row, for lists dense enough that the default 32px adds up. + +Default: false + +## class + +Additional CSS class applied to the menu. + +--- + +# MenuItem props +## id + +Unique identifier for the item, passed to `on-action` when selected. + +Type: string | number + +## is-disabled + +Prevents the item from being selected. + +Default: false + +## on-action + +Callback invoked when this specific item is selected. + +Type: function + +## text-value + +Plain-text representation of the item, used for typeahead. Required when the item's content isn't a plain string. + +Type: string + +## class + +Additional CSS class applied to the item. + +--- + +# SubMenu props + +A `sub-menu*` nests a further list of items behind one item, opening on hover or when navigated into with the keyboard. Use it in place of `menu-item*` for that item: + +```clojure +[:> sub-menu* {:trigger "Share" :on-action (fn [key] (handle-action key))} + [:> menu-item* {:id "share-link"} "Copy link"] + [:> menu-item* {:id "share-email"} "Send by email"]] +``` + +Selecting any item inside a submenu closes the whole menu, not just that submenu. + +## variant + +`"flyout"` (default) opens a nested popover next to the trigger item. `"drilldown"` replaces the parent menu's content with this submenu's items and adds a back item — use it for deep or wide trees, e.g. move-to-project's team → project nesting. Nested drilldowns keep drilling into the same list, which is never sized smaller than the root level it was entered from. + +```clojure +[:> sub-menu* {:trigger "Move to" :variant "drilldown"} + [:> menu-item* {:id "project-a"} "Project A"] + [:> sub-menu* {:trigger "Other team" :variant "drilldown"} + [:> menu-item* {:id "project-b"} "Project B"]]] +``` + +Options + +"flyout" (default), "drilldown" + +## trigger + +Label shown for the item that opens the submenu. + +Type: React element + +## id + +Unique identifier for the submenu trigger item. + +Type: string | number + +## is-disabled + +Prevents the submenu from being opened. + +Default: false + +## on-action + +Callback invoked with the selected item's `id` for actions inside this submenu. Ignored when `variant` is `"drilldown"` — those items use the parent menu's `on-action` instead. + +Type: function + +## text-value + +Plain-text representation of the trigger, used for typeahead. Required when `trigger` isn't a plain string. + +Type: string + +## max-width + +Only meaningful for the `"flyout"` variant: its nested popover is its own menu, independent of the root's. A `"drilldown"` submenu has no popover of its own to size — it renders into the root's, sized by the root Menu/ContextMenu's own `max-width` instead. + +Type: number (pixels) | string (any CSS length) + +Default: 250 + +## class + +Additional CSS class applied to the submenu's item list. + +--- + +# Accessibility + +The menu automatically provides: + +- Accessible `menu`/`menuitem` semantics +- Full keyboard navigation (arrow keys, Home/End, typeahead) +- Focus management and restoration when closed +- Dismissal with **Escape** and outside click + +--- + +# Best practices + +Use a menu for: + +- A list of actions tied to a trigger button (dropdown) +- A contextual list of actions tied to a right click (context menu) + +Avoid using a menu for: + +- Long forms or free-text input +- Navigation between pages (use a link/nav component instead) +- A single toggleable option (use a switch or checkbox instead) diff --git a/frontend/src/app/main/ui/ds/layout/menu.stories.jsx b/frontend/src/app/main/ui/ds/layout/menu.stories.jsx new file mode 100644 index 0000000000..2f46d4ecce --- /dev/null +++ b/frontend/src/app/main/ui/ds/layout/menu.stories.jsx @@ -0,0 +1,437 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. +// +// Copyright (c) KALEIDOS SUBSIDIARY SL + +import * as React from "react"; +import Components from "@target/components"; +import { + userEvent, + fireEvent, + within, + screen, + waitFor, + expect, +} from "storybook/test"; + +const { Menu, MenuItem, MenuSeparator, SubMenu, Button } = Components; + +const MenuWrapper = ({ children, ...props }) => { + const [open, setOpen] = React.useState(props.isOpen ?? false); + + React.useEffect(() => { + setOpen(props.isOpen ?? false); + }, [props.isOpen]); + + return ( + setOpen((open) => !open)}> + Open menu + + } + > + {children} + + ); +}; + +export default { + title: "Layout/Menu", + component: MenuWrapper, + args: { + placement: "bottom start", + onAction: (key) => console.log("action", key), + children: ( + <> + Rename + Duplicate + + Delete + + ), + }, + argTypes: { + placement: { + control: "select", + options: [ + "top", + "top start", + "top end", + "bottom", + "bottom start", + "bottom end", + "left", + "left top", + "left bottom", + "right", + "right top", + "right bottom", + ], + }, + maxWidth: { + control: { type: "number" }, + }, + isDense: { + control: { type: "boolean" }, + }, + }, + parameters: { + controls: { exclude: ["isOpen", "onOpenChange", "trigger", "children"] }, + }, + render: ({ ...args }) => , +}; + +export const Default = {}; + +export const WithDisabledItem = { + args: { + children: ( + <> + Rename + + Duplicate + + + Delete + + ), + }, +}; + +const subMenuActionCalls = []; + +const subMenuChildren = ( + <> + Rename + subMenuActionCalls.push(key)}> + Copy link + Send by email + + + Delete + +); + +const drilldownChildren = ( + <> + Rename + Duplicate + + + Project A + Project B + + + Project A + Project B + + + Project A + + + + + Delete + +); + +export const WithSubMenu = { + args: { children: subMenuChildren }, +}; + +export const WithDrilldownSubMenu = { + args: { children: drilldownChildren }, +}; + +export const Dense = { + args: { isDense: true }, +}; + +export const WithMaxWidth = { + args: { + maxWidth: 160, + children: ( + <> + Rename this file completely + Duplicate + + Delete + + ), + }, +}; + +export const Placement = { + args: { + placement: "right", + }, + decorators: [ + // Absolutely-positioned + transform centering, rather than flex + // align-items, because the trigger's own align-self: start (needed so + // it doesn't get stretched by a real flex/grid ancestor elsewhere) + // would otherwise override a flex parent's centering here too. + (Story) => ( +
+
+ +
+
+ ), + ], +}; + +// The popover portals out of the story root, so the menu itself is only +// reachable through screen (document-wide); the trigger stays in the canvas. +const getTrigger = (canvasElement) => + within(canvasElement).getByRole("button", { name: /open menu/i }); + +const expectMenuClosed = () => + waitFor(() => expect(screen.queryByRole("menu")).not.toBeInTheDocument()); + +export const TestTriggerTogglesMenuClosed = { + play: async ({ canvasElement, step }) => { + const trigger = getTrigger(canvasElement); + + await step("Clicking the trigger opens the menu", async () => { + await userEvent.click(trigger); + await screen.findByRole("menu"); + }); + + // The trigger sits outside the popover, so a naive outside-click dismiss + // closes on its pointerdown and lets the click reopen it. + await step("Clicking the trigger again closes the menu", async () => { + await userEvent.click(trigger); + await expectMenuClosed(); + }); + }, +}; + +export const TestFlyoutSubMenuIsNotOutside = { + args: { children: subMenuChildren }, + play: async ({ canvasElement, step }) => { + const trigger = getTrigger(canvasElement); + subMenuActionCalls.length = 0; + + await step("Hovering the submenu trigger opens the flyout", async () => { + await userEvent.click(trigger); + await userEvent.hover( + await screen.findByRole("menuitem", { name: "Share" }), + ); + await screen.findByRole("menuitem", { name: "Copy link" }); + }); + + // react-aria portals a SubmenuTrigger's popover into the root popover's + // container, making the flyout a sibling of the root popover rather than + // a descendant. Testing containment against the root popover alone + // therefore counts a press anywhere in the flyout as an outside click and + // dismisses the whole menu. + await step("A press inside the flyout does not dismiss", async () => { + const [, flyout] = screen.getAllByRole("menu"); + fireEvent.pointerDown(flyout); + + await waitFor(() => + expect( + screen.getByRole("menuitem", { name: "Copy link" }), + ).toBeInTheDocument(), + ); + }); + + await step("Selecting a flyout item fires its action", async () => { + await userEvent.click( + screen.getByRole("menuitem", { name: "Copy link" }), + ); + await waitFor(() => expect(subMenuActionCalls).toEqual(["share-link"])); + }); + + await step("Selecting it closes the whole tree", expectMenuClosed); + }, +}; + +export const TestDrilldownNavigatesAndReturns = { + args: { children: drilldownChildren }, + play: async ({ canvasElement, step }) => { + const trigger = getTrigger(canvasElement); + + await step("Drilling in replaces the menu's own content", async () => { + await userEvent.click(trigger); + await userEvent.click( + await screen.findByRole("menuitem", { name: "Move to" }), + ); + + await screen.findByRole("menuitem", { name: "Project A" }); + expect( + screen.queryByRole("menuitem", { name: "Rename" }), + ).not.toBeInTheDocument(); + }); + + await step("The back item returns to the level entered from", async () => { + await userEvent.click(screen.getByRole("menuitem", { name: /move to/i })); + + await screen.findByRole("menuitem", { name: "Rename" }); + expect( + screen.queryByRole("menuitem", { name: "Project A" }), + ).not.toBeInTheDocument(); + }); + }, +}; + +export const TestDrilldownResetsBetweenOpens = { + args: { children: drilldownChildren }, + play: async ({ canvasElement, step }) => { + const trigger = getTrigger(canvasElement); + + await step("Drill into a submenu, then close the menu", async () => { + await userEvent.click(trigger); + await userEvent.click( + await screen.findByRole("menuitem", { name: "Move to" }), + ); + await screen.findByRole("menuitem", { name: "Project A" }); + + await userEvent.keyboard("{Escape}"); + await expectMenuClosed(); + }); + + await step("Reopening starts back at the root level", async () => { + await userEvent.click(trigger); + + await screen.findByRole("menuitem", { name: "Rename" }); + expect( + screen.queryByRole("menuitem", { name: "Project A" }), + ).not.toBeInTheDocument(); + }); + }, +}; + +export const TestClosesOnEscapeAndOutsideClick = { + play: async ({ canvasElement, step }) => { + const trigger = getTrigger(canvasElement); + + await step("Escape closes the menu", async () => { + await userEvent.click(trigger); + await screen.findByRole("menu"); + + await userEvent.keyboard("{Escape}"); + await expectMenuClosed(); + }); + + await step("A click outside closes the menu", async () => { + await userEvent.click(trigger); + await screen.findByRole("menu"); + + await userEvent.click(document.body); + await expectMenuClosed(); + }); + }, +}; + +export const TestMaxWidthCapsPopoverWidth = { + args: { + maxWidth: 160, + children: ( + <> + Rename this file completely + + Delete + + ), + }, + play: async ({ canvasElement, step }) => { + const trigger = getTrigger(canvasElement); + + await step("The popover never grows past maxWidth", async () => { + await userEvent.click(trigger); + const menu = await screen.findByRole("menu"); + + await waitFor(() => + expect(menu.getBoundingClientRect().width).toBeLessThanOrEqual(160), + ); + }); + }, +}; + +export const TestDrilldownFreezesRootEdgeAndSizesToContent = { + args: { + placement: "top start", + // Deliberately taller at the root than the level drilled into, so a + // regression (re-running react-aria's own flip/collision positioning + // against the drilled-in level's own shorter content, rather than + // freezing the edge already resolved for the root) would show up either + // as the popover jumping to the opposite edge, or as it staying put but + // padded out to the root's own height instead of sizing to its content. + children: ( + <> + Rename this file completely + Duplicate + Restore from trash + + + A + + + ), + }, + decorators: [ + // Plenty of room above the trigger, none below — forces "top start" to + // actually resolve with the popover's bottom edge pinned near the + // trigger, instead of react-aria flipping it back to "bottom" for lack + // of room above, which would defeat the point of this test. + (Story) => ( +
+
+ +
+
+ ), + ], + play: async ({ canvasElement, step }) => { + const trigger = getTrigger(canvasElement); + let rootBottom, rootHeight; + + await step("Opening resolves the popover above the trigger", async () => { + await userEvent.click(trigger); + const menu = await screen.findByRole("menu"); + const popover = menu.closest("[data-placement]"); + + await waitFor(() => expect(popover.dataset.placement).toBe("top")); + ({ bottom: rootBottom, height: rootHeight } = + popover.getBoundingClientRect()); + }); + + await step( + "Drilling into a shorter level keeps the same bottom edge and shrinks to fit", + async () => { + await userEvent.click( + screen.getByRole("menuitem", { name: "Move to" }), + ); + const menu = await screen.findByRole("menu"); + await screen.findByRole("menuitem", { name: "A" }); + const popover = menu.closest("[data-placement]"); + + await waitFor(() => { + const rect = popover.getBoundingClientRect(); + // Within a couple of px, not exact: sub-pixel layout rounding + // between the two measurements, not a regression — an actual + // regression (react-aria re-flipping back to "bottom") would move + // this by the popover's full height, hundreds of px. + expect(Math.abs(rect.bottom - rootBottom)).toBeLessThan(2); + expect(rect.height).toBeLessThan(rootHeight); + }); + }, + ); + }, +};