improvement(workflow-panel): header tooltips, search shortcut hint, URL-driven active tab#4523
Conversation
|
@stylessh is attempting to deploy a commit to the Sim Team on Vercel. A member of the Team first needs to authorize it. |
PR SummaryMedium Risk Overview To support tab changes initiated outside React, the editor store dispatches a Reviewed by Cursor Bugbot for commit 6848b92. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryThis PR polishes the workflow panel's right side by migrating the active tab from Zustand's persisted store to URL state via
Confidence Score: 4/5The core tab-flash fix is well-designed and the CustomEvent bridge handles the nuqs/store boundary cleanly. The main area to watch is use-block-visual.ts — every block now subscribes to URL changes, which is a regression from the previous targeted Zustand selector on large workflows. The URL-driven tab approach is sound and the blocking-script/hydration handoff is carefully sequenced. The only functional concern is that migrating the panel tab check into useQueryState inside use-block-visual.ts makes all workflow blocks re-render on every tab switch, whereas the old Zustand selector short-circuited for non-relevant blocks. For typical workflow sizes this is unlikely to be noticeable, but it could surface on larger workflows. apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-block-visual.ts — the useQueryState call here is the main spot to revisit for render performance on large workflows. Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser
participant BlockingScript as Blocking Script (layout.tsx)
participant NuqsAdapter
participant Panel
participant EditorStore as usePanelEditorStore
participant BlockVisual as useBlockVisual (each block)
Browser->>BlockingScript: "Hard refresh /?panel=editor"
BlockingScript->>Browser: setAttribute('data-panel-active-tab', 'editor')
Note over Browser: CSS hides non-editor tabs before React hydrates
Browser->>NuqsAdapter: React hydrates
NuqsAdapter->>Panel: useQueryState('panel') → 'editor'
Panel->>Browser: removeAttribute('data-panel-active-tab')
Note over Browser: React now owns tab visibility
Browser->>EditorStore: User clicks a block
EditorStore->>EditorStore: setCurrentBlockId(blockId)
EditorStore->>Browser: dispatchEvent('panel:set-tab', 'editor')
Browser->>Panel: CustomEvent handler fires
Panel->>NuqsAdapter: setActiveTabRaw('editor')
NuqsAdapter->>Browser: "URL updated → ?panel=editor"
NuqsAdapter->>Panel: "re-render (activeTab = 'editor')"
NuqsAdapter->>BlockVisual: "re-render × N blocks (panelTab = 'editor')"
Reviews (1): Last reviewed commit: "remove redundant data attribute cleanup ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const [panelTab] = useQueryState('panel', parseAsStringLiteral(PANEL_TABS).withDefault('copilot')) | ||
| const isEditorOpen = !isPreview && !isEmbedded && isThisBlockInEditor && panelTab === 'editor' |
There was a problem hiding this comment.
useQueryState hooks are notified via React context whenever the ?panel= URL param changes, so every block on the canvas will re-render each time the user switches tabs. The previous Zustand selector approach short-circuited early — if (isPreview || isEmbedded || !isThisBlockInEditor) return false — so Zustand never subscribed those blocks to activeTab mutations. For workflows with many blocks, switching tabs will now trigger N extra renders instead of at most 1. Consider moving the panelTab === 'editor' check back into the usePanelEditorStore selector (e.g. adding activeTab back to the store as a non-persisted field, or deriving it from the URL once in the Panel and propagating it down via a lightweight context).
| const PANEL_TABS = ['copilot', 'toolbar', 'editor'] as const | ||
|
|
||
| /** | ||
| * Props for the useBlockVisual hook. |
There was a problem hiding this comment.
PANEL_TABS constant is duplicated across files
The same ['copilot', 'toolbar', 'editor'] as const array is defined independently in both use-block-visual.ts and panel.tsx. PanelTab is already exported from stores/panel/types.ts, so the backing tuple could live there too, giving a single source of truth for the literal union.
| const PANEL_TABS = ['copilot', 'toolbar', 'editor'] as const | |
| /** | |
| * Props for the useBlockVisual hook. | |
| // Consider exporting PANEL_TABS from '@/stores/panel/types' and importing it here | |
| // to avoid duplicating the constant that also lives in panel.tsx. | |
| const PANEL_TABS = ['copilot', 'toolbar', 'editor'] as const | |
| /** | |
| * Props for the useBlockVisual hook. |
| useEffect(() => { | ||
| const handler = (e: Event) => { | ||
| const tab = (e as CustomEvent<PanelTab>).detail | ||
| if (tab === 'copilot' || tab === 'toolbar' || tab === 'editor') { | ||
| setActiveTab(tab) | ||
| } | ||
| } | ||
| window.addEventListener('panel:set-tab', handler) | ||
| return () => window.removeEventListener('panel:set-tab', handler) | ||
| }, [setActiveTab]) |
There was a problem hiding this comment.
panel:set-tab event is fire-and-forget — missed events leave the tab unchanged
requestEditorTab() dispatches a synchronous CustomEvent on window, and the Panel only processes it while mounted with an active listener. If the Panel is mid-unmount/remount (e.g. during a fast route transition followed by an immediate block click), the event fires into a gap where no listener is registered and the URL never updates — the editor tab silently fails to open. A safety net like checking activeTab after a microtask, or using a small shared atom/signal instead of a one-shot event, would make the bridge more resilient.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6848b92. Configure here.
| import { usePanelEditorStore } from '@/stores/panel' | ||
| import { useWorkflowRegistry } from '@/stores/workflows/registry/store' | ||
|
|
||
| const PANEL_TABS = ['copilot', 'toolbar', 'editor'] as const |
There was a problem hiding this comment.
Duplicated PANEL_TABS array risks silent desynchronization
Low Severity
The PANEL_TABS constant is independently defined in both use-block-visual.ts and panel.tsx. Both must stay in sync with the PanelTab type in @/stores/panel/types.ts, which already serves as the source of truth for valid tab values. If a tab is added or renamed, only one array might get updated, causing the nuqs parser in the other file to silently reject the new value and fall back to 'copilot'. Extracting a single shared PANEL_TABS array next to the PanelTab type would eliminate this drift risk.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6848b92. Configure here.
1 participant
Summary
A handful of polish changes on the workflow page's right-side panel:
Infoicons next to the Triggers and Blocks labels with concise definitions ("Events that start a workflow." / "Actions that make up the steps of a workflow."). The Blocks icon stops click propagation so the section's collapse toggle isn't triggered when hovering the icon.kbd-style badge next to the toolbar search icon showing the existingMod+Alt+Fshortcut. Uses lucideOptionandCommandicons on Mac, falls back toCtrl+Alt+Ftext on Windows/Linux. The badge waits for client mount before rendering so the wrong platform doesn't flash.activeTabfrom the persisted Zustand store into the URL vianuqs(?panel=copilot|toolbar|editor). The blocking script inapp/layout.tsxnow reads?panel=instead oflocalStorage, so a hard refresh paints the correct tab before React hydrates — fixes the flash where Copilot rendered briefly before swapping to the previously-selected tab.nuqsis added toapps/simat2.8.9to match the install in #4522, andNuqsAdapteris wired inapp/layout.tsxin the same position (insideQueryProvider, outsideSessionProvider) so the two PRs don't conflict.The editor store's old
usePanelStore.getState().setActiveTab('editor')call (fired from outside React when a block is selected) goes through a newpanel:set-tabCustomEventthat the panel component listens for, since nuqs state can only be updated from inside React.Test plan
?panel=...(or clears when on the default Copilot tab).⌘⌥F/Ctrl+Alt+F→ toolbar tab focuses and the search input opens.Infoicons in the toolbar → tooltips appear with the right copy.kbdbadge next to the search icon does not flash with the wrong platform variant on first load.