From 0c9cad14955cd049f57e1bd63a3e2e068247674d Mon Sep 17 00:00:00 2001 From: sokumon Date: Mon, 27 Jul 2026 02:33:11 +0530 Subject: [PATCH 1/5] feat: unmount private/custom workspaces from the rail --- frappe/boot.py | 52 +++-- frappe/desk/doctype/workspace/workspace.json | 4 +- frappe/desk/doctype/workspace/workspace.py | 129 ++++++++++- frappe/public/js/frappe/ui/sidebar/sidebar.js | 97 ++++---- .../js/frappe/ui/sidebar/workspace_picker.js | 51 ++--- .../js/frappe/ui/toolbar/search_utils.js | 16 +- .../js/frappe/views/workspace/workspace.js | 213 ++++++++++++++---- 7 files changed, 402 insertions(+), 160 deletions(-) diff --git a/frappe/boot.py b/frappe/boot.py index 554869c08af1..b32a6c7e806a 100644 --- a/frappe/boot.py +++ b/frappe/boot.py @@ -241,12 +241,9 @@ def get_app_rail_host_map(): def load_desktop_data(bootinfo): from frappe.desk.desktop import get_user_workspaces - allowed_pages = [d.name for d in bootinfo.workspaces.get("pages")] - # Companion apps pin their workspaces into a host app's dock (rail) via `add_to_workspace_dock`, - # instead of taking an apps-screen slot of their own. Resolved once, merged per host app below. - rail_map = get_app_rail_map() - # ...and their own workspaces resolve their app context (dock + header) to that host app, so a - # companion app appears to live inside the host's rail rather than flipping to a shell of its own. + # A companion app's workspaces resolve their app context (dock + header) to the host app they + # were pinned into via `add_to_workspace_dock`, so the companion appears to live inside the + # host's rail rather than flipping the desk to a shell of its own. bootinfo.app_rail_host = get_app_rail_host_map() # The user's curated workspace selection (`User.workspaces`), ordered. Kept separate from # `bootinfo.workspaces` (which holds every permitted workspace link) so the workspace selector @@ -255,7 +252,26 @@ def load_desktop_data(bootinfo): bootinfo.workspace_sidebar_item = get_sidebar_items() bootinfo.default_workspace_map = build_default_workspace_map(bootinfo.workspace_sidebar_item) bootinfo.module_wise_workspaces = get_controller("Workspace").get_module_wise_workspaces() - bootinfo.app_data = [] + bootinfo.app_data = get_app_data([d.name for d in bootinfo.workspaces.get("pages")]) + + +def get_app_data(allowed_pages: list[str]) -> list[dict]: + """The apps the desk knows about, each with the workspaces that belong to it. + + This is what backs the apps (desktop) screen and the workspace dock: the dock lists + `app_data[app].workspaces` for whichever app is in context. Kept as its own function so + anything that re-mounts a workspace can hand the client a fresh copy without duplicating + the grouping rules (see `mount_workspace`). + + `allowed_pages` is the set of workspace names the user may see -- `bootinfo.workspaces.pages`, + i.e. every public workspace they're permitted plus their own private ones. + """ + app_data = [] + + # Companion apps pin their workspaces into a host app's dock (rail) via `add_to_workspace_dock`, + # instead of taking an apps-screen slot of their own. Resolved once, merged per host app below. + rail_map = get_app_rail_map() + app_rail_host = get_app_rail_host_map() Workspace = frappe.qb.DocType("Workspace") Module = frappe.qb.DocType("Module Def") @@ -274,7 +290,7 @@ def load_desktop_data(bootinfo): # labelled (e.g. the sidebar header subtitle) instead of falling back to the user's # name. on_apps_screen stays False so it never shows on the apps screen, and an # empty `workspaces` keeps the desk-side lookups from breaking. - bootinfo.app_data.append( + app_data.append( dict( on_apps_screen=False, sequence_id=app_info.get("sequence_id") or DEFAULT_APP_SEQUENCE_ID, @@ -294,9 +310,14 @@ def load_desktop_data(bootinfo): # A workspace belongs to this app if its module is the app's (standard, app-shipped # workspaces) or its `app` field points at it (custom workspaces have no module). Use a - # left join so module-less custom workspaces aren't dropped, and keep only public ones -- - # private workspaces are surfaced separately by the selector's private listing. Ordered by - # `sequence_id` so the dock lists them in the workspace record's configured order. + # left join so module-less custom workspaces aren't dropped. Ordered by `sequence_id` so + # the dock lists them in the workspace record's configured order. + # + # Private workspaces are included on the same footing as public ones: a private workspace + # mounted to an app belongs on that app's dock, and nowhere else. Restricting to the + # session user is belt-and-braces -- `allowed_pages` already covers it, since + # `get_workspaces()` only ever extends its page list with the user's *own* private + # workspaces -- but it keeps the query honest on its own terms. workspaces = [ r[0] for r in ( @@ -305,7 +326,8 @@ def load_desktop_data(bootinfo): .on(Workspace.module == Module.name) .select(Workspace.name) .where( - ((Module.app_name == app_name) | (Workspace.app == app_name)) & (Workspace.public == 1) + ((Module.app_name == app_name) | (Workspace.app == app_name)) + & ((Workspace.public == 1) | (Workspace.for_user == frappe.session.user)) ) .orderby(Workspace.sequence_id) .run() @@ -320,12 +342,12 @@ def load_desktop_data(bootinfo): if rail_workspace in allowed_pages and rail_workspace not in workspaces: workspaces.append(rail_workspace) - bootinfo.app_data.append( + app_data.append( dict( # whether the app opts into the apps screen via the add_to_apps_screen hook. An app # that pins into a host app's dock never takes a slot of its own, even if it still # declares add_to_apps_screen from before the dock existed -- the dock hook wins. - on_apps_screen=bool(apps) and app_name not in bootinfo.app_rail_host, + on_apps_screen=bool(apps) and app_name not in app_rail_host, # Sort order for the apps (desktop) screen; lower shows first, Framework is pinned # last (sequence_id 1000). Apps that don't declare one fall to a middle default. sequence_id=app_info.get("sequence_id") or DEFAULT_APP_SEQUENCE_ID, @@ -356,6 +378,8 @@ def load_desktop_data(bootinfo): ) ) + return app_data + def load_translations(bootinfo): from frappe.translate import get_translation_version diff --git a/frappe/desk/doctype/workspace/workspace.json b/frappe/desk/doctype/workspace/workspace.json index 0846e0348807..fafa65a1a9b5 100644 --- a/frappe/desk/doctype/workspace/workspace.json +++ b/frappe/desk/doctype/workspace/workspace.json @@ -228,7 +228,7 @@ "options": "green\ncyan\nblue\norange\nyellow\ngray\ngrey\nred\npink\ndarkgrey\npurple\nlight-blue" }, { - "depends_on": "eval: doc.standard", + "description": "The app this workspace is mounted to. Decides which app's dock (workspace rail) lists it. A standard workspace inherits this from its module; a custom one must be mounted explicitly, or it appears on no dock at all.", "fieldname": "app", "fieldtype": "Data", "label": "App" @@ -291,7 +291,7 @@ ], "in_create": 1, "links": [], - "modified": "2026-07-06 23:20:00.000000", + "modified": "2026-07-26 19:50:45.000000", "modified_by": "Administrator", "module": "Desk", "name": "Workspace", diff --git a/frappe/desk/doctype/workspace/workspace.py b/frappe/desk/doctype/workspace/workspace.py index 0d5ef1cebf78..00373640af60 100644 --- a/frappe/desk/doctype/workspace/workspace.py +++ b/frappe/desk/doctype/workspace/workspace.py @@ -122,6 +122,12 @@ def validate(self): self.app = get_module_app(self.module) + # `app` is the workspace's mount point -- it decides which app's dock lists it. A bad value + # doesn't error anywhere downstream, it just silently drops the workspace off every dock, + # so reject it here rather than let it strand the workspace. + if self.app and self.app not in frappe.get_installed_apps(): + frappe.throw(_("{0} is not an installed app.").format(frappe.bold(self.app))) + def before_rename(self, old_name, new_name, merge=False): if self.public and not is_workspace_manager() and not disable_saving_as_public(): frappe.throw( @@ -311,6 +317,34 @@ def disable_saving_as_public(): ) +def workspace_payload(**extra): + """The desk state a workspace write invalidates, for the caller to swap into `frappe.boot`. + + `app_data` is in here because the dock is app-scoped: it lists `app_data[app].workspaces`, so + a workspace that just gained or changed its `app` only moves once that mapping is rebuilt. + Without it the desk needs a full reload to show the change. + """ + from frappe.boot import get_app_data + + workspaces = get_workspaces() + return { + "workspace_pages": workspaces, + "sidebar_items": get_sidebar_items(), + "app_data": get_app_data([d.name for d in workspaces.get("pages")]), + **extra, + } + + +def can_edit_workspace(doc): + """Whether the session user may change this workspace's settings (including its app mount). + + A Workspace Manager may edit any workspace; anyone may edit their own private one. The desk + mirrors this predicate to decide whether to offer the "mount to app" action or tell the + viewer to ask a Workspace Manager, so keep the two in step. + """ + return is_workspace_manager() or (not doc.public and doc.for_user == frappe.session.user) + + def get_link_type(key): key = key.lower() @@ -384,8 +418,7 @@ def new_page(new_page: dict): ) doc.save(ignore_permissions=True) - workspaces = get_workspaces() - return {"workspace_pages": workspaces, "sidebar_items": get_sidebar_items()} + return workspace_payload() @frappe.whitelist() @@ -393,7 +426,7 @@ def save_page(name: str, public: str | int, new_widgets: dict, blocks: str): public = frappe.parse_json(public) doc = frappe.get_doc("Workspace", name) - can_edit = is_workspace_manager() or (not doc.public and doc.for_user == frappe.session.user) + can_edit = can_edit_workspace(doc) if not can_edit: frappe.throw( _("You need the Workspace Manager role to edit this workspace."), @@ -488,7 +521,9 @@ def get_manageable_workspaces(): manager for a Workspace Manager (who should see every workspace, including other users' private ones). Everyone else sees only their own private workspaces. """ - fields = ["name", "title", "icon", "public", "for_user", "standard"] + # `app` comes along so the dialog can group the workspaces that aren't mounted to any app + # (and so appear on no dock) into their own list. + fields = ["name", "title", "icon", "public", "for_user", "standard", "app"] if is_workspace_manager(): filters = {} else: @@ -516,7 +551,7 @@ def get_workspace_settings(name: str): doc = frappe.get_cached_doc("Workspace", name) - can_edit = is_workspace_manager() or (not doc.public and doc.for_user == frappe.session.user) + can_edit = can_edit_workspace(doc) if not can_edit: frappe.throw( _("You need the Workspace Manager role to manage this workspace."), @@ -551,6 +586,7 @@ def get_workspace_settings(name: str): "standard": is_standard, "access": access, "roles": sorted(roles), + "app": doc.app, } @@ -562,17 +598,18 @@ def update_workspace_settings( indicator_color: str | None = None, access: str | None = None, roles: list | str | None = None, + app: str | None = None, ): - """Save appearance + access/roles for a workspace from the Manage Workspaces dialog. + """Save appearance + access/roles + app mount for a workspace from the Manage Workspaces dialog. - A standard (app-shipped) workspace keeps its app-owned title / route / visibility; only + A standard (app-shipped) workspace keeps its app-owned title / route / visibility / app; only its appearance and role gating are captured as a Workspace Customization delta. A custom (or developer-mode) workspace is edited in place, with `access` mapped onto the underlying `public` / `for_user` / `roles` fields (mirroring `new_page`). """ doc = frappe.get_doc("Workspace", name) - can_edit = is_workspace_manager() or (not doc.public and doc.for_user == frappe.session.user) + can_edit = can_edit_workspace(doc) if not can_edit: frappe.throw( _("You need the Workspace Manager role to edit this workspace."), @@ -593,7 +630,7 @@ def update_workspace_settings( ) upsert_settings_customization(name, icon=icon, indicator_color=indicator_color, roles=role_names) - return {"workspace_pages": get_workspaces(), "sidebar_items": get_sidebar_items(), "name": name} + return workspace_payload(name=name) # custom workspace: edit in place, mapping the access choice onto public / for_user / roles make_public = 0 if access == "private" else 1 @@ -611,6 +648,10 @@ def update_workspace_settings( doc.icon = icon if indicator_color is not None: doc.indicator_color = indicator_color + # the dock the workspace is mounted to. "" is a meaningful value (unmount it), so this is + # deliberately a `not None` check rather than a truthiness one. + if app is not None: + doc.app = app doc.set("roles", [{"role": r} for r in role_names]) if doc.public != make_public: doc.sequence_id = frappe.db.count("Workspace", {"public": make_public}, cache=True) @@ -636,7 +677,7 @@ def update_workspace_settings( if child.name != new_child_name: rename_doc("Workspace", child.name, new_child_name, force=True, ignore_permissions=True) - return {"workspace_pages": get_workspaces(), "sidebar_items": get_sidebar_items(), "name": new_name} + return workspace_payload(name=new_name) @frappe.whitelist() @@ -647,7 +688,7 @@ def delete_page(name: str): if doc.standard and not frappe.conf.developer_mode: frappe.throw(_("Standard workspaces cannot be deleted. Reset to standard instead.")) - can_edit = is_workspace_manager() or (not doc.public and doc.for_user == frappe.session.user) + can_edit = can_edit_workspace(doc) if not can_edit: frappe.throw( _("You need the Workspace Manager role to delete this workspace."), @@ -655,7 +696,71 @@ def delete_page(name: str): ) frappe.delete_doc("Workspace", name, ignore_permissions=True) - return {"workspace_pages": get_workspaces(), "sidebar_items": get_sidebar_items()} + return workspace_payload() + + +@frappe.whitelist() +def get_mountable_apps(): + """Apps a workspace may be mounted to, as `[{app_name, app_title}]`. + + The installed apps that declare `add_to_apps_screen` and that the user is allowed into -- + the same permission gate `load_desktop_data` applies when building `boot.app_data`. Apps + that declare no hook have no desk presence to mount into, so they're skipped. + + Companion apps (those pinned into a host app's dock via `add_to_workspace_dock`) stay in the + list: mounting to one is meaningful, and the desk resolves it to the host's rail at read + time via `rail_host_app`. + """ + apps = [] + for app_name in frappe.get_installed_apps(): + hooks = frappe.get_hooks("add_to_apps_screen", app_name=app_name) + if not hooks: + continue + + app_info = hooks[0] + has_permission = app_info.get("has_permission") + if has_permission and not frappe.get_attr(has_permission)(): + continue + + apps.append( + { + "app_name": app_info.get("name") or app_name, + "app_title": app_info.get("title") + or (frappe.get_hooks("app_title", app_name=app_name) or [None])[0] + or app_name, + } + ) + + return apps + + +@frappe.whitelist() +def mount_workspace(name: str, app: str): + """Mount a workspace onto an app's dock, from the in-page "not on any dock" prompt. + + Narrow counterpart to `update_workspace_settings` -- it only sets `app`, so the prompt + doesn't have to round-trip the workspace's title / icon / access just to place it. + """ + doc = frappe.get_doc("Workspace", name) + + if doc.standard and not frappe.conf.developer_mode: + # a standard workspace's app is owned by its module, and Workspace Customization has no + # field to record a per-site override in + frappe.throw(_("A standard workspace is mounted by the app that ships it.")) + + if not can_edit_workspace(doc): + frappe.throw( + _("You need the Workspace Manager role to mount this workspace."), + frappe.PermissionError, + ) + + if app not in [a["app_name"] for a in get_mountable_apps()]: + frappe.throw(_("{0} is not an app you can mount a workspace to.").format(frappe.bold(app))) + + doc.app = app + doc.save(ignore_permissions=True) + + return workspace_payload(name=doc.name) def last_sequence_id(doc): diff --git a/frappe/public/js/frappe/ui/sidebar/sidebar.js b/frappe/public/js/frappe/ui/sidebar/sidebar.js index bc06848a38d9..a296d3cb8597 100644 --- a/frappe/public/js/frappe/ui/sidebar/sidebar.js +++ b/frappe/public/js/frappe/ui/sidebar/sidebar.js @@ -53,15 +53,13 @@ frappe.ui.Sidebar = class Sidebar { const route = frappe.get_route(); if (route[0] === "Workspaces") { // a workspace route names its workspace -> the app comes from the workspace itself. - // Custom (user-created, non-standard) workspaces belong to no app, so they never carry - // an app context -- even if an older one has a stale `app` value. + // `app` is the mount point for every kind of workspace: standard ones inherit it from + // their module, custom and private ones are mounted explicitly. Only a workspace that + // has never been mounted resolves to no app. const name = route[route.length - 1]; const workspace = frappe.workspaces[frappe.router.slug(name)]; const sidebar = frappe.boot.workspace_sidebar_item[name.toLowerCase()]; - const app_name = - workspace && !workspace.standard - ? null - : (workspace && workspace.app) || (sidebar && sidebar.app); + const app_name = (workspace && workspace.app) || (sidebar && sidebar.app); const app = app_name && frappe.boot.app_data.find((a) => a.app_name === this.rail_host_app(app_name)); @@ -70,8 +68,8 @@ frappe.ui.Sidebar = class Sidebar { this.header_subtitle = app.app_title; this.app_logo_url = app.app_logo_url; } else { - // no owning app (a custom workspace) -> clear the app context so the header/selector - // don't keep showing the app you came from + // unmounted workspace -> clear the app context so the header/selector don't keep + // showing the app you came from frappe.current_app = null; this.header_subtitle = frappe.session.user; } @@ -388,16 +386,13 @@ frappe.ui.Sidebar = class Sidebar { // dock belongs to whichever app's sidebar is shown, so it follows this rather than the // route-derived `frappe.current_app` (the two can diverge -- e.g. a sidebar that curates a // cross-app link keeps its own app while the route entity belongs to another). Resolved from the - // shown workspace's `app` (module sidebars carry it on the boot payload); custom (non-standard) - // workspaces belong to no app. + // shown workspace's `app` (module sidebars carry it on the boot payload). A workspace that + // isn't mounted to any app resolves to null. get_sidebar_app() { if (!this.sidebar_title) return null; const workspace = frappe.workspaces[frappe.router.slug(this.sidebar_title)]; const sidebar = frappe.boot.workspace_sidebar_item[this.sidebar_title.toLowerCase()]; - const app_name = - workspace && !workspace.standard - ? null - : (workspace && workspace.app) || (sidebar && sidebar.app); + const app_name = (workspace && workspace.app) || (sidebar && sidebar.app); return app_name ? frappe.boot.app_data.find((a) => a.app_name === this.rail_host_app(app_name)) : null; @@ -976,9 +971,16 @@ frappe.ui.Sidebar = class Sidebar { let route = this.get_first_sidebar_route(name); if (route) { frappe.set_route(route); - } else { - frappe.set_route("Workspaces", frappe.router.slug(name)); + return; } + + // No sidebar items to land on -> the workspace's own page. Route by path (as the workspace + // view itself does) rather than as a ["Workspaces", slug] standard route: the path form is + // what carries a private workspace's `private/` prefix and lets the router resolve the slug + // back to the workspace's real name. + let workspace = frappe.workspaces[frappe.router.slug(name)]; + let is_public = workspace ? workspace.public : true; + frappe.set_route(frappe.router.slug(is_public ? name : "private/" + name)); } // --------------------------------------------------------------------------------------------- @@ -986,46 +988,33 @@ frappe.ui.Sidebar = class Sidebar { // workspace dock, so the two always offer the same workspaces. // --------------------------------------------------------------------------------------------- - // The public workspaces the selector offers, as Workspace objects in display order. - // `frappe.boot.user_workspaces` is the user's personal selector preference (`User.workspaces`). - // When set, it is authoritative and may include private workspaces too. Otherwise fall back to - // `app`'s workspaces plus any public custom (user-created, non-standard) workspaces -- those - // don't belong to an app's list (a "true custom" workspace has no app), so they'd otherwise - // never appear. `app` defaults to the route's current app (used by the header dropdown); the - // dock passes the shown sidebar's app so it lists that app's workspaces. - get_public_workspaces(app = frappe.current_app) { - let user_workspaces = frappe.boot.user_workspaces || []; - let source; - if (user_workspaces.length) { - source = user_workspaces; - } else { - let app_workspaces = (app && app.workspaces) || []; - let appless_custom = Object.values(frappe.workspaces || {}) - .filter((workspace) => workspace.public && !workspace.standard && !workspace.app) - .map((workspace) => workspace.name); - source = [...new Set([...app_workspaces, ...appless_custom])]; - } - - return source.map((name) => frappe.workspaces[frappe.router.slug(name)]).filter(Boolean); - } - - // The user's private workspaces, as Workspace objects. When the user has curated a selection, - // any private workspaces they want are already part of it (via get_public_workspaces), so - // don't auto-append them again. - get_private_workspaces() { - if ((frappe.boot.user_workspaces || []).length) return []; - - return Object.values(frappe.workspaces || {}).filter( - (workspace) => !workspace.public && workspace.for_user === frappe.session.user - ); - } - - // Full ordered set of workspaces the selector covers (public then private), including the + // Full ordered set of workspaces the selector covers, as Workspace objects, including the // active one. The dock renders this whole set (highlighting the active); the header dropdown // drops the active one (see get_workspace_selector_items) since you can't switch to it. - // `app` scopes the public workspaces (defaults to the route's current app). - collect_selector_workspaces(app) { - return [...this.get_public_workspaces(app), ...this.get_private_workspaces()]; + // + // The set is exactly the app's own workspaces -- `app_data[].workspaces`, which the server + // builds from each workspace's module or its `app` mount, public and private alike. A + // workspace that isn't mounted to any app is deliberately on no dock; it stays reachable + // through global search and Manage Workspaces until someone mounts it. + // + // `app` defaults to the route's current app (used by the header dropdown); the dock passes the + // shown sidebar's app so it lists that app's workspaces. + collect_selector_workspaces(app = frappe.current_app) { + let names = (app && app.workspaces) || []; + + // `frappe.boot.user_workspaces` (`User.workspaces`) is the user's curated selector + // preference. Apply it *within* the app's set rather than as a replacement for it -- + // as a replacement it would put the same list on every app's dock, which is the + // belongs-to-every-app problem the app mount exists to fix. If the user curated a + // selection that names none of this app's workspaces, fall back to the app's full set + // rather than rendering an empty rail. + let selection = frappe.boot.user_workspaces || []; + if (selection.length) { + let scoped = selection.filter((name) => names.includes(name)); + if (scoped.length) names = scoped; + } + + return names.map((name) => frappe.workspaces[frappe.router.slug(name)]).filter(Boolean); } // Menu items for the header dropdown selector: every selector workspace except the active one. diff --git a/frappe/public/js/frappe/ui/sidebar/workspace_picker.js b/frappe/public/js/frappe/ui/sidebar/workspace_picker.js index 5eeb31d9a732..f2d95e15eac8 100644 --- a/frappe/public/js/frappe/ui/sidebar/workspace_picker.js +++ b/frappe/public/js/frappe/ui/sidebar/workspace_picker.js @@ -1,14 +1,14 @@ // "My Workspaces" picker -- lets the user curate which workspaces appear in their workspace // selector, across apps. Two draggable areas: the left is a preview of the selector (their // chosen workspaces, reorderable); the right is the full pool of permitted workspaces, browsable -// app-by-app (plus a "Private" group for the user's own workspaces and a "Custom" group for -// public workspaces that belong to no app). All data comes from `frappe.boot`; only the -// selection is saved. +// app-by-app, plus a group for the workspaces that aren't in any app. All data comes from +// `frappe.boot`; only the selection is saved. -// sentinel app value for the user's private (`for_user`) workspaces in the pool dropdown -const PRIVATE_GROUP = "__private__"; -// sentinel app value for public custom workspaces that belong to no app -const CUSTOM_GROUP = "__custom__"; +// Sentinel app value for workspaces with no app. They're in no app's sidebar, so this group is +// the only place the picker can offer them. Private and public ones sit together: once a +// workspace is placed in an app, that's what decides where it shows up -- not whether it's +// private -- so "in an app or not" is the split that matters here. +const UNMOUNTED_GROUP = "__unmounted__"; frappe.ui.WorkspacePicker = class WorkspacePicker { constructor() { @@ -103,18 +103,11 @@ frappe.ui.WorkspacePicker = class WorkspacePicker { ) .join(""); - // offer public app-less custom workspaces as their own group - if (this.get_custom_workspaces().length) { - options += ``; - } - - // offer the user's private workspaces as their own group - if (this.get_private_workspaces().length) { - options += ``; + // offer the workspaces that aren't in any app as their own group + if (this.get_unmounted_workspaces().length) { + options += ``; } let $picker = this.$body.find(".ws-app-picker"); @@ -126,17 +119,11 @@ frappe.ui.WorkspacePicker = class WorkspacePicker { }); } - // the user's own private (`for_user`) workspaces, by name - get_private_workspaces() { - return Object.values(frappe.workspaces || {}) - .filter((ws) => !ws.public && ws.for_user === frappe.session.user) - .map((ws) => ws.name); - } - - // public custom (user-created, non-standard) workspaces that belong to no app, by name - get_custom_workspaces() { + // workspaces (private or public) that aren't in any app, by name -- the ones no app's + // sidebar lists + get_unmounted_workspaces() { return Object.values(frappe.workspaces || {}) - .filter((ws) => ws.public && !ws.standard && !ws.app) + .filter((ws) => !ws.app && !ws.standard) .map((ws) => ws.name); } @@ -153,10 +140,8 @@ frappe.ui.WorkspacePicker = class WorkspacePicker { render_pool() { let names; - if (this.current_app_name === PRIVATE_GROUP) { - names = this.get_private_workspaces(); - } else if (this.current_app_name === CUSTOM_GROUP) { - names = this.get_custom_workspaces(); + if (this.current_app_name === UNMOUNTED_GROUP) { + names = this.get_unmounted_workspaces(); } else { let app = this.apps.find((a) => a.app_name === this.current_app_name); names = (app && app.workspaces) || []; diff --git a/frappe/public/js/frappe/ui/toolbar/search_utils.js b/frappe/public/js/frappe/ui/toolbar/search_utils.js index 2965f8ce45c8..791e8273d2eb 100644 --- a/frappe/public/js/frappe/ui/toolbar/search_utils.js +++ b/frappe/public/js/frappe/ui/toolbar/search_utils.js @@ -369,12 +369,17 @@ frappe.search.utils = { return out; }, + // Search covers every workspace the user is permitted (`frappe.workspaces`), not just those + // carrying sidebar items. That matters because the dock only lists workspaces mounted to an + // app -- for an unmounted one, search is the way back to it. get_workspaces: function (keywords) { var me = this; var out = []; - const sidebars = frappe.boot.workspace_sidebar_item || {}; - Object.keys(sidebars).forEach(function (key) { - const title = sidebars[key].label || key; + Object.values(frappe.workspaces || {}).forEach(function (workspace) { + const name = workspace.name; + const title = workspace.title || workspace.label || name; + if (!title) return; + const search_result = me.fuzzy_search(keywords, title, true); const level = search_result.score; if (level > 0) { @@ -383,9 +388,10 @@ frappe.search.utils = { label: __("Open {0} Workspace", [search_result.marked_string || __(title)]), value: __("Open {0} Workspace", [__(title)]), index: level, - // open the workspace's sidebar and land on its first item + // open the workspace's sidebar and land on its first item; falls back to the + // workspace's own route when it has no sidebar items onclick: function () { - frappe.app.sidebar.open_workspace(title); + frappe.app.sidebar.open_workspace(name); }, }); } diff --git a/frappe/public/js/frappe/views/workspace/workspace.js b/frappe/public/js/frappe/views/workspace/workspace.js index a5e52fc1f685..78825b6e8196 100644 --- a/frappe/public/js/frappe/views/workspace/workspace.js +++ b/frappe/public/js/frappe/views/workspace/workspace.js @@ -69,6 +69,25 @@ frappe.views.Workspace = class Workspace { this.sidebar.setup(this._page.name); } } + + // The apps a workspace can be mounted to (see `get_mountable_apps`), memoised for the life of + // the view -- the New Workspace dialog, the Manage Workspaces panel and the "not on any dock" + // prompt all need the same list, and the installed apps don't change under us. + get_mountable_apps() { + if (!this._mountable_apps) { + this._mountable_apps = frappe + .xcall("frappe.desk.doctype.workspace.workspace.get_mountable_apps") + .then((apps) => apps || []); + } + return this._mountable_apps; + } + + // `app` is stored as the app's name ("erpnext") but should read as its title ("ERPNext"), so + // the Select gets {value, label} options rather than bare strings. + app_select_options(apps) { + return apps.map((app) => ({ value: app.app_name, label: app.app_title })); + } + prepare_container() { this.body = this.wrapper.find(".layout-main-section"); this.$page = $(`
`).appendTo(this.body); @@ -258,6 +277,7 @@ frappe.views.Workspace = class Workspace { this.content = current_page.content; this.content && this.add_custom_cards_in_content(); this.content && this.add_hidden_notice_in_content(current_page); + this.add_mount_notice(current_page); $(".item-anchor").addClass("disable-click"); @@ -362,6 +382,81 @@ frappe.views.Workspace = class Workspace { }); } + // A workspace with no `app` is mounted to no dock, so it appears in no app's sidebar and is + // only reachable through global search or Manage Workspaces. Prompt whoever lands on it -- + // with the action if they can mount it themselves, otherwise with who to ask. + // + // Rendered as a banner above the editor rather than as a content block (the route + // add_hidden_notice_in_content takes) because it carries an action button, and because it + // must never be mistaken for part of the workspace's saved content. + add_mount_notice(page) { + this.body.find(".workspace-mount-notice").remove(); + + // standard workspaces are mounted by the app that ships them, via their module + if (!page || page.app || page.standard || page.type !== "Workspace") return; + + // mirrors `can_edit_workspace` on the server: a Workspace Manager may mount anything, + // anyone may mount their own private workspace + const can_mount = + this.has_access || (!page.public && page.for_user === frappe.session.user); + + const $notice = frappe.ui.alert({ + theme: "amber", + css_class: "workspace-mount-notice", + title: __("This workspace isn't in any app's sidebar"), + description: can_mount + ? __("Add it to an app so it shows up in the sidebar and is easy to get back to.") + : __( + "It can only be found through search. Ask a Workspace Manager to add it to an app." + ), + footer: can_mount + ? () => + $( + `` + ).on("click", () => this.prompt_mount_workspace(page)) + : undefined, + }); + + $notice.insertBefore(this.body.find(".editor-js-container")); + } + + // Ask which app to mount `page` to, then mount it and refresh the desk in place. + async prompt_mount_workspace(page) { + const apps = await this.get_mountable_apps(); + const d = new frappe.ui.Dialog({ + title: __("Add {0} to an app", [__(page.title)]), + fields: [ + { + label: __("App"), + fieldtype: "Select", + fieldname: "app", + reqd: 1, + options: this.app_select_options(apps), + default: frappe.current_app && frappe.current_app.app_name, + description: __("Which app's sidebar this workspace appears in"), + }, + ], + primary_action_label: __("Add"), + primary_action: (values) => { + d.hide(); + frappe.call({ + method: "frappe.desk.doctype.workspace.workspace.mount_workspace", + args: { name: page.name, app: values.app }, + freeze: true, + callback: (r) => { + if (!r.message) return; + this.apply_manager_changes(r.message); + frappe.show_alert({ + message: __("Added {0} to {1}", [__(page.title), __(values.app)]), + indicator: "green", + }); + }, + }); + }, + }); + d.show(); + } + open_workspace_manager(current_page) { // Two-pane manager: the list of workspaces the user can manage on the left, the // selected workspace's access / appearance settings on the right. Replaces the @@ -378,12 +473,18 @@ frappe.views.Workspace = class Workspace { const manageable = r.message || []; if (!manageable.length) return; - // Standard = public app-shipped, Custom = public but user-created, - // Private = per-user workspaces. + // "Not in any app" first -- those workspaces are on no dock, so this is the + // list to triage. Then Standard = public app-shipped, Custom = public but + // user-created, Private = per-user workspaces. + const unmounted = (p) => !p.app && !p.standard; const groups = [ + { label: __("Not in any app"), filter: unmounted }, { label: __("Standard"), filter: (p) => p.public && p.standard }, - { label: __("Custom"), filter: (p) => p.public && !p.standard }, - { label: __("Private"), filter: (p) => !p.public }, + { + label: __("Custom"), + filter: (p) => p.public && !p.standard && !unmounted(p), + }, + { label: __("Private"), filter: (p) => !p.public && !unmounted(p) }, ]; const tabs = []; groups.forEach(({ label, filter }) => { @@ -420,47 +521,45 @@ frappe.views.Workspace = class Workspace { }; } - render_workspace_manager_panel(panel, page) { + async render_workspace_manager_panel(panel, page) { panel.set_header({ title: __(page.title) }); panel.body.html(`
${__("Loading...")}
`); - frappe - .call({ - method: "frappe.desk.doctype.workspace.workspace.get_workspace_settings", - args: { name: page.name }, - }) - .then((r) => { - const settings = r.message; - if (!settings) return; - - const actions = []; - if (!settings.standard) { - actions.push({ - label: __("Delete"), - class: "btn-danger", - click: () => this.delete_workspace_from_manager(page), - }); - } - actions.push({ - label: __("Save"), - primary: true, - click: (p) => this.save_workspace_from_manager(p, settings), - }); - - panel.set_view({ - title: __(settings.title), - description: settings.standard - ? __( - "A standard workspace is shipped by the app; changes are saved as customizations." - ) - : __("Control who can see this workspace and how it appears."), - actions, - fields: this.workspace_manager_fields(settings), - }); + const [settings, apps] = await Promise.all([ + frappe.xcall("frappe.desk.doctype.workspace.workspace.get_workspace_settings", { + name: page.name, + }), + this.get_mountable_apps(), + ]); + if (!settings) return; + + const actions = []; + if (!settings.standard) { + actions.push({ + label: __("Delete"), + class: "btn-danger", + click: () => this.delete_workspace_from_manager(page), }); + } + actions.push({ + label: __("Save"), + primary: true, + click: (p) => this.save_workspace_from_manager(p, settings), + }); + + panel.set_view({ + title: __(settings.title), + description: settings.standard + ? __( + "A standard workspace is shipped by the app; changes are saved as customizations." + ) + : __("Control who can see this workspace and how it appears."), + actions, + fields: this.workspace_manager_fields(settings, apps), + }); } - workspace_manager_fields(settings) { + workspace_manager_fields(settings, apps) { const access_to_label = { private: ACCESS_PRIVATE, group: ACCESS_GROUP, @@ -492,6 +591,19 @@ frappe.views.Workspace = class Workspace { ? __("The title of a standard workspace is managed by the app.") : "", }, + { + label: __("App"), + fieldname: "app", + fieldtype: "Select", + options: this.app_select_options(apps || []), + default: settings.app, + // a standard workspace's app follows its module, and there's no per-site + // override to record a different one in + read_only: settings.standard ? 1 : 0, + description: settings.standard + ? __("A standard workspace stays in the app that ships it.") + : __("Which app's sidebar this workspace appears in"), + }, { label: __("Icon"), fieldname: "icon", @@ -550,6 +662,8 @@ frappe.views.Workspace = class Workspace { icon: values.icon, access, roles, + // read-only for standard workspaces, so this only ever moves a custom one + app: values.app, }, freeze: true, callback: (r) => { @@ -589,11 +703,16 @@ frappe.views.Workspace = class Workspace { this.workspaces = frappe.boot.workspaces.pages; this.setup_pages(frappe.boot.workspaces.pages); frappe.boot.workspace_sidebar_item = message.sidebar_items; + // The dock is app-scoped: it lists `app_data[app].workspaces`. A workspace that just + // changed app (or gained one) only moves docks once this mapping is swapped in. + if (message.app_data) frappe.boot.app_data = message.app_data; this.reload(); // reload() re-derives the current page synchronously; re-render its sidebar so a rename // or visibility change is reflected in the shell. if (frappe.app.sidebar && this._page) { frappe.app.sidebar.setup(this._page.name); + // ...and re-resolve the app context + dock, which `setup` leaves alone + frappe.app.sidebar.refresh(); } } @@ -703,9 +822,12 @@ frappe.views.Workspace = class Workspace { ); } - initialize_new_page() { + async initialize_new_page() { var me = this; this.get_parent_pages(); + // A workspace with no app lands on no dock, so ask for it up front rather than let the + // workspace be created stranded and rely on the "not on any dock" prompt to rescue it. + const apps = await this.get_mountable_apps(); const d = new frappe.ui.Dialog({ title: __("New Workspace"), fields: [ @@ -773,6 +895,15 @@ frappe.views.Workspace = class Workspace { ); }, }, + { + label: __("App"), + fieldtype: "Select", + fieldname: "app", + reqd: 1, + options: this.app_select_options(apps), + default: frappe.current_app && frappe.current_app.app_name, + description: __("Which app's sidebar this workspace appears in"), + }, { label: __("Icon"), fieldtype: "Icon", @@ -837,6 +968,8 @@ frappe.views.Workspace = class Workspace { icon: values.icon, roles: values.access === ACCESS_GROUP ? values.roles || [] : [], parent_page: values.parent || "", + // the app whose dock lists this workspace + app: values.app, is_editable: true, selected: true, type: values.type, From db8f0edea9c9842d97648aa1db5e11f52d85de98 Mon Sep 17 00:00:00 2001 From: sokumon Date: Mon, 27 Jul 2026 02:44:32 +0530 Subject: [PATCH 2/5] fix: don't show welcome workspace --- frappe/public/js/frappe/views/workspace/workspace.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/views/workspace/workspace.js b/frappe/public/js/frappe/views/workspace/workspace.js index 78825b6e8196..3662998b07db 100644 --- a/frappe/public/js/frappe/views/workspace/workspace.js +++ b/frappe/public/js/frappe/views/workspace/workspace.js @@ -5,6 +5,11 @@ import Undo from "editorjs-undo"; // rendered for Workspace Managers but stripped before the content is saved. const HIDDEN_NOTICE_MARKER = "workspace-hidden-notice"; +// The framework's fallback workspace, shown only when a user has no other workspace at all +// (see get_workspaces). It's special-cased out of workspace listings across the codebase; here +// it's kept out of the "not in any app" triage list, since it isn't a workspace anyone mounts. +const WELCOME_WORKSPACE = "Welcome Workspace"; + // "Access" options in the New Workspace dialog -- a virtual field that maps to the // underlying `public` / `for_user` / `roles` fields: // private -> personal (public=0, for_user=current user) @@ -476,7 +481,7 @@ frappe.views.Workspace = class Workspace { // "Not in any app" first -- those workspaces are on no dock, so this is the // list to triage. Then Standard = public app-shipped, Custom = public but // user-created, Private = per-user workspaces. - const unmounted = (p) => !p.app && !p.standard; + const unmounted = (p) => !p.app && !p.standard && p.title !== WELCOME_WORKSPACE; const groups = [ { label: __("Not in any app"), filter: unmounted }, { label: __("Standard"), filter: (p) => p.public && p.standard }, From 099c694e04df3da8af28810ce20f955a8429c775 Mon Sep 17 00:00:00 2001 From: sokumon Date: Mon, 27 Jul 2026 04:08:18 +0530 Subject: [PATCH 3/5] fix: show app mount notice as dialog --- .../js/frappe/views/workspace/workspace.js | 76 ++++++++++++------- 1 file changed, 48 insertions(+), 28 deletions(-) diff --git a/frappe/public/js/frappe/views/workspace/workspace.js b/frappe/public/js/frappe/views/workspace/workspace.js index 3662998b07db..d147d296c6fa 100644 --- a/frappe/public/js/frappe/views/workspace/workspace.js +++ b/frappe/public/js/frappe/views/workspace/workspace.js @@ -387,50 +387,56 @@ frappe.views.Workspace = class Workspace { }); } - // A workspace with no `app` is mounted to no dock, so it appears in no app's sidebar and is - // only reachable through global search or Manage Workspaces. Prompt whoever lands on it -- - // with the action if they can mount it themselves, otherwise with who to ask. - // - // Rendered as a banner above the editor rather than as a content block (the route - // add_hidden_notice_in_content takes) because it carries an action button, and because it - // must never be mistaken for part of the workspace's saved content. - add_mount_notice(page) { - this.body.find(".workspace-mount-notice").remove(); - + // A workspace with no `app` is in no app's sidebar, so it's only reachable through global + // search or Manage Workspaces. Prompt whoever lands on it: a dialog offering to place it for + // anyone who can, and an explanation for anyone who can't. Both are dismissible -- being + // unmounted is worth raising, but not worth trapping someone over. + async add_mount_notice(page) { // standard workspaces are mounted by the app that ships them, via their module if (!page || page.app || page.standard || page.type !== "Workspace") return; + // show_page runs on every navigation -- don't stack dialogs on the same workspace + if (this.mount_dialog && this.mount_dialog.page_name === page.name) return; + // ...and once it's been waved off, leave it alone for the rest of the session rather + // than re-asking every time the workspace is opened + if (this.dismissed_mount_prompts?.has(page.name)) return; // mirrors `can_edit_workspace` on the server: a Workspace Manager may mount anything, // anyone may mount their own private workspace const can_mount = this.has_access || (!page.public && page.for_user === frappe.session.user); - const $notice = frappe.ui.alert({ - theme: "amber", - css_class: "workspace-mount-notice", - title: __("This workspace isn't in any app's sidebar"), - description: can_mount - ? __("Add it to an app so it shows up in the sidebar and is easy to get back to.") - : __( - "It can only be found through search. Ask a Workspace Manager to add it to an app." - ), - footer: can_mount - ? () => - $( - `` - ).on("click", () => this.prompt_mount_workspace(page)) - : undefined, - }); + if (!can_mount) { + // there's nothing for them to act on, so say it once and don't raise it again + this.dismissed_mount_prompts = this.dismissed_mount_prompts || new Set(); + this.dismissed_mount_prompts.add(page.name); + frappe.msgprint({ + title: __("Not in any app"), + indicator: "orange", + message: __( + "This workspace isn't in any app's sidebar, so it can only be found through search. Ask a Workspace Manager to add it to an app." + ), + }); + return; + } - $notice.insertBefore(this.body.find(".editor-js-container")); + await this.prompt_mount_workspace(page); } - // Ask which app to mount `page` to, then mount it and refresh the desk in place. + // Ask which app to mount `page` to, then mount it and refresh the desk in place. Closing the + // dialog without choosing is fine -- it just won't ask again this session. async prompt_mount_workspace(page) { const apps = await this.get_mountable_apps(); + let mounted = false; const d = new frappe.ui.Dialog({ title: __("Add {0} to an app", [__(page.title)]), fields: [ + { + fieldtype: "HTML", + fieldname: "why", + options: `

