Fix CommandBar input and navigation issues with state machine refactor (Vibe Kanban) (#1885)
* Done! I've removed all 7 `shortcut` properties from the action definitions in `index.ts`:
- `NewWorkspace`: removed `shortcut: 'N'`
- `Settings`: removed `shortcut: ','`
- `ToggleSidebar`: removed `shortcut: '['`
- `ToggleGitPanel`: removed `shortcut: ']'`
- `ToggleChangesMode`: removed `shortcut: 'C'`
- `ToggleLogsMode`: removed `shortcut: 'L'`
- `TogglePreviewMode`: removed `shortcut: 'P'`
The `ActionDefinition` type (and its base `ActionBase` interface) were left unchanged as requested.
* fix tailwind nto auto creating styles
* I've successfully completed all the changes. Here's a summary of what was done:
## Summary of Changes
### 1. `ActionExecutorContext` (index.ts)
- Made all properties non-optional
- Removed `gitRepoId` from context (now passed as action parameter)
- Properties now use `| null` for values that may not exist
### 2. New `GitActionDefinition` Type (index.ts)
- Added new action type with `requiresTarget: 'git'`
- Git actions now explicitly require `repoId` as a parameter
### 3. Git Actions Updated (index.ts)
- `GitCreatePR`, `GitMerge`, `GitRebase`, `GitChangeTarget` now use `requiresTarget: 'git'`
- Each action's `execute` function takes `repoId` as third parameter
- Simplified implementations (no more fallback repo fetching logic)
### 4. `ActionsContext` Provider (ActionsContext.tsx)
- Now calls `useDevServer()` to get dev server state
- Provides all required context properties
- `executeAction` handles three action types: global, workspace, and git
- Removed `contextOverride` pattern
### 5. `ContextBarContainer` (ContextBarContainer.tsx)
- Simplified significantly - no longer builds extended context
- Uses executor context directly from provider
- Removed `containerPath` prop
### 6. `ActionVisibilityContext` (index.ts + useActionVisibility.ts)
- Made `editorType`, `devServerState`, `runningDevServerId` required
- Visibility hook now provides all values from hooks
### 7. Updated Call Sites
- `WorkspacesLayout`: Git actions now pass `repoId` as third parameter
- `WorkspacesMain`: Removed `containerPath` prop from ContextBarContainer
* Cleanup script changes for workspace e961e311-6db4-41bf-83a1-ef8dd96eb497
* All changes are complete and type checks pass. Here's a summary of the changes made:
## Summary of Changes
### 1. `ActionExecutorContext` - Made all properties non-optional
- Removed `?` from all properties
- Removed `gitRepoId` (moved to per-action parameter)
- Changed nullable properties to use `| null` type
### 2. New `GitActionDefinition` type
- Added new action type with `requiresTarget: 'git'`
- Takes `repoId` as explicit third parameter
### 3. Git actions updated
- `GitCreatePR`, `GitMerge`, `GitRebase`, `GitChangeTarget` now use `requiresTarget: 'git'`
- Each action's `execute` receives `repoId` as parameter
### 4. `ActionsContext` provider centralized
- Now calls `useDevServer()` to provide dev server state
- Provides all required context properties from hooks
- `executeAction` handles three action types: global, workspace, git
- Removed `contextOverride` pattern
### 5. `ActionVisibilityContext` - Made dev server properties required
- `editorType`, `devServerState`, `runningDevServerId` now required
- `useActionVisibilityContext` hook provides all values
### 6. `ContextBarContainer` simplified
- Removed extended context creation
- Uses executor context directly from provider
### 7. Provider order fix
- Moved `ExecutionProcessesProvider` up to `NewDesignScope`
- Required because `useDevServer` depends on `ExecutionProcessesContext`
### Files Modified:
- `vibe-kanban/frontend/src/components/ui-new/actions/index.ts`
- `vibe-kanban/frontend/src/components/ui-new/actions/useActionVisibility.ts`
- `vibe-kanban/frontend/src/contexts/ActionsContext.tsx`
- `vibe-kanban/frontend/src/components/ui-new/containers/ContextBarContainer.tsx`
- `vibe-kanban/frontend/src/components/ui-new/containers/WorkspacesLayout.tsx`
- `vibe-kanban/frontend/src/components/ui-new/views/WorkspacesMain.tsx`
- `vibe-kanban/frontend/src/components/ui-new/scope/NewDesignScope.tsx`
* The implementation is complete. Let me summarize what was changed:
## Summary of Changes
### `pages.ts`
- Added `'selectRepo'` to `PageId` type for dynamic repo selection page
- Created `StaticPageId` type (excludes `selectRepo`) for static page records
- Added `RepoItem` interface for repo selection items
- Extended `ResolvedGroupItem` union type to include `{ type: 'repo'; repo: RepoItem }`
- Updated `Pages` record and `getPageActions` to use `StaticPageId`
### `CommandBarDialog.tsx`
- Added `repos` from `useWorkspaceContext()`
- Added `pendingGitAction` state to track git actions awaiting repo selection
- Reset `pendingGitAction` when dialog opens/closes
- Modified `getPageWithItems` to generate dynamic `selectRepo` page with repo list
- Updated `handleSelect` to:
- Execute git actions immediately with single repo
- Navigate to `selectRepo` page when multiple repos exist
- Execute pending git action when user selects a repo
- Clear `pendingGitAction` when navigating back from `selectRepo` page
### `CommandBar.tsx`
- Added `FolderIcon` import
- Added rendering for `'repo'` item type with folder icon
## Behavior
1. **Single repo**: When user selects a git action (e.g., "Create Pull Request") and the workspace has only one git repo, the action executes immediately with that repo's ID.
2. **Multiple repos**: When workspace has multiple repos, selecting a git action navigates to a "Select Repository" page where the user can choose which repo to use, then the action executes with the selected repo.
3. **Back navigation**: Pressing back from the repo selection page clears the pending action and returns to the previous page.
* Done! The `CommandBarDialog` has been refactored to use a state machine pattern with `useReducer`. Here's a summary of the changes:
## Summary of Changes
**State Machine Types (lines 38-67):**
- `CommandBarState`: Discriminated union with `browsing` and `selectingRepo` states
- `CommandBarEvent`: `RESET`, `SEARCH_CHANGE`, `GO_BACK`, `SELECT_ITEM`
- `CommandBarEffect`: Side effects returned from reducer (`none` or `execute`)
**Reducer (lines 73-201):**
- Pure function returning `[newState, effect]` tuple
- Handles all state transitions based on current state and event
- Git action flow: single repo → execute, multiple repos → transition to `selectingRepo`
**Component Changes:**
- Replaced 4 `useState` calls with single `useReducer`
- `currentPage` is now derived from state (line 249-250)
- `handleSelect` computes effect from reducer, dispatches event, then handles effect (lines 456-475)
- `goBack` and `handleSearchChange` simply dispatch events (lines 446-453)
**Benefits:**
- All state transitions in one place (the reducer)
- State transitions are pure and testable
- Clear separation between state updates and side effects
- Impossible states are unrepresentable (e.g., can't have `pendingAction` while `browsing`)
* Done! Here's the final result:
## Summary
**Before:** 520 lines in 1 file (after initial state machine refactor)
**After:** 487 lines across 4 files
| File | Lines | Purpose |
|------|-------|---------|
| `CommandBarDialog.tsx` | 101 | Thin orchestration (down from 520) |
| `useCommandBarState.ts` | 177 | State machine hook |
| `useResolvedPage.ts` | 150 | Page building logic |
| `injectSearchMatches.ts` | 59 | DRY search helper |
**Key wins:**
- Main component: **520 → 101 lines** (80% reduction)
- Search injection code: **100 → 59 lines** (4× duplication eliminated)
- Each file has single responsibility
- State machine is testable in isolation
- Page building logic is reusable
* Cleanup script changes for workspace e961e311-6db4-41bf-83a1-ef8dd96eb497
* Done! Here are the final results:
## Final Summary
| File | Before | After | Reduction |
|------|--------|-------|-----------|
| `CommandBarDialog.tsx` | 101 | 106 | +5 (linter formatting) |
| `useCommandBarState.ts` | 177 | 95 | **-82 (46%)** |
| `useResolvedPage.ts` | 150 | 95 | **-55 (37%)** |
| `injectSearchMatches.ts` | 59 | 42 | **-17 (29%)** |
| **Total** | **487** | **338** | **-149 (31%)** |
**Original single file:** 398 lines
**Final 4 files:** 338 lines (15% less code overall)
Key wins:
- Main component is now just 106 lines of thin orchestration
- State machine is flattened (if statements instead of nested switch/case)
- Page resolution is simplified with reused types
- Search injection uses reduce() instead of for-loop
- All files pass type check
* Cleanup script changes for workspace e961e311-6db4-41bf-83a1-ef8dd96eb497
* I've fixed both issues with the CommandBar. Here's a summary of what was wrong and how I fixed it:
## Root Causes
### Issue 1: Nested `<Command>` Components
The `CommandDialog` component was wrapping children in a `<Command>` component, and `CommandBar` also rendered its own `<Command>`. This nested cmdk context caused conflicts with state management.
**Fix:** Removed the `<Command>` wrapper from `CommandDialog` in `Command.tsx:26-44`, and moved the styling classes to `CommandBar`'s `<Command>` component.
### Issue 2: Unstable `dispatch` Function Causing Infinite RESET Loop
In `useCommandBarState.ts`, the `dispatch` function had `[state, repoCount]` in its `useCallback` dependency array. This caused `dispatch` to be recreated on every state change.
In `CommandBarDialog.tsx`, the `useEffect` that resets state when the modal opens had `dispatch` in its dependency array:
```tsx
useEffect(() => {
if (modal.visible) {
dispatch({ type: 'RESET', page });
}
}, [modal.visible, page, dispatch]); // dispatch changes → useEffect runs → RESET
```
This created an infinite loop:
1. User types → state updates → `dispatch` recreated
2. `dispatch` changed → `useEffect` runs → `RESET` dispatched
3. State resets to empty → repeat
**Fix:** Made `dispatch` stable by using refs instead of state in dependencies:
```tsx
const stateRef = useRef<CommandBarState>(browsing(initialPage));
const repoCountRef = useRef(repoCount);
const dispatch = useCallback(
(event: CommandBarEvent): CommandBarEffect => {
const [, effect] = reducer(stateRef.current, event, repoCountRef.current);
rawDispatch(event);
return effect;
},
[] // No dependencies - uses refs for current values
);
```
## Files Changed
1. `vibe-kanban/frontend/src/components/ui-new/primitives/Command.tsx` - Removed nested `<Command>` wrapper from `CommandDialog`
2. `vibe-kanban/frontend/src/components/ui-new/primitives/CommandBar.tsx` - Added styling classes that were on the removed wrapper
3. `vibe-kanban/frontend/src/components/ui-new/dialogs/commandBar/useCommandBarState.ts` - Made `dispatch` stable using refs
This commit is contained in:
Louis Knight-Webb
committed by
GitHub
GitHub
parent
2c53fc976b
commit
6e3421c820
@@ -62,20 +62,13 @@ export type DevServerState = 'stopped' | 'starting' | 'running' | 'stopping';
|
||||
export interface ActionExecutorContext {
|
||||
navigate: NavigateFunction;
|
||||
queryClient: QueryClient;
|
||||
// Optional workspace selection context (for archive action)
|
||||
selectWorkspace?: (workspaceId: string) => void;
|
||||
activeWorkspaces?: SidebarWorkspace[];
|
||||
// Current workspace ID (for actions that optionally use workspace context)
|
||||
currentWorkspaceId?: string;
|
||||
|
||||
// ContextBar-specific state (optional, only set in ContextBar context)
|
||||
containerRef?: string; // For copy path (workspace.container_ref)
|
||||
runningDevServerId?: string; // For stopping dev server
|
||||
startDevServer?: () => void; // For starting dev server with mutation tracking
|
||||
stopDevServer?: () => void; // For stopping dev server with mutation tracking
|
||||
|
||||
// Git-specific state (optional, only set when clicking from a specific RepoCard)
|
||||
gitRepoId?: string;
|
||||
selectWorkspace: (workspaceId: string) => void;
|
||||
activeWorkspaces: SidebarWorkspace[];
|
||||
currentWorkspaceId: string | null;
|
||||
containerRef: string | null;
|
||||
runningDevServerId: string | null;
|
||||
startDevServer: () => void;
|
||||
stopDevServer: () => void;
|
||||
}
|
||||
|
||||
// Context for evaluating action visibility and state conditions
|
||||
@@ -98,10 +91,10 @@ export interface ActionVisibilityContext {
|
||||
diffViewMode: DiffViewMode;
|
||||
isAllDiffsExpanded: boolean;
|
||||
|
||||
// ContextBar-specific state (optional)
|
||||
editorType?: EditorType | null;
|
||||
devServerState?: DevServerState;
|
||||
runningDevServerId?: string;
|
||||
// Dev server state
|
||||
editorType: EditorType | null;
|
||||
devServerState: DevServerState;
|
||||
runningDevServerId: string | null;
|
||||
|
||||
// Git panel state
|
||||
hasGitRepos: boolean;
|
||||
@@ -144,10 +137,21 @@ export interface WorkspaceActionDefinition extends ActionBase {
|
||||
) => Promise<void> | void;
|
||||
}
|
||||
|
||||
// Git action (requires workspace + repoId)
|
||||
export interface GitActionDefinition extends ActionBase {
|
||||
requiresTarget: 'git';
|
||||
execute: (
|
||||
ctx: ActionExecutorContext,
|
||||
workspaceId: string,
|
||||
repoId: string
|
||||
) => Promise<void> | void;
|
||||
}
|
||||
|
||||
// Discriminated union
|
||||
export type ActionDefinition =
|
||||
| GlobalActionDefinition
|
||||
| WorkspaceActionDefinition;
|
||||
| WorkspaceActionDefinition
|
||||
| GitActionDefinition;
|
||||
|
||||
// Helper to get workspace from query cache
|
||||
function getWorkspaceFromCache(
|
||||
@@ -233,9 +237,9 @@ export const Actions = {
|
||||
const workspace = getWorkspaceFromCache(ctx.queryClient, workspaceId);
|
||||
const wasArchived = workspace.archived;
|
||||
|
||||
// Calculate next workspace before archiving (if we have the context)
|
||||
// Calculate next workspace before archiving
|
||||
let nextWorkspaceId: string | null = null;
|
||||
if (!wasArchived && ctx.selectWorkspace && ctx.activeWorkspaces) {
|
||||
if (!wasArchived) {
|
||||
const currentIndex = ctx.activeWorkspaces.findIndex(
|
||||
(ws) => ws.id === workspaceId
|
||||
);
|
||||
@@ -252,7 +256,7 @@ export const Actions = {
|
||||
invalidateWorkspaceQueries(ctx.queryClient, workspaceId);
|
||||
|
||||
// Select next workspace after successful archive
|
||||
if (!wasArchived && nextWorkspaceId && ctx.selectWorkspace) {
|
||||
if (!wasArchived && nextWorkspaceId) {
|
||||
ctx.selectWorkspace(nextWorkspaceId);
|
||||
}
|
||||
},
|
||||
@@ -289,7 +293,6 @@ export const Actions = {
|
||||
id: 'new-workspace',
|
||||
label: 'New Workspace',
|
||||
icon: PlusIcon,
|
||||
shortcut: 'N',
|
||||
requiresTarget: false,
|
||||
execute: (ctx) => {
|
||||
ctx.navigate('/workspaces/create');
|
||||
@@ -300,7 +303,6 @@ export const Actions = {
|
||||
id: 'settings',
|
||||
label: 'Settings',
|
||||
icon: GearIcon,
|
||||
shortcut: ',',
|
||||
requiresTarget: false,
|
||||
execute: (ctx) => {
|
||||
ctx.navigate('/settings');
|
||||
@@ -364,7 +366,6 @@ export const Actions = {
|
||||
? 'Hide Sidebar'
|
||||
: 'Show Sidebar',
|
||||
icon: SidebarSimpleIcon,
|
||||
shortcut: '[',
|
||||
requiresTarget: false,
|
||||
isActive: (ctx) => ctx.isSidebarVisible,
|
||||
execute: () => {
|
||||
@@ -394,7 +395,6 @@ export const Actions = {
|
||||
? 'Hide Git Panel'
|
||||
: 'Show Git Panel',
|
||||
icon: SidebarSimpleIcon,
|
||||
shortcut: ']',
|
||||
requiresTarget: false,
|
||||
isActive: (ctx) => ctx.isGitPanelVisible,
|
||||
execute: () => {
|
||||
@@ -409,7 +409,6 @@ export const Actions = {
|
||||
? 'Hide Changes Panel'
|
||||
: 'Show Changes Panel',
|
||||
icon: GitDiffIcon,
|
||||
shortcut: 'C',
|
||||
requiresTarget: false,
|
||||
isVisible: (ctx) => !ctx.isCreateMode,
|
||||
isActive: (ctx) => ctx.isChangesMode,
|
||||
@@ -426,7 +425,6 @@ export const Actions = {
|
||||
? 'Hide Logs Panel'
|
||||
: 'Show Logs Panel',
|
||||
icon: TerminalIcon,
|
||||
shortcut: 'L',
|
||||
requiresTarget: false,
|
||||
isVisible: (ctx) => !ctx.isCreateMode,
|
||||
isActive: (ctx) => ctx.isLogsMode,
|
||||
@@ -443,7 +441,6 @@ export const Actions = {
|
||||
? 'Hide Preview Panel'
|
||||
: 'Show Preview Panel',
|
||||
icon: DesktopIcon,
|
||||
shortcut: 'P',
|
||||
requiresTarget: false,
|
||||
isVisible: (ctx) => !ctx.isCreateMode,
|
||||
isActive: (ctx) => ctx.isPreviewMode,
|
||||
@@ -589,9 +586,9 @@ export const Actions = {
|
||||
getLabel: (ctx) =>
|
||||
ctx.devServerState === 'running' ? 'Stop Dev Server' : 'Start Dev Server',
|
||||
execute: (ctx) => {
|
||||
if (ctx.runningDevServerId && ctx.stopDevServer) {
|
||||
if (ctx.runningDevServerId) {
|
||||
ctx.stopDevServer();
|
||||
} else if (ctx.startDevServer) {
|
||||
} else {
|
||||
ctx.startDevServer();
|
||||
}
|
||||
},
|
||||
@@ -602,18 +599,9 @@ export const Actions = {
|
||||
id: 'git-create-pr',
|
||||
label: 'Create Pull Request',
|
||||
icon: GitPullRequestIcon,
|
||||
requiresTarget: true,
|
||||
requiresTarget: 'git',
|
||||
isVisible: (ctx) => ctx.hasWorkspace && ctx.hasGitRepos,
|
||||
execute: async (ctx, workspaceId) => {
|
||||
// Get repoId - either from context (RepoCard click) or fetch repos
|
||||
let repoId = ctx.gitRepoId;
|
||||
if (!repoId) {
|
||||
const repos = await attemptsApi.getRepos(workspaceId);
|
||||
if (repos.length === 0) throw new Error('No repositories found');
|
||||
if (repos.length > 1) throw new Error('Please select a repository');
|
||||
repoId = repos[0].id;
|
||||
}
|
||||
|
||||
execute: async (ctx, workspaceId, repoId) => {
|
||||
const workspace = getWorkspaceFromCache(ctx.queryClient, workspaceId);
|
||||
const task = await tasksApi.getById(workspace.task_id);
|
||||
|
||||
@@ -638,18 +626,9 @@ export const Actions = {
|
||||
id: 'git-merge',
|
||||
label: 'Merge',
|
||||
icon: GitMergeIcon,
|
||||
requiresTarget: true,
|
||||
requiresTarget: 'git',
|
||||
isVisible: (ctx) => ctx.hasWorkspace && ctx.hasGitRepos,
|
||||
execute: async (ctx, workspaceId) => {
|
||||
// Get repoId - either from context (RepoCard click) or fetch repos
|
||||
let repoId = ctx.gitRepoId;
|
||||
if (!repoId) {
|
||||
const repos = await attemptsApi.getRepos(workspaceId);
|
||||
if (repos.length === 0) throw new Error('No repositories found');
|
||||
if (repos.length > 1) throw new Error('Please select a repository');
|
||||
repoId = repos[0].id;
|
||||
}
|
||||
|
||||
execute: async (ctx, workspaceId, repoId) => {
|
||||
const confirmResult = await ConfirmDialog.show({
|
||||
title: 'Merge Branch',
|
||||
message:
|
||||
@@ -669,27 +648,17 @@ export const Actions = {
|
||||
id: 'git-rebase',
|
||||
label: 'Rebase',
|
||||
icon: ArrowsClockwiseIcon,
|
||||
requiresTarget: true,
|
||||
requiresTarget: 'git',
|
||||
isVisible: (ctx) => ctx.hasWorkspace && ctx.hasGitRepos,
|
||||
execute: async (ctx, workspaceId) => {
|
||||
// Fetch repos to get target_branch info
|
||||
execute: async (_ctx, workspaceId, repoId) => {
|
||||
const repos = await attemptsApi.getRepos(workspaceId);
|
||||
if (repos.length === 0) throw new Error('No repositories found');
|
||||
const repo = repos.find((r) => r.id === repoId);
|
||||
if (!repo) throw new Error('Repository not found');
|
||||
|
||||
// Get repo - either from context (RepoCard click) or use first repo
|
||||
let repo = ctx.gitRepoId
|
||||
? repos.find((r) => r.id === ctx.gitRepoId)
|
||||
: repos[0];
|
||||
|
||||
if (!repo) {
|
||||
if (repos.length > 1) throw new Error('Please select a repository');
|
||||
repo = repos[0];
|
||||
}
|
||||
|
||||
const branches = await repoApi.getBranches(repo.id);
|
||||
const branches = await repoApi.getBranches(repoId);
|
||||
await RebaseDialog.show({
|
||||
attemptId: workspaceId,
|
||||
repoId: repo.id,
|
||||
repoId,
|
||||
branches,
|
||||
initialTargetBranch: repo.target_branch,
|
||||
});
|
||||
@@ -700,18 +669,9 @@ export const Actions = {
|
||||
id: 'git-change-target',
|
||||
label: 'Change Target Branch',
|
||||
icon: CrosshairIcon,
|
||||
requiresTarget: true,
|
||||
requiresTarget: 'git',
|
||||
isVisible: (ctx) => ctx.hasWorkspace && ctx.hasGitRepos,
|
||||
execute: async (ctx, workspaceId) => {
|
||||
// Get repoId - either from context (RepoCard click) or fetch repos
|
||||
let repoId = ctx.gitRepoId;
|
||||
if (!repoId) {
|
||||
const repos = await attemptsApi.getRepos(workspaceId);
|
||||
if (repos.length === 0) throw new Error('No repositories found');
|
||||
if (repos.length > 1) throw new Error('Please select a repository');
|
||||
repoId = repos[0].id;
|
||||
}
|
||||
|
||||
execute: async (_ctx, workspaceId, repoId) => {
|
||||
const branches = await repoApi.getBranches(repoId);
|
||||
await ChangeTargetDialog.show({
|
||||
attemptId: workspaceId,
|
||||
|
||||
Reference in New Issue
Block a user