feat: Replace BuilderExitButton with new BuilderFooterToolbar#9378
Merged
pythongosssss merged 3 commits intomainfrom Mar 4, 2026
Merged
feat: Replace BuilderExitButton with new BuilderFooterToolbar#9378pythongosssss merged 3 commits intomainfrom
pythongosssss merged 3 commits intomainfrom
Conversation
- add back/next navigation to builder footer alongside exit utton - extract shared step logic into useBuilderSteps composable
Contributor
📝 WalkthroughWalkthroughA new BuilderFooterToolbar component with back/next navigation buttons replaces BuilderExitButton, consolidating step navigation logic into the expanded useBuilderSteps hook. BuilderToolbar refactored to use the new navigation pattern. Tests verify navigation behavior across step boundaries and conditional button states. Changes
Sequence DiagramsequenceDiagram
participant User
participant BuilderFooterToolbar
participant useBuilderSteps
participant appModeStore
participant useAppSetDefaultView
User->>BuilderFooterToolbar: Clicks Next Button
BuilderFooterToolbar->>useBuilderSteps: navigateToStep(stepId)
alt stepId is 'setDefaultView'
useBuilderSteps->>appModeStore: setMode('builder:arrange')
useBuilderSteps->>useAppSetDefaultView: showDialog()
useAppSetDefaultView-->>User: Display Default View Dialog
else stepId is regular step
useBuilderSteps->>appModeStore: setMode(stepId)
end
appModeStore-->>BuilderFooterToolbar: State updated
BuilderFooterToolbar-->>User: Buttons re-render with new state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 549 passed, 0 failed · 5 flaky📊 Browser Reports
|
📦 Bundle: 4.48 MB gzip 🔴 +427 BDetailsSummary
Category Glance App Entry Points — 17.7 kB (baseline 17.7 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 987 kB (baseline 985 kB) • 🔴 +2.23 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.1 kB (baseline 72.1 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 436 kB (baseline 436 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Editors & Dialogs — 736 B (baseline 736 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
UI Components — 47 kB (baseline 47 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 2.64 MB (baseline 2.64 MB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 55.5 kB (baseline 55.5 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 8.86 MB (baseline 8.86 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.89 MB (baseline 7.89 MB) • ⚪ 0 BBundles that do not match a named category
|
⚡ Performance Report
Raw data{
"timestamp": "2026-03-04T17:44:38.404Z",
"gitSha": "0a999aa739add75fecd99db42522c0b9f133fe37",
"branch": "pysssss/builder-footer-toolbar",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2005.608000000052,
"styleRecalcs": 121,
"styleRecalcDurationMs": 18.619,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 436.256,
"heapDeltaBytes": -3815632
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1831.1330000000225,
"styleRecalcs": 166,
"styleRecalcDurationMs": 46.804,
"layouts": 12,
"layoutDurationMs": 3.1090000000000004,
"taskDurationMs": 780.487,
"heapDeltaBytes": -4480552
},
{
"name": "dom-widget-clipping",
"durationMs": 584.1740000000755,
"styleRecalcs": 41,
"styleRecalcDurationMs": 11.620000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 350.959,
"heapDeltaBytes": 7307468
}
]
} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/builder/BuilderFooterToolbar.test.ts (1)
66-151: Add keyboard-path tests for Escape handling.The suite covers click navigation well, but it does not validate the new
windowEscape flow. Please add cases for:
- exits when in builder mode and no dialogs are open
- no exit when dialog stack is non-empty
- no exit when not in builder mode
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/builder/BuilderFooterToolbar.test.ts` around lines 66 - 151, Add three unit tests in BuilderFooterToolbar.test.ts that dispatch a window keydown with key "Escape" and assert exit behavior: (1) when mockState.mode is 'builder:select' or another builder mode and there are no open dialogs (ensure mockShowDialog or dialog stack is empty/false), dispatch new KeyboardEvent('keydown', { key: 'Escape' }) on window and expect mockExitBuilder toHaveBeenCalledOnce(); (2) when the dialog stack is non-empty (stub/mock whatever tracks dialogs so mockShowDialog or the dialog-stack helper returns true), dispatch the same Escape event and expect mockExitBuilder not to have been called; (3) when mockState.mode is not a builder mode (set mockState.mode to something else), dispatch Escape and expect mockExitBuilder not to have been called; use mountComponent() to mount the component before dispatching and clear mocks between tests as in beforeEach.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/builder/BuilderFooterToolbar.vue`:
- Around line 49-61: The handler registered with useEventListener currently
reacts to every keydown including repeats; add a guard to ignore repeated
keydown events (use the KeyboardEvent.repeat property) before proceeding to call
onExitBuilder so holding Escape doesn't trigger multiple exits; update the
keydown callback that references isBuilderMode.value and dialogStore.dialogStack
to early-return when e.repeat is true (or alternatively track an
exit-in-progress flag cleared on keyup) to ensure onExitBuilder is invoked only
once per physical key press.
In `@src/components/builder/useBuilderSteps.ts`:
- Around line 23-33: The code currently force-casts mode.value to BuilderStepId
in the activeStep computed which can produce an invalid step not present in
BUILDER_STEPS; update activeStep (and thereby activeStepIndex) to first check
whether mode.value is a valid member of BUILDER_STEPS (e.g.,
BUILDER_STEPS.includes(mode.value)) before casting/returning it, and if not
present fall back to a safe default such as 'builder:select' (or
'setDefaultView' when settingView.value) so that
BUILDER_STEPS.indexOf(activeStep.value) never returns -1 and downstream
navigation always receives a valid step id.
---
Nitpick comments:
In `@src/components/builder/BuilderFooterToolbar.test.ts`:
- Around line 66-151: Add three unit tests in BuilderFooterToolbar.test.ts that
dispatch a window keydown with key "Escape" and assert exit behavior: (1) when
mockState.mode is 'builder:select' or another builder mode and there are no open
dialogs (ensure mockShowDialog or dialog stack is empty/false), dispatch new
KeyboardEvent('keydown', { key: 'Escape' }) on window and expect mockExitBuilder
toHaveBeenCalledOnce(); (2) when the dialog stack is non-empty (stub/mock
whatever tracks dialogs so mockShowDialog or the dialog-stack helper returns
true), dispatch the same Escape event and expect mockExitBuilder not to have
been called; (3) when mockState.mode is not a builder mode (set mockState.mode
to something else), dispatch Escape and expect mockExitBuilder not to have been
called; use mountComponent() to mount the component before dispatching and clear
mocks between tests as in beforeEach.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7fb87c61-d51f-4444-a56f-1ec5831b5a62
📒 Files selected for processing (7)
src/components/builder/BuilderExitButton.vuesrc/components/builder/BuilderFooterToolbar.test.tssrc/components/builder/BuilderFooterToolbar.vuesrc/components/builder/BuilderToolbar.vuesrc/components/builder/useBuilderSteps.tssrc/locales/en/main.jsonsrc/views/GraphView.vue
💤 Files with no reviewable changes (1)
- src/components/builder/BuilderExitButton.vue
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/builder/BuilderFooterToolbar.vue (1)
45-57:⚠️ Potential issue | 🟡 MinorGuard Escape key-repeat to prevent repeated exits.
Line 45 currently handles every repeated
keydown; holding Escape can trigger multipleonExitBuilder()calls.Patch
useEventListener(window, 'keydown', (e: KeyboardEvent) => { if ( + !e.repeat && e.key === 'Escape' && !e.ctrlKey && !e.altKey && !e.metaKey &&#!/bin/bash # Verify the keydown guard and whether repeat is handled. rg -n "useEventListener\\(window, 'keydown'" src/components/builder/BuilderFooterToolbar.vue -A20 rg -n "e\\.repeat" src/components/builder/BuilderFooterToolbar.vue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/builder/BuilderFooterToolbar.vue` around lines 45 - 57, The keydown handler registered via useEventListener currently calls onExitBuilder repeatedly while Escape is held; add a guard to ignore repeated keydown events by checking the KeyboardEvent.repeat property (e.repeat) before proceeding, i.e., in the handler for useEventListener(window, 'keydown', (e: KeyboardEvent) => { ... }) return early if e.repeat is true so that onExitBuilder is only invoked once when dialogStore.dialogStack.length === 0 and isBuilderMode.value is true.
🧹 Nitpick comments (1)
src/components/builder/BuilderFooterToolbar.test.ts (1)
80-87: Use label-driven button lookup instead of positional indexing.Index-based selection (
buttons[0..2]) is brittle; selecting by visible label keeps the test focused on user-observable behavior.Based on learnings: "In test files, prefer selecting or asserting on accessible properties (text content, aria-label, role, accessible name) over data-testid attributes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/builder/BuilderFooterToolbar.test.ts` around lines 80 - 87, Replace the brittle positional button lookup in getButtons by selecting buttons by their visible/accessible labels: instead of using wrapper.findAll('button') and indexing, locate each control via its text content or accessible name (e.g., filter/find buttons whose text() or aria-label matches "Exit", "Back", "Next") so getButtons returns { exit, back, next } based on label-driven queries; update the getButtons function to use wrapper.find/findAll with text/aria assertions (or wrapper.find('[aria-label="..."]')) to make the test resilient to DOM order changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/builder/BuilderFooterToolbar.vue`:
- Around line 45-57: The keydown handler registered via useEventListener
currently calls onExitBuilder repeatedly while Escape is held; add a guard to
ignore repeated keydown events by checking the KeyboardEvent.repeat property
(e.repeat) before proceeding, i.e., in the handler for useEventListener(window,
'keydown', (e: KeyboardEvent) => { ... }) return early if e.repeat is true so
that onExitBuilder is only invoked once when dialogStore.dialogStack.length ===
0 and isBuilderMode.value is true.
---
Nitpick comments:
In `@src/components/builder/BuilderFooterToolbar.test.ts`:
- Around line 80-87: Replace the brittle positional button lookup in getButtons
by selecting buttons by their visible/accessible labels: instead of using
wrapper.findAll('button') and indexing, locate each control via its text content
or accessible name (e.g., filter/find buttons whose text() or aria-label matches
"Exit", "Back", "Next") so getButtons returns { exit, back, next } based on
label-driven queries; update the getButtons function to use wrapper.find/findAll
with text/aria assertions (or wrapper.find('[aria-label="..."]')) to make the
test resilient to DOM order changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1d20ce0e-beb2-4711-8ca3-1db901399492
📒 Files selected for processing (4)
src/components/builder/BuilderFooterToolbar.test.tssrc/components/builder/BuilderFooterToolbar.vuesrc/components/builder/BuilderToolbar.vuesrc/components/builder/useBuilderSteps.ts
dosubot
bot
added
the
size:L
AustinMroz
approved these changes
Mar 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes it easier and more obvious for users to navigate between steps
Changes
Screenshots (if applicable)
┆Issue is synchronized with this Notion page by Unito