${__( + "This workspace isn't in any app's sidebar yet, so there's no way to navigate to it. Pick the app it belongs to." + )}

`, + }, { label: __("App"), fieldtype: "Select", @@ -443,6 +449,7 @@ frappe.views.Workspace = class Workspace { ], primary_action_label: __("Add"), primary_action: (values) => { + mounted = true; d.hide(); frappe.call({ method: "frappe.desk.doctype.workspace.workspace.mount_workspace", @@ -459,6 +466,19 @@ frappe.views.Workspace = class Workspace { }); }, }); + + // tracked so a re-render of the same workspace doesn't stack a second copy on top + d.page_name = page.name; + this.mount_dialog = d; + d.$wrapper.on("hidden.bs.modal", () => { + if (this.mount_dialog === d) this.mount_dialog = null; + // closed without picking an app -> take the hint and stop asking for this session + if (!mounted) { + this.dismissed_mount_prompts = this.dismissed_mount_prompts || new Set(); + this.dismissed_mount_prompts.add(page.name); + } + }); + d.show(); } From 7ea17dde84c756196a7b99034433f074a5273df6 Mon Sep 17 00:00:00 2001 From: sokumon Date: Mon, 27 Jul 2026 04:20:28 +0530 Subject: [PATCH 4/5] fix: redo the workspace picker as dock picker --- frappe/public/js/desk.bundle.js | 2 +- .../{workspace_picker.js => dock_manager.js} | 143 +++++++----------- frappe/public/js/frappe/ui/sidebar/sidebar.js | 4 +- .../js/frappe/ui/sidebar/sidebar_header.js | 4 +- ...orkspace_picker.scss => dock_manager.scss} | 11 +- frappe/public/scss/desk/index.scss | 2 +- 6 files changed, 65 insertions(+), 101 deletions(-) rename frappe/public/js/frappe/ui/sidebar/{workspace_picker.js => dock_manager.js} (50%) rename frappe/public/scss/desk/{workspace_picker.scss => dock_manager.scss} (94%) diff --git a/frappe/public/js/desk.bundle.js b/frappe/public/js/desk.bundle.js index 5d03276e7f9f..831d41981603 100644 --- a/frappe/public/js/desk.bundle.js +++ b/frappe/public/js/desk.bundle.js @@ -31,7 +31,7 @@ import "./frappe/ui/keyboard.js"; import "./frappe/ui/colors.js"; import "./frappe/ui/sidebar/sidebar_header.js"; import "./frappe/ui/sidebar/sidebar_header.html"; -import "./frappe/ui/sidebar/workspace_picker.js"; +import "./frappe/ui/sidebar/dock_manager.js"; import "./frappe/ui/sidebar/sidebar.html"; import "./frappe/ui/sidebar/sidebar_item.html"; import "./frappe/ui/sidebar/sidebar.js"; diff --git a/frappe/public/js/frappe/ui/sidebar/workspace_picker.js b/frappe/public/js/frappe/ui/sidebar/dock_manager.js similarity index 50% rename from frappe/public/js/frappe/ui/sidebar/workspace_picker.js rename to frappe/public/js/frappe/ui/sidebar/dock_manager.js index f2d95e15eac8..bd3b2eed9990 100644 --- a/frappe/public/js/frappe/ui/sidebar/workspace_picker.js +++ b/frappe/public/js/frappe/ui/sidebar/dock_manager.js @@ -1,25 +1,24 @@ -// "My Workspaces" picker -- lets the user curate which workspaces appear in their workspace -// selector, across apps. Two draggable areas: the left is a preview of the selector (their -// chosen workspaces, reorderable); the right is the full pool of permitted workspaces, browsable -// app-by-app, plus a group for the workspaces that aren't in any app. All data comes from -// `frappe.boot`; only the selection is saved. - -// Sentinel app value for workspaces with no app. They're in no app's sidebar, so this group is -// the only place the picker can offer them. Private and public ones sit together: once a -// workspace is placed in an app, that's what decides where it shows up -- not whether it's -// private -- so "in an app or not" is the split that matters here. -const UNMOUNTED_GROUP = "__unmounted__"; - -frappe.ui.WorkspacePicker = class WorkspacePicker { +// Dock manager -- lets the user curate the workspace dock (rail) for the app they're currently +// in: which of that app's workspaces appear on it, and in what order. Two draggable areas: the +// left is a preview of the dock (their chosen workspaces, reorderable); the right is the rest of +// the app's workspaces, ready to drag in. +// +// Scoped to one app on purpose -- a dock belongs to an app, so there's nothing to choose between +// here and no app switcher. Workspaces in other apps are managed from those apps' docks; +// workspaces in no app at all appear on no dock, and are placed from Manage Workspaces. +// +// All data comes from `frappe.boot`; only the selection is saved. +frappe.ui.DockManager = class DockManager { constructor() { this.make(); } make() { + this.app = frappe.current_app; this.selection = this.initial_selection(); this.dialog = new frappe.ui.Dialog({ - title: __("Workspaces"), + title: this.app ? __("Manage {0} Dock", [__(this.app.app_title)]) : __("Manage Dock"), size: "extra-large", fields: [{ fieldtype: "HTML", fieldname: "picker" }], primary_action_label: __("Save"), @@ -31,14 +30,21 @@ frappe.ui.WorkspacePicker = class WorkspacePicker { this.dialog.show(); } - // Start from the user's saved selection; if they have none, seed it with the current app's - // workspaces (what the selector shows by default) so they can trim from there. + // Every workspace this app's dock can show, in the server's order. + app_workspaces() { + return ((this.app && this.app.workspaces) || []).filter((name) => this.has_meta(name)); + } + + // The user's curated picks for this app, in their order. `User.workspaces` is a single flat + // list across every app, so this app's picks are the ones naming its workspaces. Nothing + // curated for this app yet -> start from everything it offers (what the dock shows by + // default), so the user trims rather than builds from scratch. initial_selection() { - let sel = (frappe.boot.user_workspaces || []).slice(); - if (!sel.length) { - sel = ((frappe.current_app && frappe.current_app.workspaces) || []).slice(); - } - return sel.filter((name) => this.has_meta(name)); + const app_workspaces = this.app_workspaces(); + const curated = (frappe.boot.user_workspaces || []).filter((name) => + app_workspaces.includes(name) + ); + return curated.length ? curated : app_workspaces; } has_meta(name) { @@ -51,18 +57,18 @@ frappe.ui.WorkspacePicker = class WorkspacePicker { render() { this.$body.html(` -
+
- ${__("Your workspaces")} - + ${__("On the dock")} +
-
${__("Workspaces shown in your sidebar switcher. Drag to reorder.")}
+
${__("Drag to reorder. Reset brings all of them back.")}
-
${__("All workspaces")}
-
+
${__("Not on the dock")}
+
${__("Drag one over to add it.")}
@@ -73,13 +79,14 @@ frappe.ui.WorkspacePicker = class WorkspacePicker { this.$body.find(".ws-clear-all").on("click", () => this.clear_all()); - this.render_app_picker(); this.render_selection(); this.render_pool(); this.setup_selection_sortable(); this.setup_pool_sortable(); } + // An empty selection isn't stored as "an empty dock": the dock falls back to the app's full + // list when the user has curated nothing for it, so clearing is a reset to default. clear_all() { if (!this.selection.length) return; this.selection = []; @@ -87,46 +94,6 @@ frappe.ui.WorkspacePicker = class WorkspacePicker { this.render_pool(); } - render_app_picker() { - // only apps that expose workspaces are worth listing - this.apps = (frappe.boot.app_data || []).filter((a) => (a.workspaces || []).length); - this.current_app_name = - (frappe.current_app && frappe.current_app.app_name) || - (this.apps[0] && this.apps[0].app_name); - - let options = this.apps - .map( - (a) => - `` - ) - .join(""); - - // offer the workspaces that aren't in any app as their own group - if (this.get_unmounted_workspaces().length) { - options += ``; - } - - let $picker = this.$body.find(".ws-app-picker"); - $picker.html(``); - $picker.find("select").on("change", (e) => { - this.current_app_name = e.target.value; - this.render_pool(); - this.setup_pool_sortable(); - }); - } - - // workspaces (private or public) that aren't in any app, by name -- the ones no app's - // sidebar lists - get_unmounted_workspaces() { - return Object.values(frappe.workspaces || {}) - .filter((ws) => !ws.app && !ws.standard) - .map((ws) => ws.name); - } - render_selection() { this.$selection.empty(); if (!this.selection.length) { @@ -138,18 +105,19 @@ frappe.ui.WorkspacePicker = class WorkspacePicker { this.selection.forEach((name) => this.$selection.append(this.selection_item(name))); } + // The pool is the app's workspaces that aren't on the dock yet -- everything droppable in one + // place, with nothing to filter between. render_pool() { - let names; - if (this.current_app_name === UNMOUNTED_GROUP) { - names = this.get_unmounted_workspaces(); - } else { - let app = this.apps.find((a) => a.app_name === this.current_app_name); - names = (app && app.workspaces) || []; - } + const names = this.app_workspaces().filter((name) => !this.selection.includes(name)); + this.$pool.empty(); - names - .filter((name) => this.has_meta(name)) - .forEach((name) => this.$pool.append(this.pool_item(name))); + if (!names.length) { + this.$pool.append( + `
${__("Everything is on the dock")}
` + ); + return; + } + names.forEach((name) => this.$pool.append(this.pool_item(name))); } item(name, cls) { @@ -182,9 +150,7 @@ frappe.ui.WorkspacePicker = class WorkspacePicker { } pool_item(name) { - // already-chosen workspaces are shown disabled (can't be added twice) - let selected = this.selection.includes(name); - return this.item(name, "ws-pool-item" + (selected ? " is-selected" : "")); + return this.item(name, "ws-pool-item"); } setup_selection_sortable() { @@ -212,7 +178,6 @@ frappe.ui.WorkspacePicker = class WorkspacePicker { group: { name: "ws", pull: "clone", put: false }, sort: false, animation: 150, - filter: ".is-selected", }); } @@ -228,12 +193,20 @@ frappe.ui.WorkspacePicker = class WorkspacePicker { async save() { this.sync_order(); + + // `User.workspaces` is one flat list across every app, but a dock belongs to an app -- + // so replace only this app's entries and leave every other app's curation untouched. + const app_workspaces = new Set(this.app_workspaces()); + const other_apps = (frappe.boot.user_workspaces || []).filter( + (name) => !app_workspaces.has(name) + ); + await frappe.xcall("frappe.desk.desktop.save_workspace_preferences", { - workspaces: JSON.stringify(this.selection), + workspaces: JSON.stringify([...other_apps, ...this.selection]), }); this.dialog.hide(); - frappe.show_alert({ message: __("Workspaces updated"), indicator: "green" }); - // reload so the sidebar selector reflects the new preference + frappe.show_alert({ message: __("Dock updated"), indicator: "green" }); + // reload so the dock reflects the new preference setTimeout(() => location.reload(), 600); } }; diff --git a/frappe/public/js/frappe/ui/sidebar/sidebar.js b/frappe/public/js/frappe/ui/sidebar/sidebar.js index a296d3cb8597..c0bf31f69738 100644 --- a/frappe/public/js/frappe/ui/sidebar/sidebar.js +++ b/frappe/public/js/frappe/ui/sidebar/sidebar.js @@ -548,10 +548,10 @@ frappe.ui.Sidebar = class Sidebar { }, { name: "workspace-selector", - label: __("Manage Workspaces"), + label: __("Manage Dock"), icon: "monitor", onClick: function () { - new frappe.ui.WorkspacePicker(); + new frappe.ui.DockManager(); }, }, ...frappe.boot.navbar_settings.settings_dropdown.map((item) => ({ diff --git a/frappe/public/js/frappe/ui/sidebar/sidebar_header.js b/frappe/public/js/frappe/ui/sidebar/sidebar_header.js index 057be9cf7dff..6e54ee38930d 100644 --- a/frappe/public/js/frappe/ui/sidebar/sidebar_header.js +++ b/frappe/public/js/frappe/ui/sidebar/sidebar_header.js @@ -19,9 +19,9 @@ frappe.ui.SidebarHeader = class SidebarHeader { if (apps_section) items.push(apps_section); items.push({ name: "workspace-selector", - label: __("Workspaces"), + label: __("Manage Dock"), icon: "monitor", - onClick: () => new frappe.ui.WorkspacePicker(), + onClick: () => new frappe.ui.DockManager(), }); return items; diff --git a/frappe/public/scss/desk/workspace_picker.scss b/frappe/public/scss/desk/dock_manager.scss similarity index 94% rename from frappe/public/scss/desk/workspace_picker.scss rename to frappe/public/scss/desk/dock_manager.scss index a470704e4da6..21c422ea757e 100644 --- a/frappe/public/scss/desk/workspace_picker.scss +++ b/frappe/public/scss/desk/dock_manager.scss @@ -1,4 +1,4 @@ -.workspace-picker { +.dock-manager { display: flex; gap: var(--margin-lg); @@ -36,10 +36,6 @@ margin-top: calc(-1 * var(--margin-xs)); } - .ws-app-picker .form-control { - width: 100%; - } - .ws-list { flex: 1; display: flex; @@ -125,11 +121,6 @@ &:active { cursor: grabbing; } - - &.is-selected { - opacity: 0.45; - cursor: not-allowed; - } } .ws-item-ghost { diff --git a/frappe/public/scss/desk/index.scss b/frappe/public/scss/desk/index.scss index 8df4d8d34179..4f434412271f 100644 --- a/frappe/public/scss/desk/index.scss +++ b/frappe/public/scss/desk/index.scss @@ -62,5 +62,5 @@ @import "menu"; @import "sidebar_header"; @import "card"; -@import "workspace_picker"; +@import "dock_manager"; @import "document_template"; From 86eb4c617b38192e1d7601f36373bac9461dbf6f Mon Sep 17 00:00:00 2001 From: Nikhil Kothari Date: Mon, 27 Jul 2026 11:18:53 +0530 Subject: [PATCH 5/5] feat(ui): add support for async options in dropdown menu (#41258) --- cypress/integration/es_components.js | 58 +++++ .../component_explorer/component_explorer.js | 70 ++++++ .../public/css/espresso/components/menu.css | 15 ++ .../js/frappe/ui/components/context_menu.js | 2 +- .../js/frappe/ui/components/dropdown.js | 2 +- frappe/public/js/frappe/ui/components/menu.js | 213 ++++++++++++++---- 6 files changed, 318 insertions(+), 42 deletions(-) diff --git a/cypress/integration/es_components.js b/cypress/integration/es_components.js index 22843bd69f3a..8860c0295401 100644 --- a/cypress/integration/es_components.js +++ b/cypress/integration/es_components.js @@ -96,6 +96,64 @@ context("Espresso components", () => { // .es-menu__item itself, not the inner label span. cy.contains(".es-menu__item", "Archive").should("match", "button").and("be.disabled"); }); + + it("async options open in a loading state, then fill in when the promise settles", () => { + cy.contains(".explorer-group", "Async items") + .find('[aria-haspopup="menu"]') + .first() + .click(); + + // the panel opens immediately with the placeholder (aria-busy), + // no rows yet + cy.get(".es-menu[data-state='open']") + .should("have.attr", "aria-busy", "true") + .find(".es-menu__loading") + .should("exist"); + cy.get(".es-menu__item").should("not.exist"); + + // the demo resolves after 600ms — the rows replace the placeholder + // and the busy state clears + cy.contains(".es-menu__item", "Fetched row 1").should("exist"); + cy.get(".es-menu[data-state='open']").should("not.have.attr", "aria-busy"); + cy.get(".es-menu__loading").should("not.exist"); + + // the swapped-in rows are live + cy.contains(".es-menu__item", "Fetched row 2").click(); + cy.get(".es-menu[data-state='open']").should("not.exist"); + cy.contains(".es-toast", "Row 2").should("exist"); + }); + + it("a function submenu loads on hover and swaps in its rows", () => { + cy.contains(".explorer-group", "Async items") + .find('[aria-haspopup="menu"]') + .eq(1) + .click(); + + // hover intent starts the fetch and opens the panel after the delay + cy.contains(".es-menu__item", "Recent documents").trigger("pointerenter"); + cy.get(".es-menu[data-state='open']").should("have.length", 2); + + // resolved rows land in the submenu panel and activate normally + cy.contains(".es-menu__item", "Doc A").should("exist"); + cy.contains(".es-menu__item", "Doc A").click(); + cy.get(".es-menu[data-state='open']").should("not.exist"); + cy.contains(".es-toast", "Doc A").should("exist"); + }); + + it("a rejected submenu shows the failure notice instead of a dead panel", () => { + cy.contains(".explorer-group", "Async items") + .find('[aria-haspopup="menu"]') + .eq(1) + .click(); + + cy.contains(".es-menu__item", "Fails to load").trigger("pointerenter"); + // the panel opens loading, then swaps to the inert notice + cy.get(".es-menu[data-state='open']").should("have.length", 2); + cy.contains(".es-menu__empty", "Couldn't load options").should("exist"); + // the root menu is still alive — Escape closes everything + cy.focused().trigger("keydown", { key: "Escape" }); + cy.get(".es-menu[data-state='open']").should("not.exist"); + }); }); describe("Toast — legacy HTML message sanitisation", () => { diff --git a/frappe/core/page/component_explorer/component_explorer.js b/frappe/core/page/component_explorer/component_explorer.js index 63dcd9102d45..e16707bbccdb 100644 --- a/frappe/core/page/component_explorer/component_explorer.js +++ b/frappe/core/page/component_explorer/component_explorer.js @@ -131,6 +131,76 @@ frappe.pages["component-explorer"].on_page_load = function (wrapper) { }, ], }, + { + title: __("Async items (loading state)"), + items: [ + { + button: { label: "Async options" }, + options: () => + new Promise((resolve) => + setTimeout( + () => + resolve([ + { + label: "Fetched row 1", + onclick: () => + frappe.ui.toast({ message: "Row 1" }), + }, + { + label: "Fetched row 2", + onclick: () => + frappe.ui.toast({ message: "Row 2" }), + }, + ]), + 600 + ) + ), + }, + { + button: { label: "Async submenu" }, + options: [ + { + label: "Recent documents", + icon: "clock", + submenu: () => + new Promise((resolve) => + setTimeout( + () => + resolve([ + { + label: "Doc A", + onclick: () => + frappe.ui.toast({ + message: "Doc A", + }), + }, + { + label: "Doc B", + onclick: () => + frappe.ui.toast({ + message: "Doc B", + }), + }, + ]), + 600 + ) + ), + }, + { + label: "Fails to load", + icon: "cloud-off", + submenu: () => + new Promise((resolve, reject) => + setTimeout( + () => reject(new Error("network down")), + 600 + ) + ), + }, + ], + }, + ], + }, { title: __("Selected row, descriptions and links"), items: [ diff --git a/frappe/public/css/espresso/components/menu.css b/frappe/public/css/espresso/components/menu.css index 517441f87a92..0bce8713995b 100644 --- a/frappe/public/css/espresso/components/menu.css +++ b/frappe/public/css/espresso/components/menu.css @@ -238,6 +238,21 @@ color: var(--ink-gray-5); } +/* ---- loading state (async options/submenu, swapped out by fill()) ---- */ +.es-menu__loading { + display: flex; + align-items: center; + gap: calc(var(--spacing) * 2); + padding: calc(var(--spacing) * 1.5) calc(var(--spacing) * 2); + font-size: var(--text-base); + color: var(--ink-gray-5); +} + +.es-menu__loading > .es-spinner { + width: calc(var(--spacing) * 3.5); + height: calc(var(--spacing) * 3.5); +} + /* ---- motion: scale in from the trigger corner (per the frappe-ui spec: enter 180ms / exit 140ms, keyboard opens skip the scale and only get a short fade that masks the position-settle frame) ---- */ diff --git a/frappe/public/js/frappe/ui/components/context_menu.js b/frappe/public/js/frappe/ui/components/context_menu.js index 8d1fb24b146f..c5a60afa56f0 100644 --- a/frappe/public/js/frappe/ui/components/context_menu.js +++ b/frappe/public/js/frappe/ui/components/context_menu.js @@ -5,7 +5,7 @@ frappe.provide("frappe.ui"); /** * @typedef {Object} ContextMenuOpts * @property {Element|JQuery} target Right-clicking anywhere in this element opens the menu at the cursor. - * @property {Array|function} options Same menu items/groups as frappe.ui.Dropdown. A function is called fresh on every open — handy for menus that depend on which row was clicked. + * @property {Array|function} options Same menu items/groups as frappe.ui.Dropdown. A function is called fresh on every open — handy for menus that depend on which row was clicked — and may return a Promise of the items (the menu opens with a loading row and fills in when it settles). * @property {string} [empty_text] Shown when no items are visible. * @property {function} [on_open] Called with the contextmenu event, before the menu renders — set per-row options here. * @property {function} [on_close] Called with the reason: "activate" | "escape" | "outside" | "tab" | "owner". diff --git a/frappe/public/js/frappe/ui/components/dropdown.js b/frappe/public/js/frappe/ui/components/dropdown.js index 3b8185323a55..3bbd00be049c 100644 --- a/frappe/public/js/frappe/ui/components/dropdown.js +++ b/frappe/public/js/frappe/ui/components/dropdown.js @@ -8,7 +8,7 @@ frappe.provide("frappe.ui"); * @typedef {Object} DropdownOpts * @property {Element|JQuery} [trigger] An existing element that opens the menu. Omit to have one made from `button`. * @property {Object} [button] Options for the generated frappe.ui.button trigger (ignored when `trigger` is passed). - * @property {Array|function} options Menu items and { group, options } sections — see MenuItem in menu.js. A function is called fresh on every open. + * @property {Array|function} options Menu items and { group, options } sections — see MenuItem in menu.js. A function is called fresh on every open, and may return a Promise of the items — the menu opens with a loading row and fills in when it settles. Individual items can lazy-load the same way via a function `submenu`. * @property {"top"|"right"|"bottom"|"left"} [side="bottom"] Which side of the trigger the menu opens on. * @property {"start"|"center"|"end"} [align="start"] How the menu lines up along that side. * @property {number} [offset=4] Gap between trigger and menu, in px. diff --git a/frappe/public/js/frappe/ui/components/menu.js b/frappe/public/js/frappe/ui/components/menu.js index 84772b06a1d5..f46a677f2dc6 100644 --- a/frappe/public/js/frappe/ui/components/menu.js +++ b/frappe/public/js/frappe/ui/components/menu.js @@ -24,6 +24,11 @@ import { place } from "./position.js"; * * Rows are built with createElement + textContent, so labels can never * smuggle markup — there is no HTML channel here at all. + * + * Async: the root options (via a function returning a promise, or a bare + * promise) and function submenus may resolve later — the panel opens with a + * loading placeholder and fill() swaps the rows in when the data lands, + * re-measuring the position. Late resolutions after a close are discarded. */ /** @@ -38,7 +43,7 @@ import { place } from "./position.js"; * @property {function} [onclick] Called with the click event. The menu closes after. * @property {string} [href] Renders the row as a link. Code-running schemes are refused. * @property {function} [condition] Checked on every open; return false to hide the row. - * @property {MenuItem[]} [submenu] Nested rows — the row opens a side panel instead of acting. + * @property {MenuItem[]|function} [submenu] Nested rows — the row opens a side panel instead of acting. A function runs at hover-start (once per menu open, result cached) and may return the rows or a Promise of them; the panel shows a loading state until it settles. */ /** @@ -50,6 +55,10 @@ import { place } from "./position.js"; const THEMES = ["gray", "red"]; +function is_thenable(value) { + return !!value && typeof value.then === "function"; +} + const SUBMENU_OFFSET = 4; const SUBMENU_OPEN_DELAY = 150; const EXIT_MS = 140; @@ -220,12 +229,10 @@ function build_item(item, { reserve_icon_space, component, taken }) { return { el, mnemonic }; } -function build_panel(groups, { empty_text, component }) { - const panel = document.createElement("div"); - panel.className = "es-menu"; - panel.setAttribute("role", "menu"); - panel.setAttribute("tabindex", "-1"); - +// fill a bare panel with its group/row elements (or the empty text) and +// return the rows. Separate from build_panel so a panel that opened in its +// loading state can be filled in place when the async items arrive. +function render_content(panel, groups, { empty_text, component }) { const rows = []; // [{ el, item, mnemonic }] in visual order const taken = new Set(); // Alt-mnemonic letters, unique within a panel @@ -234,7 +241,7 @@ function build_panel(groups, { empty_text, component }) { empty.className = "es-menu__empty"; empty.textContent = empty_text || __("No options"); panel.appendChild(empty); - return { panel, rows }; + return rows; } for (const group of groups) { @@ -267,6 +274,39 @@ function build_panel(groups, { empty_text, component }) { panel.appendChild(group_el); } + return rows; +} + +// the placeholder shown while async items load: a spinner + "Loading...", +// inert like the empty state (no rows, so keyboard navigation has nothing +// to land on; Escape and outside-click work through the panel as usual) +function render_loading(panel) { + const loading = document.createElement("div"); + loading.className = "es-menu__loading"; + const spinner = document.createElement("span"); + spinner.className = "es-spinner"; + spinner.setAttribute("aria-hidden", "true"); + const text = document.createElement("span"); + text.textContent = __("Loading..."); + loading.append(spinner, text); + panel.appendChild(loading); + panel.setAttribute("aria-busy", "true"); +} + +// `groups === null` means the items are still loading (async source) — the +// panel opens with the loading placeholder and fill() swaps the rows in +function build_panel(groups, { empty_text, component }) { + const panel = document.createElement("div"); + panel.className = "es-menu"; + panel.setAttribute("role", "menu"); + panel.setAttribute("tabindex", "-1"); + + if (groups === null) { + render_loading(panel); + return { panel, rows: [] }; + } + + const rows = render_content(panel, groups, { empty_text, component }); return { panel, rows }; } @@ -287,6 +327,9 @@ export class MenuTree { this.lock_scroll = lock_scroll; this.panels = []; // [{ panel, rows, anchor, side, align, offset, parent_row }] this.closed = false; + // function submenus resolve once per menu open: row -> items or the + // pending promise (see get_submenu_items) + this.submenu_cache = new Map(); this.submenu_timer = null; this.typeahead_buffer = ""; this.typeahead_timer = null; @@ -295,17 +338,22 @@ export class MenuTree { open({ side = "bottom", align = "start", offset = 4, motion = "animated", focus = null }) { // options can be a function so the menu reflects right-now state - // (which row was clicked, what's selected); it runs on every open - const groups = normalize_options( - typeof this.options === "function" ? this.options() : this.options - ); - const entry = this.mount(groups, { - anchor: this.anchor, - side, - align, - offset, - motion, - }); + // (which row was clicked, what's selected); it runs on every open. + // A promise (or a function returning one) opens the menu in its + // loading state, and the rows fill in when it settles. + const value = typeof this.options === "function" ? this.options() : this.options; + const mount_opts = { anchor: this.anchor, side, align, offset, motion }; + let entry; + if (is_thenable(value)) { + entry = this.mount(null, mount_opts); + entry.pending_focus = focus === "first" || focus === "last" ? focus : null; + value.then( + (items) => this.fill(entry, normalize_options(items)), + (error) => this.fill_failed(entry, error) + ); + } else { + entry = this.mount(normalize_options(value), mount_opts); + } // pressing down anywhere that isn't a panel (or the trigger, which // handles its own clicks) closes the menu @@ -352,9 +400,11 @@ export class MenuTree { // keyboard opens land on a row right away (ArrowDown = first, // ArrowUp = last); mouse opens just focus the panel so keys work - // without stealing the highlight from wherever the pointer is - if (focus === "first") this.focus_step(entry, 1); - else if (focus === "last") this.focus_step(entry, -1); + // without stealing the highlight from wherever the pointer is. An + // async panel has no rows yet — focus the panel so Escape works, and + // fill() honors pending_focus when the rows arrive + if (focus === "first" && entry.rows.length) this.focus_step(entry, 1); + else if (focus === "last" && entry.rows.length) this.focus_step(entry, -1); else entry.panel.focus({ preventScroll: true }); } @@ -388,9 +438,26 @@ export class MenuTree { } bind_panel(entry) { - const { panel, rows } = entry; + this.bind_rows(entry); + + // whichever row has focus carries the highlight — this is the only + // place data-highlighted is ever set. Reads entry.rows live (not a + // captured copy) so an async fill's new rows are covered too. + entry.panel.addEventListener("focusin", (e) => { + for (const row of entry.rows) { + row.el.toggleAttribute("data-highlighted", row.el === e.target); + } + }); + + // key presses on focused rows bubble up to the panel, so one + // listener per panel covers all of its rows + entry.panel.addEventListener("keydown", (e) => this.handle_keydown(entry, e)); + } - for (const row of rows) { + // per-row listeners; called at mount and again after an async fill + // replaces a panel's rows + bind_rows(entry) { + for (const row of entry.rows) { row.el.addEventListener("click", (e) => { // disabled buttons don't fire click, but a scripted or AT // activation can still reach here — never act on a disabled row @@ -417,18 +484,47 @@ export class MenuTree { this.schedule_submenu(entry, row); }); } + } - // whichever row has focus carries the highlight — this is the only - // place data-highlighted is ever set - panel.addEventListener("focusin", (e) => { - for (const row of rows) { - row.el.toggleAttribute("data-highlighted", row.el === e.target); - } + // swap a loading panel's placeholder for the resolved rows, in place. + // Late resolutions are dropped: the whole menu may have closed, or just + // that submenu panel (hover moved on) — either way there's nothing to do. + fill(entry, groups) { + if (this.closed || !this.panels.includes(entry)) return; + // read focus before replaceChildren blurs whatever held it + const had_focus = + entry.panel === document.activeElement || entry.panel.contains(document.activeElement); + entry.panel.replaceChildren(); + entry.panel.removeAttribute("aria-busy"); + entry.rows = render_content(entry.panel, groups, { + empty_text: this.empty_text, + component: this.component, }); + this.bind_rows(entry); + // the panel's size just changed — measure, flip and clamp again + place(entry.panel, entry.anchor(), entry.side, entry.align, entry.offset); + if (entry.pending_focus) { + this.focus_step(entry, entry.pending_focus === "last" ? -1 : 1); + entry.pending_focus = null; + } else if (had_focus) { + entry.panel.focus({ preventScroll: true }); + } + } - // key presses on focused rows bubble up to the panel, so one - // listener per panel covers all of its rows - panel.addEventListener("keydown", (e) => this.handle_keydown(entry, e)); + // async items rejected: keep the panel up with an inert notice — a dead + // hover with no feedback would look like the menu is broken + fill_failed(entry, error) { + if (this.closed || !this.panels.includes(entry)) return; + console.warn(`frappe.ui.${this.component}: menu items failed to load`, error); + entry.panel.replaceChildren(); + entry.panel.removeAttribute("aria-busy"); + const failed = document.createElement("div"); + failed.className = "es-menu__empty"; + failed.textContent = __("Couldn't load options"); + entry.panel.appendChild(failed); + entry.rows = []; + entry.pending_focus = null; + place(entry.panel, entry.anchor(), entry.side, entry.align, entry.offset); } // all keyboard behavior for one panel. `entry` is the panel the focused @@ -605,6 +701,10 @@ export class MenuTree { // other row closes panels deeper than this one schedule_submenu(entry, row) { clearTimeout(this.submenu_timer); + // a function submenu starts resolving at hover-START, before the open + // delay — a fast source is ready before the panel ever shows, so the + // loading state often never appears at all + if (row.item.submenu) this.get_submenu_items(row); const depth = this.panels.indexOf(entry); this.submenu_timer = setTimeout(() => { if (this.closed) return; @@ -613,19 +713,41 @@ export class MenuTree { }, SUBMENU_OPEN_DELAY); } + // a submenu's items: an array passes through; a function runs once per + // menu open and its result — value or promise — is cached, so hovering + // away and back doesn't refetch. When a promise resolves, the cache is + // swapped to the plain items so the next open of that row is synchronous. + // A rejected promise stays cached (no retry until the next menu open); + // the rejection itself is handled where the panel consumes it. + get_submenu_items(row) { + const source = row.item.submenu; + if (typeof source !== "function") return source; + if (!this.submenu_cache.has(row)) { + const value = source(); + this.submenu_cache.set(row, value); + if (is_thenable(value)) { + value.then( + (items) => this.submenu_cache.set(row, items), + () => {} // fill_failed reports it; this guard just keeps the prefetch handled + ); + } + } + return this.submenu_cache.get(row); + } + open_submenu(parent_entry, row, { focus }) { // this row's submenu is already open — just step into it if asked const depth = this.panels.indexOf(parent_entry); const already = this.panels[depth + 1]; if (already && already.parent_row === row.el) { - if (focus) this.focus_step(already, 1); + if (focus && already.rows.length) this.focus_step(already, 1); return; } // a sibling's submenu may be open — only one branch at a time this.close_submenus_from(depth + 1); - const groups = normalize_options(row.item.submenu); - const entry = this.mount(groups, { + const source = this.get_submenu_items(row); + const mount_opts = { // the submenu hangs off its row and follows it around, opening // toward the reading direction (left in RTL) anchor: () => row.el.getBoundingClientRect(), @@ -634,10 +756,21 @@ export class MenuTree { offset: SUBMENU_OFFSET, motion: "animated", parent_row: row.el, - }); - // keyboard opens step onto the first row; hover opens leave focus - // on the parent row until the pointer moves in - if (focus) this.focus_step(entry, 1); + }; + let entry; + if (is_thenable(source)) { + entry = this.mount(null, mount_opts); + if (focus) entry.pending_focus = "first"; + source.then( + (items) => this.fill(entry, normalize_options(items)), + (error) => this.fill_failed(entry, error) + ); + } else { + entry = this.mount(normalize_options(source), mount_opts); + // keyboard opens step onto the first row; hover opens leave focus + // on the parent row until the pointer moves in + if (focus) this.focus_step(entry, 1); + } } // close every panel deeper than `depth` (the Math.max keeps the root