[OPIK-7126] [FE] feat: unify workspace menu with projects menu#7484
Conversation
Bring the workspace switcher in line with the projects menu (OPIK_7125): scoped search, recency-ordered + pinnable sections, and a "View all" footer. - Search placeholder "Search" -> "Search workspace". - Order workspaces by recency: new useRecentWorkspaces hook records visits in localStorage (capped) and sorts recently-visited first, alphabetical fallback for never-visited. - Cluster into "Pinned" + "Recently visited" (top 10) sections; flat virtualized list while searching. Pinning via new usePinnedWorkspaces hook (org-scoped localStorage), mirroring usePinnedProjects; pin order stays stable (insertion order). - Footer "Manage workspaces" -> "View all workspaces" with an arrow-up-right icon, matching the projects menu. - Org header always opens the organizations submenu (chevron), matching the Figma; the per-org settings action lives inside that submenu. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
⏱️ pre-commit per-hook timing
⏭️ 37 skipped (no matching files changed)
|
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
| <ListAction variant="default" size="sm" asChild> | ||
| <a href={buildUrl("account-settings/workspaces", workspaceName)}> | ||
| <Settings2 className="size-3.5 text-light-slate" /> | ||
| <span>Manage workspaces</span> | ||
| <span>View all workspaces</span> |
There was a problem hiding this comment.
Footer no longer switches workspaces
View all workspaces now points to account-settings/workspaces, which opens the Workspace preferences tab instead of a workspace list or switcher, so hidden workspaces can't be reached from the footer — should we route it to a workspace-selection page or rename it to match the settings destination?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/plugins/comet/WorkspaceMenuContent.tsx around lines 323-325, the
footer ListAction renders an <a> with the label “View all workspaces” but it
navigates to buildUrl("account-settings/workspaces", workspaceName), which leads to
ConfigurationPage’s WorkspacePreferencesTab instead of the workspace switcher/list.
Fix by either updating the href to the actual workspace-selection/list route used for
switching (so the action opens a page that shows all workspaces), or if no such route
exists, rename the CTA to reflect the destination (e.g., “Workspace preferences”) to
avoid misleading UX. Verify the route you choose matches how
ConfigurationPage/WorkspacePreferencesTab is reached today and that unpinned/non-recent
workspaces become reachable from this CTA.
There was a problem hiding this comment.
Not a regression — dismissing this one.
Two things are off here:
-
The destination didn't change in this PR. The
hrefis untouched; only the label (Manage workspaces→View all workspaces) and the icon (gear → arrow) changed. This footer has always pointed ataccount-settings/workspaces, so nothing about workspace reachability regressed. The "now points to" framing is what drove the severity, and it's not accurate. -
It's not
WorkspacePreferencesTab.buildUrlprefixesVITE_BASE_COMET_URL, soaccount-settings/workspacesresolves to the Comet platform's account-settings → workspaces page (the same URLuseInviteMembersURL.tsbuilds), not opik's internalConfigurationPage/WorkspacePreferencesTab. That platform page is the canonical "all workspaces" list, so theView all workspaceslabel is accurate.
No code change needed.
There was a problem hiding this comment.
Thanks for the correction — that makes sense. I'll save this to memory once the PR is merged.
There was a problem hiding this comment.
Commit 8e84029 addressed this comment by renaming the footer CTA from "Manage workspaces" to "View all workspaces" while leaving the existing destination unchanged. That matches the suggested fallback fix of aligning the label with the settings page destination.
| // Scoped per organization: pinning is only meaningful among the workspaces of the | ||
| // current org. Mirrors usePinnedProjects (localStorage, {id,name}, functional updates). | ||
| const usePinnedWorkspaces = ( | ||
| organizationId: string | undefined, | ||
| ): UsePinnedWorkspacesResult => { | ||
| const [pinnedWorkspaces = [], setPinnedWorkspaces] = useLocalStorageState< | ||
| PinnedWorkspace[] | ||
| >(`workspaces:pinnedConfig:${organizationId ?? "unknown"}`, { | ||
| defaultValue: [], | ||
| }); | ||
|
|
||
| const isPinned = useCallback( |
There was a problem hiding this comment.
Duplicated pinning hook
useLocalStorageState mirrors usePinnedProjects in src/v2/layout/ProjectMenu/usePinnedProjects.ts, so persistence and deduplication logic live in two places and will likely diverge — should we share the existing hook or extract a small reusable helper?
Want Baz to fix this for you? Activate Fixer
|
✅ Test environment is now available! To configure additional Environment variables for your environment, run [Deploy Opik AdHoc Environment workflow] (https://github.com/comet-ml/comet-deployment/actions/workflows/deploy_opik_adhoc_env.yaml) Access Information
The deployment has completed successfully and the version has been verified. |
Match the Figma instruction note and the projects menu section header
("Recently updated"). Only the user-facing label changes; the underlying
recency mechanism (client-side visit tracking) and its hook/variable names
are unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| <MenuSectionLabel>Recently updated</MenuSectionLabel> | ||
| {recentlyVisitedWorkspaces.map(renderWorkspaceItem)} |
There was a problem hiding this comment.
MenuSectionLabel says "Recently updated" but the list is driven by client-side visit timestamps from useRecentWorkspaces/getVisitedAt, not a backend updated_at field — should we rename it back to "Recently visited" and update any nearby comments to match?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
In apps/opik-frontend/src/plugins/comet/WorkspaceMenuContent.tsx around lines 313-314,
the MenuSectionLabel text is set to "Recently updated" even though the list is derived
from client-side visit timestamps (useRecentWorkspaces/getVisitedAt) rather than any
backend updated_at field. Change the MenuSectionLabel text to "Recently visited" and
update any surrounding comments in this section to match. Do not change the data
fetching logic unless the backend provides last-updated timestamps. After the change,
verify there are no other "Recently updated" labels in this component that refer to this
same recentlyVisitedWorkspaces section.
There was a problem hiding this comment.
Commit 1f00c83 addressed this comment by renaming the section label from "Recently updated" to "Recently visited". It also updated the nearby comment to describe the section as "Pinned + Recently visited," matching the client-side visit timestamp logic.
| const pinWorkspace = useCallback( | ||
| (workspace: PinnedWorkspace) => { | ||
| setPinnedWorkspaces((prev = []) => | ||
| prev.some((w) => w.workspaceId === workspace.workspaceId) | ||
| ? prev |
There was a problem hiding this comment.
Duplicated pin-hook logic
pinWorkspace is almost the same as pinProject, so the localStorage, deduplication, and append logic will drift whenever one side changes — should we extract a generic usePinnedItems helper parametrized by the storage key and id/name selectors?
Want Baz to fix this for you? Activate Fixer
Revert the previous rename: "Recently visited" is the correct label for the workspace menu (it reflects the client-side visit-based ordering, and matches the workspace-menu component in Figma). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| <> | ||
| {pinnedWorkspaces.length > 0 && ( | ||
| <> | ||
| <MenuSectionLabel>Pinned</MenuSectionLabel> | ||
| {pinnedWorkspaces.map(renderWorkspaceItem)} | ||
| </> | ||
| )} | ||
| {recentlyVisitedWorkspaces.length > 0 && ( | ||
| <> | ||
| {pinnedWorkspaces.length > 0 && ( | ||
| <DropdownMenuSeparator className="my-1" /> | ||
| )} |
There was a problem hiding this comment.
Duplicate section layout
The pinned/recent section markup in the new workspace menu duplicates ProjectMenuContent.tsx, so the two views can drift on labels, spacing, and empty-state behavior and any tweak has to be made twice — should we extract the shared section rendering into a common helper/component?
Want Baz to fix this for you? Activate Fixer
awkoy
added
test-environment
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
|
✅ Test environment is now available! To configure additional Environment variables for your environment, run [Deploy Opik AdHoc Environment workflow] (https://github.com/comet-ml/comet-deployment/actions/workflows/deploy_opik_adhoc_env.yaml) Access Information
The deployment has completed successfully and the version has been verified. |
|
🌙 Nightly cleanup: The test environment for this PR ( |
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
|
✅ Test environment is now available! To configure additional Environment variables for your environment, run [Deploy Opik AdHoc Environment workflow] (https://github.com/comet-ml/comet-deployment/actions/workflows/deploy_opik_adhoc_env.yaml) Access Information
The deployment has completed successfully and the version has been verified. |
Keep the hasMultipleOrganizations branch: single-org users get the inline org header with a directly-visible settings gear instead of a submenu that discloses a one-item list and buries org settings a click deeper. Multi-org users keep the organizations submenu. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
|
||
| // When searching, show all matches flat (virtualized above the threshold). | ||
| // Otherwise cluster into Pinned + Recently visited (capped) sections. | ||
| const workspacesById = new Map( |
There was a problem hiding this comment.
💡 suggestion | Performance
Derived workspace collections are rebuilt on every render, including during search when they're discarded unused.
workspacesById (a new Map scanning all workspaces), pinnedWorkspaces (map+filter), and recentlyVisitedWorkspaces (filter+slice) are computed unconditionally in the component body. The menu re-renders on every search keystroke, so these O(n) derivations re-run each time — and they're only consumed in the non-search branch, so during search they're computed and thrown away.
The sibling ProjectMenuContent.tsx memoizes the analogous projectsById with useMemo. Consider wrapping these in useMemo.
🤖 Review posted via /review-github-pr
There was a problem hiding this comment.
Done in c05ff5d — wrapped workspacesById, pinnedWorkspaces, and recentlyVisitedWorkspaces in useMemo, matching ProjectMenuContent's memoized projectsById. Deps are referentially stable: sortedWorkspaces is already memoized in useWorkspaceSelectorData, and isPinned/pinnedConfig come from useCallback/useLocalStorageState. Lint, typecheck, and the 11 hook tests (Node 20) are green.
There was a problem hiding this comment.
Commit c05ff5d addressed this comment by memoizing workspacesById, pinnedWorkspaces, and recentlyVisitedWorkspaces with useMemo. This prevents those O(n) derivations from being rebuilt on every render, including search re-renders where they would otherwise be discarded.
|
👋 Review summary What looks good
Overall Inline comments: 1 optional suggestion left (memoize the derived workspace collections for parity with 🤖 Review posted via /review-github-pr |
Wrap workspacesById, pinnedWorkspaces, and recentlyVisitedWorkspaces in useMemo so the O(n) derivations don't re-run on every render (e.g. each search keystroke), matching ProjectMenuContent's memoized projectsById. sortedWorkspaces is already memoized upstream in useWorkspaceSelectorData, so the dependencies are referentially stable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3 participants
Details
Unifies the workspace switcher with the projects menu shipped in OPIK_7125, so switching workspace feels the same as switching project. The menu now has a scoped search, recency-ordered and pinnable sections, and a "View all" footer.
Because workspaces come from the Comet platform API (
/workspaces/lite), which exposes no last-updated field, recency is tracked client-side as "recently visited" (per the design's label) — no backend dependency.Search→Search workspace.useRecentWorkspaceshook records visits in localStorage (capped at 50) and sorts recently-visited first, with an alphabetical fallback for never-visited workspaces.usePinnedWorkspaceshook (org-scoped localStorage), mirroringusePinnedProjects; pinned order is stable (insertion order).Manage workspaces→ View all workspaces with an arrow-up-right icon, matching the projects menu.Figma: https://www.figma.com/design/MHqvEwnuCHBLVvN9ilmhw8/Information-Architecture?node-id=716-5458
Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
Testing
Ran the frontend in
--mode cometagainstdev.comet.com(workspacesyaroslav-boiko/v2jj):#94A3B8section labels,#373D4Drows, 32px row height).npm run typecheckandeslint --max-warnings=0on the changed files — both clean.usePinnedWorkspaces,useRecentWorkspaces) — 11 tests, green on Node 20 (CI's version).Documentation
N/A — navigation/menu behavior change; no user-facing docs affected.
🤖 Generated with Claude Code