Files
vibe-kanban/frontend
Louis Knight-Webb 18ae6c5fd6 Refactor FeatureShowcaseModal to use lib/modals (vibe-kanban) (#1308)
* Perfect! All type checks pass. The refactoring is complete.

## Summary

Successfully refactored [FeatureShowcaseModal.tsx](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/dd42-refactor-feature/frontend/src/components/showcase/FeatureShowcaseModal.tsx) to use the `lib/modals` pattern:

**Changes made:**

1. **FeatureShowcaseModal.tsx:**
   - Wrapped component with `NiceModal.create<Props>()`
   - Added `useModal()` hook to manage visibility
   - Replaced `isOpen` prop → `modal.visible`
   - Replaced `onClose()` → `modal.hide()`
   - Added `onExitComplete` to AnimatePresence for proper cleanup
   - Updated keyboard scope effect to restore scopes only on cleanup
   - Exported via `defineModal<Props, void>()`

2. **project-tasks.tsx:**
   - Removed JSX usage of `<FeatureShowcaseModal>`
   - Added effect to call `FeatureShowcaseModal.show()` when trigger fires
   - Properly handles cleanup with `.finally()` callback

All type checks pass ✓

* Cleanup script changes for task attempt dd42978a-2a1a-44bd-9e2e-aa0c6575d236

* Perfect! Refactoring complete.

## Summary

Successfully refactored FeatureShowcaseModal to use the `lib/modals` pattern:

**Files changed:**

1. **Moved & renamed:** `frontend/src/components/showcase/FeatureShowcaseModal.tsx` → [frontend/src/components/dialogs/global/FeatureShowcaseDialog.tsx](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/dd42-refactor-feature/frontend/src/components/dialogs/global/FeatureShowcaseDialog.tsx)
   - Wrapped with `NiceModal.create<Props>()`
   - Uses `useModal()` for visibility control
   - Exports via `defineModal<Props, void>()`
   - Updated keyboard scope management to restore on cleanup only

2. **Updated:** [frontend/src/pages/project-tasks.tsx](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/dd42-refactor-feature/frontend/src/pages/project-tasks.tsx)
   - Removed JSX usage
   - Calls `FeatureShowcaseDialog.show()` when trigger fires
   - Properly handles cleanup with `.finally()` callback

All type checks and linting pass ✓

* Cleanup script changes for task attempt dd42978a-2a1a-44bd-9e2e-aa0c6575d236

* Refactor FeatureShowcaseDialog to use Dialog component (vibe-kanban 8a3e4ceb)

Use the Dialog component, like all the other dialogs/modals

* Perfect! Refactoring complete.

## Summary

Successfully eliminated the state-based trigger pattern by enhancing `useShowcaseTrigger` with a direct callback approach.

**Changes made:**

1. **[useShowcaseTrigger.ts](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/dd42-refactor-feature/frontend/src/hooks/useShowcaseTrigger.ts):**
   - Added `onShow?: () => Promise<unknown>` to options
   - Stored callback in ref to avoid stale closures
   - Calls `onShow()` directly when conditions are met
   - Automatically calls `close()` when promise resolves (marks as seen)

2. **[project-tasks.tsx](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/dd42-refactor-feature/frontend/src/pages/project-tasks.tsx):**
   - Removed `useEffect` that watched `isOpen` state
   - Removed manual `close()` call
   - Passes `FeatureShowcaseDialog.show()` as `onShow` callback
   - Reduced from 13 lines to 4 lines

**Result:** Clean, declarative API that matches the lib/modals imperative pattern. No more state to trigger side effects.

All type checks pass ✓

* Perfect! The persistence tracking is now fixed.

## Summary

Successfully fixed showcase persistence tracking by resolving the stale closure issue.

**Changes to [useShowcaseTrigger.ts](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/dd42-refactor-feature/frontend/src/hooks/useShowcaseTrigger.ts):**

1. **Destructured persistence** to get stable function references:
   - `const { isLoaded, hasSeen, markSeen } = useShowcasePersistence()`
   - Avoids object identity changes causing re-renders

2. **Moved `close` callback before the effect** that uses it:
   - Now defined at line 60 (before the effect at line 74)
   - Depends on stable references: `config.id`, `markSeenOnClose`, `markSeen`

3. **Added `close` to effect dependencies**:
   - Updated dependency array: `[isLoaded, enabled, hasSeenState, openDelay, resetOnDisable, close]`
   - Effect now stays in sync with the latest `close` function

4. **Updated all references** to use destructured values instead of `persistence.*`

**Result:** When the dialog closes, `markSeen(config.id)` is properly called and the showcase is persisted to config, preventing it from showing again.

All type checks pass ✓

* Perfect! Successfully eliminated the `useShowcaseTrigger` hook.

## Summary

Removed unnecessary abstraction layer by inlining showcase logic directly where it's used.

**Changes:**

1. **[project-tasks.tsx](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/dd42-refactor-feature/frontend/src/pages/project-tasks.tsx):**
   - Replaced `useShowcaseTrigger` with direct `useShowcasePersistence` usage
   - Added inline effect that:
     - Checks if showcase has been seen
     - Waits 300ms before showing (prevents flash during mount)
     - Shows `FeatureShowcaseDialog` when panel opens for first time
     - Marks as seen when dialog closes
     - Properly cleans up timer with `cancelled` flag
   - Much clearer: logic is visible at point of use

2. **Deleted:** `frontend/src/hooks/useShowcaseTrigger.ts` (no longer needed)

**Result:**
- **Simpler:** 20 lines of clear, inline logic vs. 127-line hook file
- **More readable:** No indirection - you can see exactly what happens when
- **Same behavior:** Persistence tracking, delay, proper cleanup all preserved

All type checks pass ✓

* Cleanup script changes for task attempt dd42978a-2a1a-44bd-9e2e-aa0c6575d236

* Perfect! Successfully eliminated the `useShowcasePersistence` hook.

Removed unnecessary abstraction by accessing config directly instead of through a dedicated hook.

**Changes:**

1. **[project-tasks.tsx](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/dd42-refactor-feature/frontend/src/pages/project-tasks.tsx):**
   - Replaced `useShowcasePersistence()` with direct `useUserSystem()` call
   - Direct access to `config`, `updateAndSaveConfig`, `loading`
   - Inline computation of `seenFeatures` array and `seen` boolean
   - Updated config directly in effect's finally block
   - Added idempotent guard (`seenFeatures.includes(showcaseId)`) before saving

2. **Deleted:** `frontend/src/hooks/useShowcasePersistence.ts` (42 lines, unused)

**Result:**
- **Simpler:** No intermediate hook, direct config access
- **Clearer:** Can see exactly what config is being read/written
- **No unnecessary abstractions:** Removed `useCallback` wrappers that didn't stabilize anything
- **Same behavior:** Persistence tracking, idempotent writes, proper cleanup

All type checks pass ✓

Successfully refactored FeatureShowcaseModal to use lib/modals pattern and eliminated unnecessary abstractions:

1.  Converted FeatureShowcaseModal → FeatureShowcaseDialog with NiceModal
2.  Moved to dialogs/global/ directory
3.  Fixed keyboard scope management
4.  Eliminated state-based trigger pattern (removed useEffect watching isOpen)
5.  Removed useShowcaseTrigger hook (127 lines → inline)
6.  Removed useShowcasePersistence hook (42 lines → direct config access)

**Final state:** Clean, readable, direct implementation with no unnecessary abstractions.

* Cleanup script changes for task attempt dd42978a-2a1a-44bd-9e2e-aa0c6575d236

* Perfect! Removed the unnecessary timeout.

## Summary

Simplified the showcase effect by removing the 300ms delay.

**Change to [project-tasks.tsx](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/dd42-refactor-feature/frontend/src/pages/project-tasks.tsx):**
- Removed `setTimeout` wrapper and 300ms delay
- Removed timer cleanup from return statement
- Kept `cancelled` flag to prevent config update after unmount
- Dialog now shows immediately when conditions are met

**Result:** Simpler, more direct code - showcase appears immediately when panel opens for the first time.

All type checks pass ✓

* Cleanup script changes for task attempt dd42978a-2a1a-44bd-9e2e-aa0c6575d236

* Perfect! The showcase logic is now extremely clean and simple.

## Summary

Removed the unnecessary `cancelled` flag from the showcase effect.

**Change to [project-tasks.tsx](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/dd42-refactor-feature/frontend/src/pages/project-tasks.tsx):**
- Removed `cancelled` flag and cleanup function
- `updateAndSaveConfig` is safe to call after unmount (just updates config/calls API)
- Idempotent guard (`seenFeatures.includes(showcaseId)`) prevents duplicate saves

**Final showcase implementation (10 lines):**
```tsx
useEffect(() => {
  if (!isLoaded || !isPanelOpen || seen) return;

  FeatureShowcaseDialog.show({ config: showcases.taskPanel }).finally(() => {
    if (seenFeatures.includes(showcaseId)) return;
    void updateAndSaveConfig({
      showcases: { seen_features: [...seenFeatures, showcaseId] },
    });
  });
}, [isLoaded, isPanelOpen, seen, showcaseId, updateAndSaveConfig, seenFeatures]);
```

All type checks pass ✓

## Complete Refactoring Results

Transformed the feature showcase system from complex abstractions to simple, direct code:

-  Converted to lib/modals pattern (NiceModal)
-  Eliminated 2 unnecessary hooks (169 lines removed)
-  Removed unnecessary timeout and cancellation logic
-  Final result: **10 lines of clear, direct code** that shows dialog and saves config

* Cleanup script changes for task attempt dd42978a-2a1a-44bd-9e2e-aa0c6575d236

* hide and save properly
2025-11-18 16:26:13 +00:00
..