native macOS codings agent orchestrator
6
fork

Configure Feed

Select the types of activity you want to include in your feed.

Plan sidebar container refactor

Refs #249

onevcat 4c9b72e3 c135c92c

+508
+508
doc-onevcat/plans/2026-05-03-sidebar-container-refactor-plan.md
··· 1 + # Sidebar Container Refactor Plan 2 + 3 + Status: planning 4 + Issue: [#249](https://github.com/onevcat/Prowl/issues/249) 5 + Related: [#222](https://github.com/onevcat/Prowl/issues/222) 6 + 7 + ## Goal 8 + 9 + Refactor the repository sidebar so each repository behaves as one stable visual and drag unit, while worktrees remain selectable, reorderable, and efficient to update. 10 + 11 + This should fix the structural mismatch where the app treats repositories as reorderable units but SwiftUI `List` sees repository headers and worktree rows as separate rows. That mismatch shows up as: 12 + 13 + - incorrect repository drag insertion indicators when dragging downward across expanded repositories 14 + - unstable bulk expand/collapse animations 15 + - potential sidebar flicker during drag when live terminal / notification / ordering updates arrive 16 + 17 + ## Current Findings 18 + 19 + ### 1. Repository sections are not actual list rows 20 + 21 + `SidebarListView` renders repositories through an outer `ForEach(...).onMove`. 22 + 23 + `RepositorySectionView` then returns: 24 + 25 + ```swift 26 + Group { 27 + header 28 + .tag(SidebarSelection.repository(repository.id)) 29 + if isExpanded { 30 + WorktreeRowsView(...) 31 + } 32 + } 33 + ``` 34 + 35 + In practice, the outer data model says "repository row", but `List` receives separate rows: 36 + 37 + ```text 38 + Repository A header 39 + Repository A worktree 40 + Repository A worktree 41 + Repository B header 42 + Repository B worktree 43 + ``` 44 + 45 + This explains the observed downward-drag indicator bug: 46 + 47 + ```text 48 + Target Repo header 49 + o----------- 50 + Target Repo worktree 51 + ``` 52 + 53 + SwiftUI is placing the indicator between list rows. It does not know that the target repository header and its worktree rows should be treated as one repository-level drop zone. 54 + 55 + ### 2. `List(selection:)` is doing too much 56 + 57 + The current `List` carries several behaviors at once: 58 + 59 + - special rows: Canvas, Shelf, archived worktrees, repository list header 60 + - repository row selection for plain folders 61 + - worktree multi-selection 62 + - repository expand/collapse 63 + - native repository reorder 64 + - native worktree reorder for pinned and unpinned groups 65 + - reveal-in-sidebar via `ScrollViewReader.scrollTo` 66 + - native sidebar styling and accessibility 67 + 68 + This makes local fixes brittle because changing row structure affects selection, drag, animation, and scroll identity at the same time. 69 + 70 + ### 3. Live state still reaches rows during drag 71 + 72 + Some expensive state reads have already been isolated, such as moving repository tab-count reads into `RepoHeaderTabCountBadge`. 73 + 74 + Remaining drag-time churn sources include: 75 + 76 + - `WorktreeRowsView` animates changes to `rowIDs` 77 + - each worktree row reads terminal notification/task/run-script state 78 + - notification-driven reorder can call `withAnimation(.snappy)` and mutate `worktreeOrderByRepository` 79 + - row hover/action UI changes while a drag session is active 80 + 81 + These are not necessarily the root cause of the drop-indicator bug, but they are credible contributors to #222-style flicker. 82 + 83 + ## Recommended Direction 84 + 85 + Use a custom sidebar scroll container rather than trying to keep the current flat `List` structure. 86 + 87 + Recommended shape: 88 + 89 + ```text 90 + ScrollViewReader 91 + └── ScrollView 92 + └── LazyVStack or VStack 93 + ├── Special rows 94 + ├── RepositoryContainerRow 95 + │ ├── RepositoryHeaderRow 96 + │ └── WorktreeRows 97 + └── FailedRepositoryRow 98 + ``` 99 + 100 + Key property: repository containers are the only repository-level siblings in the outer stack. Expanded worktrees are children inside the container, not siblings beside it. 101 + 102 + This aligns UI boundaries with model boundaries: 103 + 104 + - repository reorder indicators target repository containers 105 + - expand/collapse animates inside a container 106 + - worktree reorder indicators target worktree rows inside one container 107 + - live worktree updates do not change the outer repository list shape 108 + 109 + ## Options Considered 110 + 111 + ### Option A: Keep `List`, wrap worktrees inside one repository row 112 + 113 + Pros: 114 + 115 + - preserves some native sidebar styling 116 + - repository-level `onMove` might remain mostly native 117 + 118 + Cons: 119 + 120 + - nested selectable worktree rows inside a single `List` row no longer participate naturally in `List(selection:)` 121 + - worktree-level `onMove` becomes awkward inside a row 122 + - native selection and keyboard behavior still need replacement 123 + - likely keeps a hard-to-debug mix of native and custom drag logic 124 + 125 + This option reduces the indicator bug but does not cleanly solve the broader sidebar design. 126 + 127 + ### Option B: Move fully to `ScrollView` + explicit rows 128 + 129 + Pros: 130 + 131 + - model and visual structure match 132 + - repo and worktree drag/drop can be made explicit and testable 133 + - selection, focus, and reveal behavior are owned by our code instead of `List` side effects 134 + - easier to freeze drag-time updates intentionally 135 + - eliminates `List` cell reuse as a class of expand/collapse animation bugs 136 + 137 + Cons: 138 + 139 + - must replace native `List(selection:)` 140 + - must rebuild keyboard navigation, multi-selection, reorder, and accessibility affordances 141 + - more implementation work 142 + 143 + This is the recommended route for #249 if the goal is "fix the sidebar design once" rather than patch one symptom. 144 + 145 + ### Option C: Short-term drag-time freeze only 146 + 147 + Pros: 148 + 149 + - small 150 + - may help #222 flicker 151 + 152 + Cons: 153 + 154 + - does not fix repository insertion indicator because row boundaries remain wrong 155 + - leaves the main structural mismatch in place 156 + 157 + This can be kept as a subset of Option B, but it is not enough alone. 158 + 159 + ## Proposed Architecture 160 + 161 + ### SidebarPresentationModel 162 + 163 + Introduce a pure presentation model that flattens current repository state into explicit sidebar units. 164 + 165 + Suggested model: 166 + 167 + ```swift 168 + struct SidebarPresentation: Equatable { 169 + var items: [SidebarItem] 170 + } 171 + 172 + enum SidebarItem: Equatable, Identifiable { 173 + case listHeader(SidebarListHeaderModel) 174 + case special(SidebarSpecialRowModel) 175 + case repository(SidebarRepositoryContainerModel) 176 + case failedRepository(FailedRepositoryModel) 177 + } 178 + 179 + struct SidebarRepositoryContainerModel: Equatable, Identifiable { 180 + var repositoryID: Repository.ID 181 + var title: String 182 + var rootURL: URL 183 + var kind: Repository.Kind 184 + var isExpanded: Bool 185 + var isRemoving: Bool 186 + var worktreeSections: WorktreeRowSections 187 + } 188 + ``` 189 + 190 + Rules: 191 + 192 + - outer `items` contains one item per repository, not one item per row 193 + - worktree sections remain inside the repository container 194 + - presentation construction should be pure and unit-tested 195 + - live terminal state should not be part of the broad presentation model unless required for layout identity 196 + 197 + ### Selection 198 + 199 + Replace `List(selection:)` with explicit selection handling. 200 + 201 + Keep `RepositoriesFeature.State.selection` and `sidebarSelectedWorktreeIDs` as the source of truth, but route clicks through helper functions: 202 + 203 + - repository header click: 204 + - plain folder: select repository 205 + - git repository: toggle expand by default, or select repository if a future repository-detail mode needs it 206 + - worktree row click: 207 + - normal click: select worktree and focus terminal 208 + - Cmd-click: toggle multi-selection 209 + - Shift-click: optional follow-up, only if current behavior supports it through `List` 210 + - Canvas / Shelf / Archived rows: dispatch existing actions 211 + 212 + Selection visuals should be explicit in `RepositoryHeaderRow` and `WorktreeRow`, not inherited from `List`. 213 + 214 + ### Keyboard Navigation 215 + 216 + Preserve the existing command actions first: 217 + 218 + - `selectNextWorktree` 219 + - `selectPreviousWorktree` 220 + - `revealSelectedWorktreeInSidebar` 221 + - numbered hotkeys 222 + 223 + Do not try to rebuild full Finder-like keyboard navigation in the first pass unless it is currently user-visible and relied upon. 224 + 225 + Required V1 behavior: 226 + 227 + - command shortcuts still select worktrees 228 + - selected row is scrolled into view on reveal 229 + - focus returns to terminal after single worktree selection 230 + - sidebar focus does not accidentally forward text while Canvas / Shelf rules say it should not 231 + 232 + ### Repository Reorder 233 + 234 + Replace `ForEach(...).onMove` with explicit repository drag/drop. 235 + 236 + Suggested approach: 237 + 238 + - make `RepositoryContainerRow` draggable with repository ID payload 239 + - render a custom repository insertion indicator between repository containers 240 + - compute drop destination as a repository index 241 + - dispatch existing `.worktreeOrdering(.repositoriesMoved(offsets, destination))` or a new clearer action such as `.repositoriesReordered([Repository.ID])` 242 + 243 + The custom indicator should always render at repository container boundaries: 244 + 245 + ```text 246 + Target Repo header 247 + Target Repo worktree 248 + o----------- 249 + ``` 250 + 251 + This directly fixes the current downward-drag indicator bug. 252 + 253 + ### Worktree Reorder 254 + 255 + Keep worktree reorder scoped inside one repository container. 256 + 257 + Suggested approach: 258 + 259 + - worktree rows are draggable with worktree ID payload 260 + - pinned and unpinned sections keep separate drop zones 261 + - main and pending rows remain non-movable 262 + - drop destination maps to existing reducer actions: 263 + - `.pinnedWorktreesMoved(repositoryID, offsets, destination)` 264 + - `.unpinnedWorktreesMoved(repositoryID, offsets, destination)` 265 + 266 + Cross-repository worktree drag can stay out of scope. The current model does not appear to support moving worktrees between repositories. 267 + 268 + ### Drag-Time Freeze 269 + 270 + Add a small sidebar drag state to suppress non-essential row churn. 271 + 272 + During any sidebar drag: 273 + 274 + - freeze hover-only row actions 275 + - hide pull request / notification popover affordances that resize rows 276 + - suppress row-ID animations caused by notification-driven reorder 277 + - defer "move notified worktree to top" until drag ends, or apply it without animation after drop 278 + 279 + This addresses #222 without requiring every live data read to stop. 280 + 281 + ### Expand / Collapse 282 + 283 + Move expand/collapse animation into `RepositoryContainerRow`. 284 + 285 + Rules: 286 + 287 + - outer repository container identity must not change when worktrees appear/disappear 288 + - single repo expand/collapse animates child rows inside the container 289 + - bulk expand/collapse updates many containers, but the outer stack still has stable repository items 290 + - avoid animating row identity and live status changes in the same transaction 291 + 292 + ### Reveal In Sidebar 293 + 294 + `ScrollViewReader.scrollTo` can still work, but scroll IDs must be explicit: 295 + 296 + - repository container: `SidebarScrollID.repository(repositoryID)` 297 + - worktree row: `SidebarScrollID.worktree(worktreeID)` 298 + - special rows: `SidebarScrollID.canvas`, etc. 299 + 300 + When revealing a collapsed worktree: 301 + 302 + 1. expand its repository 303 + 2. yield for layout materialization 304 + 3. scroll to `SidebarScrollID.worktree(worktreeID)` 305 + 4. consume pending reveal 306 + 307 + This matches the current two-yield approach but removes dependency on `List` row materialization. 308 + 309 + ### Accessibility 310 + 311 + Minimum accessibility requirements: 312 + 313 + - repository headers expose button/row labels and expanded state 314 + - worktree rows expose selection state 315 + - drag handles or rows expose reorder affordance where AppKit/SwiftUI can support it 316 + - Canvas / Shelf / Archived rows keep meaningful labels 317 + 318 + If full native `List` accessibility cannot be matched in V1, document the gap and keep keyboard command coverage strong. 319 + 320 + ## Implementation Plan 321 + 322 + ### Phase 0: Baseline and Guardrails 323 + 324 + - Add a short manual repro checklist for: 325 + - repository drag up/down over expanded target 326 + - bulk expand/collapse with many repositories 327 + - worktree reorder in pinned/unpinned groups 328 + - sidebar multi-selection 329 + - reveal-in-sidebar 330 + - Add signposts around sidebar presentation build and drag state transitions if trace work is needed. 331 + - Keep current `List` code untouched until presentation tests exist. 332 + 333 + ### Phase 1: Pure Presentation and Reorder Mapping 334 + 335 + Files likely involved: 336 + 337 + - `supacode/Features/Repositories/Models/SidebarPresentation.swift` (new) 338 + - `supacodeTests/SidebarPresentationTests.swift` (new) 339 + - existing reducer ordering tests 340 + 341 + Deliver: 342 + 343 + - pure sidebar presentation builder 344 + - stable scroll IDs 345 + - pure drop-destination mapping for repository and worktree reorder 346 + - tests for: 347 + - expanded repository keeps one outer item with child rows 348 + - failed repositories preserve order 349 + - plain folders produce repository containers with no worktree children 350 + - pinned/main/pending/unpinned sections are preserved 351 + - repository drop destinations map to expected order 352 + - worktree drop destinations map within pinned/unpinned sections 353 + 354 + ### Phase 2: New Container Views Behind a Switch 355 + 356 + Files likely involved: 357 + 358 + - `SidebarListView.swift` 359 + - `RepositorySectionView.swift` 360 + - `WorktreeRowsView.swift` 361 + - new `SidebarContainerListView.swift` 362 + - new `RepositoryContainerRow.swift` 363 + 364 + Deliver: 365 + 366 + - render the new container sidebar behind a local compile-time or private runtime switch 367 + - no reducer changes except new presentation helpers if needed 368 + - preserve row styling visually before enabling custom drag/drop 369 + 370 + This phase should be screenshot/manual verified before deleting the old `List` path. 371 + 372 + ### Phase 3: Explicit Selection and Reveal 373 + 374 + Deliver: 375 + 376 + - click handling for repository and worktree rows 377 + - explicit selection visuals 378 + - multi-selection behavior matching current sidebar expectations 379 + - reveal-in-sidebar via new scroll IDs 380 + - focused terminal handoff after single worktree selection 381 + 382 + Tests: 383 + 384 + - pure selection helper tests if logic is factored out 385 + - existing reducer selection tests should keep passing 386 + 387 + ### Phase 4: Custom Repository Reorder 388 + 389 + Deliver: 390 + 391 + - repository drag payload 392 + - custom repo-level insertion indicator 393 + - drop handling that dispatches repository reorder 394 + - drag-time UI freeze for non-essential row affordances 395 + 396 + Manual verification: 397 + 398 + - dragging a repository upward shows indicator below the target repository container when appropriate 399 + - dragging a repository downward never shows the indicator between target header and target worktree rows 400 + - failed repository rows either reorder correctly or are explicitly non-reorderable 401 + 402 + ### Phase 5: Custom Worktree Reorder 403 + 404 + Deliver: 405 + 406 + - pinned/unpinned scoped worktree drop zones 407 + - custom worktree insertion indicator 408 + - main/pending rows stay non-movable 409 + - existing persistence paths remain unchanged 410 + 411 + Manual verification: 412 + 413 + - pinned worktree reorder persists 414 + - unpinned worktree reorder persists 415 + - dragging over main/pending rows does not create invalid moves 416 + 417 + ### Phase 6: Remove Old `List` Path and Polish 418 + 419 + Deliver: 420 + 421 + - delete old `List(selection:)` implementation 422 + - remove obsolete `RepositorySectionView` / `WorktreeRowsView` pieces or fold them into new components 423 + - final accessibility pass 424 + - final animation pass for bulk expand/collapse 425 + - update issue #249 with final implementation notes 426 + 427 + ## Verification Matrix 428 + 429 + Automated: 430 + 431 + - `SidebarPresentationTests` 432 + - existing `RepositoriesFeatureTests` ordering tests 433 + - existing `RepositorySectionViewTests` migrated or renamed 434 + - `make check` 435 + - `make build-app` 436 + 437 + Manual: 438 + 439 + 1. Select a plain folder repository row. 440 + 2. Select a git repository worktree row and confirm terminal focus. 441 + 3. Cmd-click multiple worktree rows and confirm bulk archive/delete commands still target selected rows. 442 + 4. Expand/collapse one repository. 443 + 5. Bulk expand/collapse at least 10 repositories. 444 + 6. Drag repository upward and downward across expanded repositories. 445 + 7. Drag pinned worktrees within a repository. 446 + 8. Drag unpinned worktrees within a repository. 447 + 9. Trigger reveal-in-sidebar from Canvas or command. 448 + 10. Verify Canvas / Shelf / Archived rows remain selectable. 449 + 11. Verify notification/task/run-script indicators update without moving rows during a drag. 450 + 451 + ## Risks 452 + 453 + ### Native `List` behavior loss 454 + 455 + Risk: custom scroll rows may lose some free AppKit sidebar behavior. 456 + 457 + Mitigation: 458 + 459 + - preserve command-based navigation first 460 + - add explicit accessibility labels/traits 461 + - keep manual keyboard/accessibility checklist 462 + 463 + ### Reorder implementation complexity 464 + 465 + Risk: custom drag/drop can become more complex than native `.onMove`. 466 + 467 + Mitigation: 468 + 469 + - keep pure drop-index mapping tested 470 + - keep repository reorder and worktree reorder separate 471 + - defer cross-repository worktree moves 472 + 473 + ### UI regressions from broad rewrite 474 + 475 + Risk: replacing the sidebar in one PR touches selection, animation, and drag. 476 + 477 + Mitigation: 478 + 479 + - stage behind a private switch until visual behavior is verified 480 + - land presentation model tests first 481 + - keep reducer actions and persistence shape stable 482 + 483 + ### Performance regressions 484 + 485 + Risk: replacing lazy `List` with `VStack` could render too much. 486 + 487 + Mitigation: 488 + 489 + - start with `LazyVStack` 490 + - switch only repository containers to non-lazy child stacks if expand/collapse animation needs it 491 + - measure with the existing signpost / Instruments workflow if the sidebar feels worse 492 + 493 + ## Recommendation 494 + 495 + Proceed with Option B as the #249 plan: a custom `ScrollView` sidebar with repository containers as outer items. 496 + 497 + Do not attempt to fix the repository insertion indicator through reducer index changes. The indicator is a symptom of the current `List` row structure, not the persisted ordering logic. 498 + 499 + The safest execution path is: 500 + 501 + 1. pure presentation model and tests 502 + 2. render-only new sidebar path 503 + 3. explicit selection/reveal 504 + 4. custom repository reorder 505 + 5. custom worktree reorder 506 + 6. remove old `List` path 507 + 508 + This is larger than a tactical #222 fix, but it addresses the underlying sidebar design mismatch and gives future sidebar features a cleaner foundation.