From b4ef8e3215870b8582f1ff411bdddeb560f4a1be Mon Sep 17 00:00:00 2001 From: Cheun Kitabayashi Date: Thu, 8 Oct 2026 19:45:23 +0200 Subject: [PATCH 1/2] feat(sidebar): list Mission children under their parent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A parent that opens review worktrees through nirux-worktree listed every child flat, in opening order. docs/sidebar-groups.md: a workspace opened as a Mission child now sits under its parent, indented, when both are open in the same project and section. The order lives in WorkspaceStore.visibleWorkspaceIndices (depth first), so the cards, the rail, the workspace strip and Cmd+Up/Down agree. A row under the parent folds its children ("▸ 3 · 1 waiting · ✕ 1 · 2 done"), saved in state.json; a folded child on screen or asking the user stays listed, as in the folded INACTIVE section. Move and drag act among siblings, children following. Approval text wraps at 26 characters so an indented card never clips it. Co-Authored-By: Claude Opus 5.5 (1M context) --- Sources/Nirux/Model/Persistence.swift | 7 +- Sources/Nirux/Model/WorkspaceState.swift | 7 + Sources/Nirux/Model/WorkspaceStore.swift | 46 ++++- .../NiruxShellView+PermissionApprovals.swift | 10 +- .../Views/NiruxShellView+Persistence.swift | 2 + .../Views/NiruxShellView+SidebarGit.swift | 4 +- .../Views/NiruxShellView+Workspaces.swift | 3 + Sources/Nirux/Views/SidebarCardLayout.swift | 7 +- .../Nirux/Views/SidebarExpandedMetrics.swift | 9 +- Sources/Nirux/Views/SidebarModels.swift | 11 + Sources/Nirux/Views/SidebarView+Drag.swift | 32 ++- Sources/Nirux/Views/SidebarView+Groups.swift | 79 ++++++++ .../Nirux/Views/SidebarView+Rendering.swift | 11 +- Sources/Nirux/Views/SidebarView.swift | 30 ++- .../PermissionApprovalSidebarTests.swift | 21 +- Tests/NiruxTests/SidebarGroupsTests.swift | 190 ++++++++++++++++++ docs/sidebar-groups.md | 71 +++++++ 17 files changed, 497 insertions(+), 43 deletions(-) create mode 100644 Sources/Nirux/Views/SidebarView+Groups.swift create mode 100644 Tests/NiruxTests/SidebarGroupsTests.swift create mode 100644 docs/sidebar-groups.md diff --git a/Sources/Nirux/Model/Persistence.swift b/Sources/Nirux/Model/Persistence.swift index b52d739e..275cc5ed 100644 --- a/Sources/Nirux/Model/Persistence.swift +++ b/Sources/Nirux/Model/Persistence.swift @@ -558,6 +558,7 @@ struct PersistedWorkspace: Codable { /// Mission owning this child workspace, when the experimental handoff /// flow created it. Optional for backward compatibility. var missionID: String? + var isGroupFolded: Bool var purpose: String? /// Optional manual phase override. Nil keeps the workspace on automatic /// phase derivation. @@ -581,6 +582,7 @@ struct PersistedWorkspace: Codable { profileID: String? = nil, isInactive: Bool = false, missionID: String? = nil, + isGroupFolded: Bool = false, purpose: String? = nil, phase: WorkspacePhase? = nil, unknownPhaseRawValue: String? = nil, @@ -599,6 +601,7 @@ struct PersistedWorkspace: Codable { self.profileID = profileID self.isInactive = isInactive self.missionID = missionID + self.isGroupFolded = isGroupFolded self.purpose = purpose self.phase = phase self.unknownPhaseRawValue = phase == nil ? unknownPhaseRawValue : nil @@ -611,7 +614,7 @@ struct PersistedWorkspace: Codable { } enum CodingKeys: String, CodingKey { - case id, title, cwd, columns, focusedColumnIndex, profileID, isInactive, missionID + case id, title, cwd, columns, focusedColumnIndex, profileID, isInactive, missionID, isGroupFolded case purpose, phase, lastSummary, lastSummaryIsManual, lastActivityAt case nextStep, blocker, reviewRuns } @@ -626,6 +629,7 @@ struct PersistedWorkspace: Codable { profileID = try container.decodeIfPresent(String.self, forKey: .profileID) isInactive = try container.decodeIfPresent(Bool.self, forKey: .isInactive) ?? false missionID = try container.decodeIfPresent(String.self, forKey: .missionID) + isGroupFolded = try container.decodeIfPresent(Bool.self, forKey: .isGroupFolded) ?? false purpose = try container.decodeIfPresent(String.self, forKey: .purpose) let phaseRawValue = try container.decodeIfPresent(String.self, forKey: .phase) phase = phaseRawValue.flatMap(WorkspacePhase.init(rawValue:)) @@ -649,6 +653,7 @@ struct PersistedWorkspace: Codable { try container.encodeIfPresent(profileID, forKey: .profileID) try container.encode(isInactive, forKey: .isInactive) try container.encodeIfPresent(missionID, forKey: .missionID) + try container.encode(isGroupFolded, forKey: .isGroupFolded) try container.encodeIfPresent(purpose, forKey: .purpose) try container.encodeIfPresent( phase?.rawValue ?? unknownPhaseRawValue, diff --git a/Sources/Nirux/Model/WorkspaceState.swift b/Sources/Nirux/Model/WorkspaceState.swift index ebb87f3d..b9fc6ea1 100644 --- a/Sources/Nirux/Model/WorkspaceState.swift +++ b/Sources/Nirux/Model/WorkspaceState.swift @@ -47,6 +47,13 @@ final class WorkspaceState { /// must not be selected or closed again. var isClosing = false let missionID: String? + /// The sidebar lists this workspace's children under a summary row + /// (docs/sidebar-groups.md). + var isGroupFolded = false + /// The child of a Mission that reported `completed`. + var isMissionCompleted: Bool { + MissionStore.shared.missions.contains { $0.id == missionID && $0.status == .completed } + } /// Controls mission variables for terminals created after a Settings /// change. Already-running shells retain the environment they launched /// with and pick up changes after a new terminal or app restart. diff --git a/Sources/Nirux/Model/WorkspaceStore.swift b/Sources/Nirux/Model/WorkspaceStore.swift index 1e19baff..52b5bcb8 100644 --- a/Sources/Nirux/Model/WorkspaceStore.swift +++ b/Sources/Nirux/Model/WorkspaceStore.swift @@ -172,9 +172,10 @@ final class WorkspaceStore { return true } - /// Move a workspace to an absolute position within its own - /// active/inactive group (0 = top of group). The position is clamped to - /// the group's bounds; a move never crosses the active/inactive boundary. + /// Move a workspace to an absolute position among its siblings (0 = + /// first), its children with it. The position is clamped to the + /// siblings' bounds; a move never crosses the active/inactive boundary + /// nor leaves its parent. @discardableResult func moveWorkspace(at index: Int, toPosition targetPosition: Int) -> Bool { guard let (candidates, position) = visibleGroupPosition(of: index) else { return false } @@ -183,12 +184,15 @@ final class WorkspaceStore { return moveWorkspace(at: index, delta: clamped - position) } - /// A workspace's same-group neighbours within the visible (active - /// profile) list, plus its position among them. + /// A workspace's siblings within the visible (active profile) list: + /// same section, same parent. Plus its position among them. private func visibleGroupPosition(of index: Int) -> (candidates: [Int], position: Int)? { guard workspaces.indices.contains(index) else { return nil } let isInactive = workspaces[index].isInactive - let candidates = visibleWorkspaceIndices.filter { workspaces[$0].isInactive == isInactive } + let parent = groupParentIndex(of: index) + let candidates = visibleWorkspaceIndices.filter { + workspaces[$0].isInactive == isInactive && groupParentIndex(of: $0) == parent + } guard let position = candidates.firstIndex(of: index) else { return nil } return (candidates, position) } @@ -283,9 +287,37 @@ final class WorkspaceStore { return true } + /// ACTIVE, then INACTIVE; each workspace followed by its children, + /// depth first (docs/sidebar-groups.md). func visibleWorkspaceIndices(in profileID: String) -> [Int] { let matching = workspaces.indices.filter { workspaces[$0].profileID == profileID } - return matching.filter { !workspaces[$0].isInactive } + matching.filter { workspaces[$0].isInactive } + let sections = matching.filter { !workspaces[$0].isInactive } + matching.filter { workspaces[$0].isInactive } + var children: [Int?: [Int]] = [:] + for index in sections { children[groupParentIndex(of: index), default: []].append(index) } + func withDescendants(_ index: Int) -> [Int] { [index] + (children[index] ?? []).flatMap(withDescendants) } + return (children[nil] ?? []).flatMap(withDescendants) + } + + /// The workspace a Mission was opened from, by Mission id. Read on each + /// call: missions load after the workspaces are restored. + var missionParentID: (String) -> String? = { missionID in + MissionStore.shared.missions.first { $0.id == missionID }?.parentWorkspaceID + } + + /// The workspace this one is listed under: its Mission's parent, when + /// that one is open in the same project and section. + func groupParentIndex(of index: Int) -> Int? { + let workspace = workspaces[index] + guard let parentID = workspace.missionID.flatMap(missionParentID) else { return nil } + return workspaces.firstIndex { + $0.id == parentID && $0.profileID == workspace.profileID && $0.isInactive == workspace.isInactive + } + } + + /// Under a folded workspace, at any depth. + func isInFoldedGroup(_ index: Int) -> Bool { + guard let parent = groupParentIndex(of: index) else { return false } + return workspaces[parent].isGroupFolded || isInFoldedGroup(parent) } private func reconcileSelection(preferActiveProfile: Bool) { diff --git a/Sources/Nirux/Views/NiruxShellView+PermissionApprovals.swift b/Sources/Nirux/Views/NiruxShellView+PermissionApprovals.swift index 2370a5e9..55db87c0 100644 --- a/Sources/Nirux/Views/NiruxShellView+PermissionApprovals.swift +++ b/Sources/Nirux/Views/NiruxShellView+PermissionApprovals.swift @@ -51,15 +51,19 @@ extension NiruxShellView { /// drawn with the buttons, for a column the user is not looking at /// (its terminal dialog answers). Not in a collapsed sidebar, another /// space (`listed` holds the listed workspaces), a workspace in the - /// folded inactive section (listed there when it waits, without its - /// buttons), or under VoiceOver, which the buttons don't serve. + /// folded inactive section or a folded group (listed there when it + /// waits, without its buttons), or under VoiceOver, which the buttons + /// don't serve. func approvalHold(workspaceIndex: Int, columnIndex: Int, listed: Set) -> PermissionApprovalHold { guard workspaces.indices.contains(workspaceIndex) else { return .never } let workspace = workspaces[workspaceIndex] let isOnScreen = columnIndex == workspace.focusedIndex && workspaceIndex == activeWSIndex let cardShown = isSidebarExpanded && !isOnScreen && listed.contains(workspaceIndex) - && sidebar.listsWorkspace(isInactive: workspace.isInactive, isActive: workspaceIndex == activeWSIndex) + && sidebar.listsWorkspace( + isInactive: workspace.isInactive, isInFoldedGroup: workspaceStore.isInFoldedGroup(workspaceIndex), + isActive: workspaceIndex == activeWSIndex + ) && !NSWorkspace.shared.isVoiceOverEnabled let userSeesSidebar = NSApp.isActive && window?.isVisible == true && window?.isMiniaturized == false && window?.occlusionState.contains(.visible) == true diff --git a/Sources/Nirux/Views/NiruxShellView+Persistence.swift b/Sources/Nirux/Views/NiruxShellView+Persistence.swift index 49208630..c33e1ac5 100644 --- a/Sources/Nirux/Views/NiruxShellView+Persistence.swift +++ b/Sources/Nirux/Views/NiruxShellView+Persistence.swift @@ -44,6 +44,7 @@ extension NiruxShellView { missionHandoffsEnabled: state.settings?.missionHandoffsEnabled == true ) workspace.isInactive = persistedWS.isInactive + workspace.isGroupFolded = persistedWS.isGroupFolded workspace.purpose = persistedWS.purpose workspace.phase = persistedWS.phase workspace.unknownPhaseRawValue = persistedWS.unknownPhaseRawValue @@ -269,6 +270,7 @@ extension NiruxShellView { profileID: workspace.profileID, isInactive: workspace.isInactive, missionID: workspace.missionID, + isGroupFolded: workspace.isGroupFolded, purpose: workspace.purpose, phase: workspace.phase, unknownPhaseRawValue: workspace.unknownPhaseRawValue, diff --git a/Sources/Nirux/Views/NiruxShellView+SidebarGit.swift b/Sources/Nirux/Views/NiruxShellView+SidebarGit.swift index a73fe5aa..5c984a2b 100644 --- a/Sources/Nirux/Views/NiruxShellView+SidebarGit.swift +++ b/Sources/Nirux/Views/NiruxShellView+SidebarGit.swift @@ -60,7 +60,9 @@ extension NiruxShellView { blocker: workspace.blocker, phase: workspace.effectivePhase, lastSummary: workspace.lastSummary, lastActivityAt: workspace.lastActivityAt, reviewBadges: workspace.reviewBadges, - mergedCleanup: mergedCleanupOffer(workspaceIndex: index)) + mergedCleanup: mergedCleanupOffer(workspaceIndex: index), + groupParentID: workspaceStore.groupParentIndex(of: index).map { workspaces[$0].id }, isGroupFolded: workspace.isGroupFolded, + isInFoldedGroup: workspaceStore.isInFoldedGroup(index), isMissionCompleted: workspace.isMissionCompleted) } tickHiddenSpaceAgents(visibleIndices: visibleIndices, foregroundProcesses: foregroundProcesses) updateColumnHeaders(infos: infos, foregroundProcesses: foregroundProcesses, now: now) diff --git a/Sources/Nirux/Views/NiruxShellView+Workspaces.swift b/Sources/Nirux/Views/NiruxShellView+Workspaces.swift index b9ebe6f9..1796a803 100644 --- a/Sources/Nirux/Views/NiruxShellView+Workspaces.swift +++ b/Sources/Nirux/Views/NiruxShellView+Workspaces.swift @@ -308,6 +308,9 @@ extension NiruxShellView { if didChange { workspaceStore.selectWorkspace(at: workspaceIndex) } case .markInactive: didChange = workspaceStore.setWorkspaceInactive(at: workspaceIndex, true) + case .toggleGroup: + workspaces[workspaceIndex].isGroupFolded.toggle() + didChange = true case .close: requestCloseWorkspace(at: workspaceIndex) return diff --git a/Sources/Nirux/Views/SidebarCardLayout.swift b/Sources/Nirux/Views/SidebarCardLayout.swift index a641623d..e513e100 100644 --- a/Sources/Nirux/Views/SidebarCardLayout.swift +++ b/Sources/Nirux/Views/SidebarCardLayout.swift @@ -168,6 +168,7 @@ struct SidebarCardLayout { actions = isCompact ? [] : workspace.cardActions let rowWidth = sidebarWidth - (Metrics.workspaceInsetX + Metrics.cardPaddingX) * 2 - Metrics.cardIndent + - (workspace.groupParentID == nil ? 0 : Metrics.groupIndent) let linksWidth = links.reduce(CGFloat(0)) { $0 + $1.width } + Self.linkGaps(links) let firstRowWidth = rowWidth - (links.isEmpty ? 0 : linksWidth + Theme.Space.sm) var rows: [[ChipPlacement]] = [] @@ -202,8 +203,10 @@ struct SidebarCardLayout { } } - var cardX: CGFloat { Metrics.workspaceInsetX } - var cardWidth: CGFloat { sidebarWidth - Metrics.workspaceInsetX * 2 } + /// A child card sits right of its parent's. + private var groupIndent: CGFloat { workspace.groupParentID == nil ? 0 : Metrics.groupIndent } + var cardX: CGFloat { Metrics.workspaceInsetX + groupIndent } + var cardWidth: CGFloat { sidebarWidth - Metrics.workspaceInsetX * 2 - groupIndent } var contentX: CGFloat { cardX + Metrics.cardPaddingX } var contentMaxX: CGFloat { cardX + cardWidth - Metrics.cardPaddingX } var indentX: CGFloat { contentX + Metrics.cardIndent } diff --git a/Sources/Nirux/Views/SidebarExpandedMetrics.swift b/Sources/Nirux/Views/SidebarExpandedMetrics.swift index 9a07da53..ba8155c7 100644 --- a/Sources/Nirux/Views/SidebarExpandedMetrics.swift +++ b/Sources/Nirux/Views/SidebarExpandedMetrics.swift @@ -41,6 +41,11 @@ enum SidebarExpandedMetrics { // An INACTIVE workspace that asks nothing: one line. static let compactRowHeight: CGFloat = 26 + // A workspace's children (docs/sidebar-groups.md): indented, under its + // summary row. + static let groupIndent = Theme.Space.md + static let groupToggleHeight: CGFloat = 18 + // The action block: rows separated by `actionRowGap`; a line above it // unless it only says something (the next step, the review badges). static let actionBlockGap: CGFloat = 8 @@ -55,9 +60,9 @@ enum SidebarExpandedMetrics { // font, hard-wrapped at a fixed column so the lines are exactly the // text, cut nowhere. static var approvalFont: NSFont { Theme.Font.code } - static let approvalCharactersPerLine = 28 + static let approvalCharactersPerLine = 26 static let approvalLineHeight: CGFloat = 16 - /// 28 characters of `approvalFont` take 208 of the box's 212. + /// 26 characters of `approvalFont` take 193 of a child card's box, 200. static let approvalBoxPaddingX: CGFloat = 6 static let approvalBoxPaddingY: CGFloat = 6 diff --git a/Sources/Nirux/Views/SidebarModels.swift b/Sources/Nirux/Views/SidebarModels.swift index 3f133290..5eb382cc 100644 --- a/Sources/Nirux/Views/SidebarModels.swift +++ b/Sources/Nirux/Views/SidebarModels.swift @@ -238,6 +238,15 @@ struct WorkspaceInfo: Hashable { var reviewBadges: ReviewBadges? = nil /// What the card shows after "#N merged"; nil shows nothing. var mergedCleanup: MergedCleanupOffer? + /// The workspace this card is listed under, indented + /// (docs/sidebar-groups.md). + var groupParentID: String? + /// Its children are folded under its summary row. + var isGroupFolded = false + /// Under a folded workspace, at any depth. + var isInFoldedGroup = false + /// The child of a Mission that reported `completed`. + var isMissionCompleted = false var sidebarAction: (text: String, isBlocker: Bool)? { if let blocker = normalizedContextText(blocker) { @@ -325,6 +334,8 @@ enum WorkspaceSidebarAction { case closeColumn(columnIndex: Int) case cleanUpWorktree case askWhyCIFailed, rerunFailedCI + /// Folds or unfolds the workspace's children. + case toggleGroup } /// Hover highlight target in the sidebar. Links have their own dedicated diff --git a/Sources/Nirux/Views/SidebarView+Drag.swift b/Sources/Nirux/Views/SidebarView+Drag.swift index d1d2c62e..91a584cb 100644 --- a/Sources/Nirux/Views/SidebarView+Drag.swift +++ b/Sources/Nirux/Views/SidebarView+Drag.swift @@ -7,7 +7,7 @@ struct SidebarWorkspaceDrag { let workspaceID: String // stable identity — store indices can shift mid-drag let title: String // shown on the floating ghost card (a rail ghost shows the tile) let rowFrame: NSRect // dragged card's (or rail tile's) hit area - let groupRowFrames: [NSRect] // rows in the same active/inactive group, top → bottom + let groupRowFrames: [NSRect] // its siblings' rows with their children, top → bottom let position: Int // dragged card's position within groupRowFrames let startPoint: NSPoint var isDragging = false // movement exceeded the click threshold @@ -118,13 +118,10 @@ extension SidebarView { private func makeWorkspaceDrag(workspaceIndex: Int, rowFrame: NSRect, startPoint: NSPoint) -> SidebarWorkspaceDrag? { guard let dragged = lastInfos.first(where: { $0.index == workspaceIndex }) else { return nil } - let group = displayedWorkspaceInfos.filter { $0.isInactive == dragged.isInactive } - let frames = group.compactMap { info in - hitAreas.first { area in - if case .workspace(let index) = area.region { return index == info.index } - return false - }?.frame + let group = displayedWorkspaceInfos.filter { + $0.isInactive == dragged.isInactive && $0.groupParentID == dragged.groupParentID } + let frames = group.compactMap(blockFrame) guard frames.count == group.count, let position = group.firstIndex(where: { $0.index == workspaceIndex }) else { return nil } @@ -138,6 +135,27 @@ extension SidebarView { ) } + /// A workspace's card, or tile, with its listed children and its + /// summary row: they move together, one drop target. + private func blockFrame(of workspace: WorkspaceInfo) -> NSRect? { + let indices = Set([workspace.index] + groupMembers(of: workspace).map(\.index)) + let toggles = Set(indices.map { Self.groupToggleActionURL(workspaceIndex: $0) }) + var card: NSRect? + var block = NSRect.null + for area in hitAreas { + switch area.region { + case .workspace(let index) where indices.contains(index): + if index == workspace.index { card = area.frame } + block = block.union(area.frame) + case .link(let url, _) where toggles.contains(url): + block = block.union(area.frame) + default: + continue + } + } + return card.map { _ in block } + } + private func dragMoved(_ event: NSEvent, drag: inout SidebarWorkspaceDrag) { let point = contentDocumentView.convert(event.locationInWindow, from: nil) if !drag.isDragging { diff --git a/Sources/Nirux/Views/SidebarView+Groups.swift b/Sources/Nirux/Views/SidebarView+Groups.swift new file mode 100644 index 00000000..01ffdfb9 --- /dev/null +++ b/Sources/Nirux/Views/SidebarView+Groups.swift @@ -0,0 +1,79 @@ +import AppKit + +// MARK: - Groups (docs/sidebar-groups.md) + +extension SidebarView { + /// Some workspace is listed under it. + func hasGroup(_ workspace: WorkspaceInfo) -> Bool { + lastInfos.contains { $0.groupParentID == workspace.id } + } + + /// Its children, each followed by its own, listed or folded. + func groupMembers(of workspace: WorkspaceInfo) -> [WorkspaceInfo] { + lastInfos.filter { $0.groupParentID == workspace.id }.flatMap { [$0] + groupMembers(of: $0) } + } + + /// The summary rows under the workspaces of `infos` that have children. + func groupTogglesHeight(_ infos: [WorkspaceInfo]) -> CGFloat { + CGFloat(infos.filter(hasGroup).count) * (SidebarExpandedMetrics.groupToggleHeight + SidebarExpandedMetrics.workspaceGap) + } + + /// The row under a workspace with children, where its children start: + /// it folds and unfolds them. + func buildGroupToggle(_ parent: WorkspaceInfo, yOffset: CGFloat) -> CGFloat { + let metrics = SidebarExpandedMetrics.self + let rowX = metrics.workspaceInsetX + metrics.groupIndent + let rowFrame = NSRect( + x: rowX, y: yOffset - metrics.groupToggleHeight, + width: bounds.width - metrics.workspaceInsetX - rowX, height: metrics.groupToggleHeight + ) + let textX = rowX + metrics.cardPaddingX + let label = NSTextField.sidebarLine(Self.groupSummary(members: groupMembers(of: parent), isFolded: parent.isGroupFolded)) + label.frame = NSRect(x: textX, y: rowFrame.minY + 1, width: rowFrame.maxX - metrics.cardPaddingX - textX, height: 16) + label.setAccessibilityElement(false) + addSubviewDoc(label) + expandedViews.append(label) + hitAreas.append(SidebarHitArea( + frame: rowFrame, region: .link(url: Self.groupToggleActionURL(workspaceIndex: parent.index), label: label) + )) + let toggle = SidebarSectionToggleView(frame: rowFrame) + toggle.toolTip = "\(parent.isGroupFolded ? "Show" : "Hide") the workspaces opened from \(parent.title)" + toggle.setAccessibilityLabel("Workspaces opened from \(parent.title): \(label.stringValue.dropFirst(2))") + toggle.setAccessibilityExpanded(!parent.isGroupFolded) + let index = parent.index + toggle.onPress = { [weak self] in self?.onWorkspaceAction?(.toggleGroup, index) } + addSubviewDoc(toggle) + expandedViews.append(toggle) + return rowFrame.minY + } + + /// "▾ 3 workspaces"; folded, where they stand, most pressing first: + /// "▸ 3 · 1 waiting · ✕ 1 · 2 done · 1 working", each count in its + /// state's color. ✕ counts open pull requests with red checks; done, + /// children whose Mission completed or whose pull request merged. + static func groupSummary(members: [WorkspaceInfo], isFolded: Bool) -> NSAttributedString { + let font = Theme.Font.caption + func count(_ isCounted: (WorkspaceInfo) -> Bool) -> Int { isFolded ? members.filter(isCounted).count : 0 } + let allCounts: [(word: String, count: Int, color: NSColor)] = [ + ("waiting", count { $0.cardState == .waiting }, Theme.Color.waiting), + ("✕", count { $0.prInfo?.state == "OPEN" && $0.prInfo?.ciStatus == "FAILURE" }, Theme.Color.error), + ("done", count { $0.isMissionCompleted || $0.prInfo?.state == "MERGED" }, Theme.Color.done), + ("working", count { $0.cardState == .working }, Theme.Color.working) + ] + let counts = allCounts.filter { $0.count > 0 } + // The counts say what they are: the noun would push them out. + let total = counts.isEmpty ? workspaceCountText(members.count) : "\(members.count)" + let text = NSMutableAttributedString( + string: "\(isFolded ? "▸" : "▾") \(total)", + attributes: [.font: font, .foregroundColor: Theme.Color.textTertiary] + ) + for (word, count, color) in counts { + text.append(NSAttributedString(string: " · ", attributes: [.font: font, .foregroundColor: Theme.Color.textTertiary])) + text.append(NSAttributedString( + string: word == "✕" ? "✕ \(count)" : "\(count) \(word)", + attributes: [.font: font, .foregroundColor: color] + )) + } + return text + } +} diff --git a/Sources/Nirux/Views/SidebarView+Rendering.swift b/Sources/Nirux/Views/SidebarView+Rendering.swift index 9b941f79..48c2b84b 100644 --- a/Sources/Nirux/Views/SidebarView+Rendering.swift +++ b/Sources/Nirux/Views/SidebarView+Rendering.swift @@ -73,6 +73,7 @@ extension SidebarView { let padding = SidebarExpandedMetrics.padding let activeInfos = displayedWorkspaceInfos.filter { !$0.isInactive } let inactiveInfos = displayedWorkspaceInfos.filter { $0.isInactive } + let listedActiveInfos = activeInfos.filter(listsWorkspace) let listedInactiveInfos = inactiveInfos.filter(listsWorkspace) let hasWorkspaces = !activeInfos.isEmpty || !inactiveInfos.isEmpty let activeProfile = lastProfiles.first(where: { $0.isActive }) @@ -99,7 +100,7 @@ extension SidebarView { if !activeInfos.isEmpty { yOffset = buildSectionHeader("active", count: activeInfos.count, padding: padding, yOffset: yOffset) - yOffset = buildWorkspaceGroup(activeInfos, padding: padding, yOffset: yOffset) + yOffset = buildWorkspaceGroup(listedActiveInfos, padding: padding, yOffset: yOffset) } if !inactiveInfos.isEmpty { yOffset -= SidebarExpandedMetrics.sectionGap @@ -165,18 +166,21 @@ extension SidebarView { activeInfos: [WorkspaceInfo], inactiveInfos: [WorkspaceInfo], listedInactiveInfos: [WorkspaceInfo], activityCount: Int, onboardingHeight: CGFloat? ) -> CGFloat { + let listedActiveInfos = activeInfos.filter(listsWorkspace) var height = SidebarExpandedMetrics.verticalPadding + SidebarExpandedMetrics.spaceHeaderHeight + SidebarExpandedMetrics.spaceHeaderBottomGap + SidebarExpandedMetrics.bottomReserve if !activeInfos.isEmpty { height += SidebarExpandedMetrics.sectionHeaderAdvance - height += SidebarExpandedMetrics.groupHeight(for: activeInfos, sidebarWidth: bounds.width) + height += SidebarExpandedMetrics.groupHeight(for: listedActiveInfos, sidebarWidth: bounds.width) + + groupTogglesHeight(listedActiveInfos) } if !inactiveInfos.isEmpty { height += SidebarExpandedMetrics.sectionGap height += SidebarExpandedMetrics.sectionHeaderAdvance height += SidebarExpandedMetrics.groupHeight(for: listedInactiveInfos, sidebarWidth: bounds.width) + + groupTogglesHeight(listedInactiveInfos) } if let onboardingHeight { height += SidebarExpandedMetrics.onboardingCardGap + onboardingHeight @@ -441,6 +445,9 @@ extension SidebarView { for workspace in infos { currentY = buildWorkspaceSection(workspace: workspace, padding: padding, yOffset: currentY) currentY -= SidebarExpandedMetrics.workspaceGap + if hasGroup(workspace) { + currentY = buildGroupToggle(workspace, yOffset: currentY) - SidebarExpandedMetrics.workspaceGap + } } return currentY } diff --git a/Sources/Nirux/Views/SidebarView.swift b/Sources/Nirux/Views/SidebarView.swift index 5f357298..affbefec 100644 --- a/Sources/Nirux/Views/SidebarView.swift +++ b/Sources/Nirux/Views/SidebarView.swift @@ -293,7 +293,10 @@ final class SidebarView: NSView { /// and, in the folded section, an amber one (`railState`). var railWorkspaceInfos: [WorkspaceInfo] { displayedWorkspaceInfos.filter { - listsWorkspace(isInactive: $0.isInactive, isActive: $0.isActive, asksUser: $0.asksUser || $0.railState == .waiting) + listsWorkspace( + isInactive: $0.isInactive, isInFoldedGroup: $0.isInFoldedGroup, isActive: $0.isActive, + asksUser: $0.asksUser || $0.railState == .waiting + ) } } @@ -438,6 +441,8 @@ final class SidebarView: NSView { onDiffStatsClicked?(workspaceIndex) } else if let workspaceIndex = Self.actionWorkspaceIndex(url, prefix: Self.cleanupActionPrefix) { onWorkspaceAction?(.cleanUpWorktree, workspaceIndex) + } else if let workspaceIndex = Self.actionWorkspaceIndex(url, prefix: Self.groupToggleActionPrefix) { + onWorkspaceAction?(.toggleGroup, workspaceIndex) } else if let workspaceID = Self.prFeedbackActionWorkspaceID(url) { prFeedbackMenu?(workspaceID)?.popUp(positioning: nil, at: convert(event.locationInWindow, from: nil), in: self) } else if let (workspaceIndex, url) = Self.openActionTarget(url), @@ -624,6 +629,7 @@ final class SidebarView: NSView { private static let diffActionPrefix = "action:diff:" private static let cleanupActionPrefix = "action:cleanup:" + private static let groupToggleActionPrefix = "action:group-toggle:" static func diffActionURL(workspaceIndex: Int) -> String { diffActionPrefix + String(workspaceIndex) @@ -634,6 +640,11 @@ final class SidebarView: NSView { cleanupActionPrefix + String(workspaceIndex) } + /// The summary row under a workspace with children. + static func groupToggleActionURL(workspaceIndex: Int) -> String { + groupToggleActionPrefix + String(workspaceIndex) + } + static func prFeedbackActionURL(workspaceID: String) -> String { "action:pr-feedback:\(workspaceID)" } @@ -668,16 +679,19 @@ final class SidebarView: NSView { // MARK: - Inactive section extension SidebarView { - /// Whether the sidebar lists a workspace. The folded section still - /// lists the inactive workspace on screen, alone, until the user moves - /// to another one, and those whose agent waits on the user or broke - /// (`asksUser`) — the section itself stays folded. - func listsWorkspace(isInactive: Bool, isActive: Bool, asksUser: Bool = false) -> Bool { - !isInactive || isActive || asksUser || !isInactiveSectionCollapsed + /// Whether the sidebar lists a workspace. The folded section, or a + /// folded group, still lists the workspace on screen, alone, until the + /// user moves to another one, and those whose agent waits on the user + /// or broke (`asksUser`) — the section itself stays folded. + func listsWorkspace(isInactive: Bool, isInFoldedGroup: Bool = false, isActive: Bool, asksUser: Bool = false) -> Bool { + isActive || asksUser || ((!isInactive || !isInactiveSectionCollapsed) && !isInFoldedGroup) } func listsWorkspace(_ workspace: WorkspaceInfo) -> Bool { - listsWorkspace(isInactive: workspace.isInactive, isActive: workspace.isActive, asksUser: workspace.asksUser) + listsWorkspace( + isInactive: workspace.isInactive, isInFoldedGroup: workspace.isInFoldedGroup, + isActive: workspace.isActive, asksUser: workspace.asksUser + ) } var hasInactiveWorkspaces: Bool { lastInfos.contains(where: \.isInactive) } diff --git a/Tests/NiruxTests/PermissionApprovalSidebarTests.swift b/Tests/NiruxTests/PermissionApprovalSidebarTests.swift index 060ba84e..0e4a207f 100644 --- a/Tests/NiruxTests/PermissionApprovalSidebarTests.swift +++ b/Tests/NiruxTests/PermissionApprovalSidebarTests.swift @@ -241,13 +241,13 @@ final class PermissionApprovalSidebarTests: XCTestCase { XCTAssertEqual(SidebarExpandedMetrics.approvalLines("ls"), ["ls"]) } - /// Every character of a full line fits the request's box: nothing is - /// clipped. + /// Every character of a full line fits the request's box, on a child + /// card too: nothing is clipped. func testFullLineFitsTheExpandedSidebar() { let metrics = SidebarExpandedMetrics.self let line = String(repeating: "W", count: metrics.approvalCharactersPerLine) let width = (line as NSString).size(withAttributes: [.font: metrics.approvalFont]).width - let card = 260 - (metrics.workspaceInsetX + metrics.cardPaddingX) * 2 + let card = 260 - (metrics.workspaceInsetX + metrics.cardPaddingX) * 2 - metrics.groupIndent XCTAssertLessThanOrEqual(width, card - metrics.approvalBoxPaddingX * 2) } @@ -355,22 +355,23 @@ final class PermissionApprovalSidebarTests: XCTestCase { XCTAssertTrue(decisionRegions(result.hitAreas).isEmpty) } - /// A line break never hides a space: `rm -rf ./dist/assets/old-js *` - /// must not read as `old-js*`. + /// A line break never hides a space: `rm -rf ./build/assets/old *` + /// must not read as `old*`. func testSpacesAtLineBreaksStayVisible() { - let text = "rm -rf ./dist/assets/old-js *" + let text = "rm -rf ./build/assets/old *" let lines = SidebarExpandedMetrics.approvalLines(text) - XCTAssertEqual(lines, ["rm -rf ./dist/assets/old-js ", "*"]) + XCTAssertEqual(lines, ["rm -rf ./build/assets/old ", "*"]) let shown = SidebarApprovalBlockRenderer.attributedLines(lines).string - XCTAssertEqual(shown, "rm -rf ./dist/assets/old-js\u{2423}\n*") + XCTAssertEqual(shown, "rm -rf ./build/assets/old\u{2423}\n*") for line in shown.split(separator: "\n") { XCTAssertFalse(line.hasPrefix(" ") || line.hasSuffix(" "), String(line)) } let inner = SidebarApprovalBlockRenderer.attributedLines(["a b"]).string XCTAssertEqual(inner, "a b", "spaces inside a line stay plain") + let full = "x" + String(repeating: "y", count: SidebarExpandedMetrics.approvalCharactersPerLine - 2) XCTAssertEqual( - SidebarApprovalBlockRenderer.attributedLines(SidebarExpandedMetrics.approvalLines("x" + String(repeating: "y", count: 26) + " z")).string, - "x" + String(repeating: "y", count: 26) + "\u{2423}\nz" + SidebarApprovalBlockRenderer.attributedLines(SidebarExpandedMetrics.approvalLines(full + " z")).string, + full + "\u{2423}\nz" ) } diff --git a/Tests/NiruxTests/SidebarGroupsTests.swift b/Tests/NiruxTests/SidebarGroupsTests.swift new file mode 100644 index 00000000..c7de18e5 --- /dev/null +++ b/Tests/NiruxTests/SidebarGroupsTests.swift @@ -0,0 +1,190 @@ +import XCTest +@testable import Nirux + +/// docs/sidebar-groups.md: a Mission child listed under its parent. +@MainActor +final class SidebarGroupsTests: XCTestCase { + + /// `parents` maps a child's id to its parent's: the child gets a + /// Mission whose parent is that workspace. + private func makeStore( + _ ids: [String], parents: [String: String] = [:], inactive: Set = [] + ) -> WorkspaceStore { + let store = WorkspaceStore() + store.missionParentID = { missionID in parents[String(missionID.dropFirst("mission-".count))] } + for id in ids { + let workspace = WorkspaceState( + id: id, title: id, cwd: "/tmp/\(id)", missionID: parents[id] == nil ? nil : "mission-\(id)" + ) + workspace.isInactive = inactive.contains(id) + store.appendWorkspace(workspace, activate: false) + } + return store + } + + private func visibleIDs(_ store: WorkspaceStore) -> [String] { + store.visibleWorkspaceIndices.map { store.workspaces[$0].id } + } + + private func index(_ id: String, in store: WorkspaceStore) -> Int { + store.workspaces.firstIndex { $0.id == id }! + } + + // MARK: - Order + + func testChildrenFollowTheirParentDepthFirst() { + let store = makeStore(["a", "b", "a1", "b1", "a2", "a1x"], parents: ["a1": "a", "a2": "a", "b1": "b", "a1x": "a1"]) + XCTAssertEqual(visibleIDs(store), ["a", "a1", "a1x", "a2", "b", "b1"]) + XCTAssertEqual(store.groupParentIndex(of: index("a1x", in: store)), index("a1", in: store)) + XCTAssertNil(store.groupParentIndex(of: index("a", in: store))) + } + + func testChildInAnotherSectionThanItsParentIsTopLevel() { + let store = makeStore(["a", "a1", "z"], parents: ["a1": "a"], inactive: ["a1", "z"]) + XCTAssertEqual(visibleIDs(store), ["a", "a1", "z"]) + XCTAssertNil(store.groupParentIndex(of: index("a1", in: store))) + } + + func testChildOfAClosedOrElsewhereParentIsTopLevel() { + let store = makeStore(["b", "a1", "c1"], parents: ["a1": "gone", "c1": "c"]) + let other = WorkspaceProfile(id: "other", name: "other", colorHex: "#FF0000") + store.replaceProfiles([WorkspaceProfile.defaultProfile, other], activeProfileID: WorkspaceProfile.defaultID) + let parent = WorkspaceState(id: "c", title: "c", cwd: "/tmp/c") + parent.profileID = "other" + store.appendWorkspace(parent, activate: false) + XCTAssertEqual(visibleIDs(store), ["b", "a1", "c1"]) + XCTAssertNil(store.groupParentIndex(of: index("a1", in: store))) + XCTAssertNil(store.groupParentIndex(of: index("c1", in: store))) + } + + // MARK: - Folding + + func testFoldedParentFoldsEveryDescendant() { + let store = makeStore(["a", "a1", "a1x", "b"], parents: ["a1": "a", "a1x": "a1"]) + store.workspaces[index("a", in: store)].isGroupFolded = true + XCTAssertFalse(store.isInFoldedGroup(index("a", in: store))) + XCTAssertTrue(store.isInFoldedGroup(index("a1", in: store))) + XCTAssertTrue(store.isInFoldedGroup(index("a1x", in: store))) + XCTAssertFalse(store.isInFoldedGroup(index("b", in: store))) + } + + // MARK: - Moves + + func testTopLevelMoveCarriesItsChildren() { + let store = makeStore(["a", "a1", "b", "c"], parents: ["a1": "a"]) + XCTAssertTrue(store.moveWorkspace(at: index("a", in: store), delta: 1)) + XCTAssertEqual(visibleIDs(store), ["b", "a", "a1", "c"]) + XCTAssertTrue(store.moveWorkspace(at: index("a", in: store), toPosition: 2)) + XCTAssertEqual(visibleIDs(store), ["b", "c", "a", "a1"]) + } + + func testChildMovesAmongItsSiblingsOnly() { + let store = makeStore(["a", "a1", "a2", "b"], parents: ["a1": "a", "a2": "a"]) + XCTAssertFalse(store.moveWorkspace(at: index("a1", in: store), delta: -1)) + XCTAssertFalse(store.moveWorkspace(at: index("a2", in: store), delta: 1)) + XCTAssertTrue(store.moveWorkspace(at: index("a2", in: store), toPosition: 0)) + XCTAssertEqual(visibleIDs(store), ["a", "a2", "a1", "b"]) + } + + // MARK: - Persistence + + func testFoldedStateRoundTrips() throws { + let folded = PersistedWorkspace( + title: "a", cwd: "/tmp/a", columns: [], focusedColumnIndex: 0, isGroupFolded: true + ) + let decoded = try JSONDecoder().decode(PersistedWorkspace.self, from: JSONEncoder().encode(folded)) + XCTAssertTrue(decoded.isGroupFolded) + let old = Data(#"{"title":"a","cwd":"/tmp/a","columns":[],"focusedColumnIndex":0,"isInactive":false,"lastSummaryIsManual":false}"#.utf8) + XCTAssertFalse(try JSONDecoder().decode(PersistedWorkspace.self, from: old).isGroupFolded) + } + + // MARK: - Sidebar + + private func info( + _ id: String, index: Int, parent: String? = nil, isFolded: Bool = false, inFoldedGroup: Bool = false, + isActive: Bool = false, isWaiting: Bool = false, prState: String? = nil, ciStatus: String? = nil, + isMissionCompleted: Bool = false + ) -> WorkspaceInfo { + let waiting = ColumnInfo( + index: 0, processName: "claude", abbreviatedCwd: nil, isFocused: false, isWebView: false, + webTitle: nil, terminalTitle: nil, agentStatus: .needsAttention, isEditor: false, editorFileName: nil + ) + let pullRequest = prState.map { + PRInfo( + number: 1, state: $0, isDraft: false, ciStatus: ciStatus, checks: [], reviewDecision: nil, + mergeable: nil, url: "https://github.com/o/r/pull/1", additions: nil, deletions: nil, changedFiles: nil + ) + } + return WorkspaceInfo( + id: id, index: index, title: id, profileID: WorkspaceProfile.defaultID, isInactive: false, + columnCount: isWaiting ? 1 : 0, focusedColumn: 0, gitBranch: nil, notification: nil, isActive: isActive, + columns: isWaiting ? [waiting] : [], prInfo: pullRequest, diffStats: nil, purpose: nil, nextStep: nil, + blocker: nil, phase: .active, lastSummary: nil, lastActivityAt: nil, + groupParentID: parent, isGroupFolded: isFolded, isInFoldedGroup: inFoldedGroup, + isMissionCompleted: isMissionCompleted + ) + } + + private func expandedSidebar(_ infos: [WorkspaceInfo]) -> SidebarView { + let sidebar = SidebarView(frame: NSRect(x: 0, y: 0, width: 260, height: 900)) + sidebar.isExpanded = true + sidebar.update(profiles: [], workspaces: infos) + return sidebar + } + + private func cardFrames(_ sidebar: SidebarView) -> [Int: NSRect] { + var frames: [Int: NSRect] = [:] + for area in sidebar.hitAreas { + if case .workspace(let index) = area.region { frames[index] = area.frame } + } + return frames + } + + func testChildCardIsIndentedUnderItsParentsSummaryRow() throws { + let sidebar = expandedSidebar([info("a", index: 0), info("a1", index: 1, parent: "a"), info("b", index: 2)]) + let cards = cardFrames(sidebar) + XCTAssertEqual(try XCTUnwrap(cards[1]).minX - XCTUnwrap(cards[0]).minX, SidebarExpandedMetrics.groupIndent) + XCTAssertEqual(try XCTUnwrap(cards[2]).minX, try XCTUnwrap(cards[0]).minX) + let toggle = try XCTUnwrap(sidebar.hitAreas.first { + if case .link(let url, _) = $0.region { return url == SidebarView.groupToggleActionURL(workspaceIndex: 0) } + return false + }) + XCTAssertLessThan(toggle.frame.maxY, try XCTUnwrap(cards[0]).minY) + XCTAssertGreaterThan(toggle.frame.minY, try XCTUnwrap(cards[1]).maxY) + } + + func testFoldedGroupListsOnlyTheChildOnScreenOrAskingTheUser() { + let sidebar = expandedSidebar([ + info("a", index: 0, isFolded: true), + info("quiet", index: 1, parent: "a", inFoldedGroup: true), + info("waiting", index: 2, parent: "a", inFoldedGroup: true, isWaiting: true), + info("on-screen", index: 3, parent: "a", inFoldedGroup: true, isActive: true) + ]) + XCTAssertEqual(cardFrames(sidebar).keys.sorted(), [0, 2, 3]) + XCTAssertEqual(sidebar.railWorkspaceInfos.map(\.id), ["a", "waiting", "on-screen"]) + } + + func testSummaryCountsWhereFoldedChildrenStand() { + let members = [ + info("w", index: 1, parent: "a", isWaiting: true), + info("red", index: 2, parent: "a", prState: "OPEN", ciStatus: "FAILURE"), + info("merged", index: 3, parent: "a", prState: "MERGED"), + info("completed", index: 4, parent: "a", isMissionCompleted: true) + ] + XCTAssertEqual(SidebarView.groupSummary(members: members, isFolded: false).string, "▾ 4 workspaces") + XCTAssertEqual( + SidebarView.groupSummary(members: members, isFolded: true).string, + "▸ 4 · 1 waiting · ✕ 1 · 2 done" + ) + XCTAssertEqual(SidebarView.groupSummary(members: [members[0]], isFolded: true).string, "▸ 1 · 1 waiting") + let quiet = info("q", index: 5, parent: "a") + XCTAssertEqual(SidebarView.groupSummary(members: [quiet], isFolded: true).string, "▸ 1 workspace") + } + + func testSummaryCountsGrandchildren() { + let sidebar = expandedSidebar([ + info("a", index: 0), info("a1", index: 1, parent: "a"), info("a1x", index: 2, parent: "a1") + ]) + XCTAssertEqual(sidebar.groupMembers(of: sidebar.lastInfos[0]).map(\.id), ["a1", "a1x"]) + } +} diff --git a/docs/sidebar-groups.md b/docs/sidebar-groups.md new file mode 100644 index 00000000..b880fbdc --- /dev/null +++ b/docs/sidebar-groups.md @@ -0,0 +1,71 @@ +# Sidebar Groups + +Status: v1 implemented. Direction validated by the user on 2026-10-07: +"faire des sortes de dossier, en dessous de celui-ci pourraient vivre des +workspaces". Shape validated on 2026-10-08, with the folded state saved and +red checks counted. + +A **group** is a workspace and the workspaces nested under it, inside a +project's sidebar. A parent that opens four review worktrees through +`nirux-worktree` lists five cards today, flat, in opening order: nothing +says which ones it opened, and they push the rest of the project down. + +What Nirux already knows: a workspace opened as a Mission child keeps its +`missionID` (state.json), and its Mission records `parentWorkspaceID` +(missions.json). No new data is needed for v1. + +## Shape + +**1. One order for everything.** `WorkspaceStore.visibleWorkspaceIndices` +lists each workspace followed by its children, depth first, siblings in +store order. The sidebar cards, the rail, the workspace strip, ⌘↑/↓ and the +close fallback already follow that list, so they agree without changes. + +- A workspace's **parent** is its Mission's `parentWorkspaceID`, when that + workspace is open in the same project and the same section (ACTIVE or + INACTIVE). Otherwise it sits at the top level: a parent closed, moved to + another project or parked in INACTIVE lets its children go. +- Read from `MissionStore` on each pass, not cached: missions load after + the workspaces are restored. +- A grandchild follows its own parent (depth first), drawn at the same + indent as a child: one level of indent in a 260pt sidebar. + +**2. Show it.** + +- A child card is indented by 12pt, the same card otherwise. +- Under a parent card, a toggle row: `▾ 3 workspaces`. Folded, where they + stand, most pressing first: `▸ 3 · 1 waiting · ✕ 1 · 2 done · 1 working`, + each count in its state's color. ✕ counts open pull requests with red + checks; done, children whose Mission completed or whose PR merged. With + no count, `▸ 3 workspaces`. +- A folded group still lists a child on screen or one that asks the user + (waiting, broken): the rule the folded INACTIVE section follows + (`listsWorkspace`), without Allow / Deny buttons there too. ⌘↑/↓ walks + into folded children like into folded inactive ones, and the sidebar + then lists the one on screen. +- Folding is saved in state.json, on the parent (`isGroupFolded`): a + parent with many reviews stays folded across launches. +- **Rail** (collapsed sidebar, #106): same order, no indent and no toggle + tile; a folded group hides its quiet children there too. + +**3. Move.** + +- Move Up / Move Down and drag-reorder move a workspace among its + siblings; its children follow. A top-level drag slides over whole groups + (the card and its listed children are one drop target). +- A Mission child can't leave its parent in v1: that is step 2. + +## Not in v1 + +- **Manual folders** (step 2, own PR): "Nest Under ▸" in the card menu, or + a drop onto a card, persisted in state.json as the workspace's chosen + parent, which wins over the Mission's. A nest that would make a loop is + refused. +- **PR stacks** ([pr-stacks.md](pr-stacks.md)) as groups: only if step 2 + makes it fall out. + +## Decided + +- **Approval text wraps at 26 characters**, not 28: an indented card's box + is 12pt narrower, and the request text is never clipped. Every card uses + the same width so a card doesn't reflow when its parent closes. From 0a216baa2e7ecee4b0cffdc183433dda1a7113a7 Mon Sep 17 00:00:00 2001 From: Cheun Kitabayashi Date: Thu, 8 Oct 2026 20:02:07 +0200 Subject: [PATCH 2/2] fix(sidebar): groups dissolve in place, and fold like one card From the review passes on the first commit: - The store keeps the sidebar's order: a Mission child goes after its parent's group, and a move carries the whole group. A closed parent left its children at the bottom of the list, the strip sliding with them. - A new child unfolds its parent, instead of hiding under an old fold. - Cmd+Up/Down steps over a folded group, and closing a folded parent selects the next card rather than a hidden child. - Missions load before the restore: the first layout is already grouped. - No summary row when the children can't show (a parent in the folded INACTIVE section, listed for being on screen). Folding keeps the row under the pointer, like the INACTIVE header. A child's drop line starts at its card. Co-Authored-By: Claude Opus 5.5 (1M context) --- Sources/Nirux/Model/WorkspaceStore.swift | 45 ++++++++--- Sources/Nirux/NiruxApp.swift | 3 +- .../Views/NiruxShellView+Workspaces.swift | 2 +- Sources/Nirux/Views/SidebarView+Drag.swift | 14 ++-- Sources/Nirux/Views/SidebarView+Groups.swift | 46 ++++++----- .../Nirux/Views/SidebarView+Rendering.swift | 2 +- Sources/Nirux/Views/SidebarView.swift | 28 ++++--- Tests/NiruxTests/SidebarGroupsTests.swift | 76 ++++++++++++++++++- docs/sidebar-groups.md | 17 +++-- 9 files changed, 175 insertions(+), 58 deletions(-) diff --git a/Sources/Nirux/Model/WorkspaceStore.swift b/Sources/Nirux/Model/WorkspaceStore.swift index 52b5bcb8..48d9104a 100644 --- a/Sources/Nirux/Model/WorkspaceStore.swift +++ b/Sources/Nirux/Model/WorkspaceStore.swift @@ -56,8 +56,15 @@ final class WorkspaceStore { reconcileSelection(preferActiveProfile: true) } + /// A Mission child goes after its parent's group, which it unfolds: the + /// store keeps the sidebar's order, so a group dissolves in place. func appendWorkspace(_ workspace: WorkspaceState, activate: Bool = true) { workspaces.append(workspace) + if let parent = groupParentIndex(of: workspaces.count - 1) { + workspaces[parent].isGroupFolded = false + let groupEnd = groupBlock(of: parent).filter { $0 != workspaces.count - 1 }.max() ?? parent + workspaces.insert(workspaces.removeLast(), at: groupEnd + 1) + } if activate { selectWorkspace(id: workspace.id) } } @@ -117,7 +124,8 @@ final class WorkspaceStore { @discardableResult func selectAdjacentWorkspace(delta: Int) -> Int? { - let visible = visibleWorkspaceIndices + // A folded group is one stop, as in the sidebar. + let visible = visibleWorkspaceIndices.filter { $0 == activeWorkspaceIndex || !isInFoldedGroup($0) } guard let current = visible.firstIndex(of: activeWorkspaceIndex) else { if let first = visible.first { selectWorkspace(at: first); return first } return nil @@ -146,7 +154,9 @@ final class WorkspaceStore { let list = visibleWorkspaceIndices(in: profileID) let position = list.firstIndex(of: index) ?? list.count let neighbours = (Array(list[position...]) + list[.. Bool { - guard delta != 0, let (candidates, position) = visibleGroupPosition(of: index) else { return false } + guard delta != 0, let (candidates, position) = visibleSiblingPosition(of: index) else { return false } let newPosition = position + delta guard candidates.indices.contains(newPosition) else { return false } - let workspace = workspaces[index] - let targetWorkspace = workspaces[candidates[newPosition]] - workspaces.remove(at: index) - let adjustedTarget = workspaces.firstIndex { $0 === targetWorkspace } ?? workspaces.count - let insertIndex = delta > 0 ? adjustedTarget + 1 : adjustedTarget - workspaces.insert(workspace, at: min(insertIndex, workspaces.count)) + // The group moves as one block of the store, past the target's. + let moving = groupBlock(of: index).sorted().map { workspaces[$0] } + let targetBlock = groupBlock(of: candidates[newPosition]) + let anchor = workspaces[(delta > 0 ? targetBlock.max() : targetBlock.min()) ?? candidates[newPosition]] + workspaces.removeAll { workspace in moving.contains { $0 === workspace } } + let anchorIndex = workspaces.firstIndex { $0 === anchor } ?? workspaces.count + workspaces.insert(contentsOf: moving, at: min(delta > 0 ? anchorIndex + 1 : anchorIndex, workspaces.count)) reconcileSelection(preferActiveProfile: true) return true } + /// A workspace and its descendants, in the sidebar's order. + private func groupBlock(of index: Int) -> [Int] { + let list = visibleWorkspaceIndices(in: workspaces[index].profileID) + guard let start = list.firstIndex(of: index) else { return [index] } + return [index] + list[(start + 1)...].prefix { isDescendant($0, of: index) } + } + + private func isDescendant(_ index: Int, of ancestor: Int) -> Bool { + guard let parent = groupParentIndex(of: index) else { return false } + return parent == ancestor || isDescendant(parent, of: ancestor) + } + /// Move a workspace to an absolute position among its siblings (0 = /// first), its children with it. The position is clamped to the /// siblings' bounds; a move never crosses the active/inactive boundary /// nor leaves its parent. @discardableResult func moveWorkspace(at index: Int, toPosition targetPosition: Int) -> Bool { - guard let (candidates, position) = visibleGroupPosition(of: index) else { return false } + guard let (candidates, position) = visibleSiblingPosition(of: index) else { return false } let clamped = max(0, min(targetPosition, candidates.count - 1)) guard clamped != position else { return false } return moveWorkspace(at: index, delta: clamped - position) @@ -186,7 +209,7 @@ final class WorkspaceStore { /// A workspace's siblings within the visible (active profile) list: /// same section, same parent. Plus its position among them. - private func visibleGroupPosition(of index: Int) -> (candidates: [Int], position: Int)? { + private func visibleSiblingPosition(of index: Int) -> (candidates: [Int], position: Int)? { guard workspaces.indices.contains(index) else { return nil } let isInactive = workspaces[index].isInactive let parent = groupParentIndex(of: index) diff --git a/Sources/Nirux/NiruxApp.swift b/Sources/Nirux/NiruxApp.swift index 78d29f66..b55fc197 100644 --- a/Sources/Nirux/NiruxApp.swift +++ b/Sources/Nirux/NiruxApp.swift @@ -144,6 +144,8 @@ final class NiruxApp: NSObject, NSApplicationDelegate, SPUUpdaterDelegate, NSMen window.makeKeyAndOrderFront(nil) mainWindow = window + // Before the restore: missions group the workspaces it lays out. + MissionStore.shared.load() // Restore previous session if available shellView.restoreState() @@ -152,7 +154,6 @@ final class NiruxApp: NSObject, NSApplicationDelegate, SPUUpdaterDelegate, NSMen // run AFTER restoreState so queued events resolve to live columns. installAgentHooks(reportingTo: shellView) ActivityStore.shared.load() - MissionStore.shared.load() ActivityStore.shared.onChange = { [weak shellView] in shellView?.refreshActivitySidebar() } diff --git a/Sources/Nirux/Views/NiruxShellView+Workspaces.swift b/Sources/Nirux/Views/NiruxShellView+Workspaces.swift index 1796a803..387a28e0 100644 --- a/Sources/Nirux/Views/NiruxShellView+Workspaces.swift +++ b/Sources/Nirux/Views/NiruxShellView+Workspaces.swift @@ -345,7 +345,7 @@ extension NiruxShellView { } /// Drop handler for sidebar drag-reorder: move the workspace to an - /// absolute position within its active/inactive group, then run the + /// absolute position among its siblings, then run the /// same refresh dance as the context-menu moves. func handleWorkspaceReorder(workspaceIndex: Int, targetPosition: Int) { guard workspaces.indices.contains(workspaceIndex), diff --git a/Sources/Nirux/Views/SidebarView+Drag.swift b/Sources/Nirux/Views/SidebarView+Drag.swift index 91a584cb..105b4bb3 100644 --- a/Sources/Nirux/Views/SidebarView+Drag.swift +++ b/Sources/Nirux/Views/SidebarView+Drag.swift @@ -137,15 +137,15 @@ extension SidebarView { /// A workspace's card, or tile, with its listed children and its /// summary row: they move together, one drop target. - private func blockFrame(of workspace: WorkspaceInfo) -> NSRect? { + func blockFrame(of workspace: WorkspaceInfo) -> NSRect? { let indices = Set([workspace.index] + groupMembers(of: workspace).map(\.index)) let toggles = Set(indices.map { Self.groupToggleActionURL(workspaceIndex: $0) }) - var card: NSRect? + var hasCard = false var block = NSRect.null for area in hitAreas { switch area.region { case .workspace(let index) where indices.contains(index): - if index == workspace.index { card = area.frame } + hasCard = hasCard || index == workspace.index block = block.union(area.frame) case .link(let url, _) where toggles.contains(url): block = block.union(area.frame) @@ -153,7 +153,8 @@ extension SidebarView { continue } } - return card.map { _ in block } + guard hasCard else { return nil } + return block } private func dragMoved(_ event: NSEvent, drag: inout SidebarWorkspaceDrag) { @@ -322,8 +323,9 @@ extension SidebarView { } else { y = (rows[slot - 1].minY + rows[slot].maxY) / 2 } - let inset = isExpanded ? SidebarExpandedMetrics.workspaceInsetX : SidebarRailMetrics.tileX - let width = isExpanded ? bounds.width - inset * 2 : SidebarRailMetrics.tileSize + // A child's line starts at its indented card. + let inset = isExpanded ? rows[0].minX : SidebarRailMetrics.tileX + let width = isExpanded ? bounds.width - SidebarExpandedMetrics.workspaceInsetX - inset : SidebarRailMetrics.tileSize indicator.frame = NSRect(x: inset, y: y - 1, width: width, height: 2) indicator.isHidden = false } diff --git a/Sources/Nirux/Views/SidebarView+Groups.swift b/Sources/Nirux/Views/SidebarView+Groups.swift index 01ffdfb9..2cb2771f 100644 --- a/Sources/Nirux/Views/SidebarView+Groups.swift +++ b/Sources/Nirux/Views/SidebarView+Groups.swift @@ -3,9 +3,12 @@ import AppKit // MARK: - Groups (docs/sidebar-groups.md) extension SidebarView { - /// Some workspace is listed under it. - func hasGroup(_ workspace: WorkspaceInfo) -> Bool { + /// Some workspace is listed under it, and could show: not when the + /// card itself is listed only for being on screen or asking the user, + /// in a folded section or group. + func showsGroupToggle(_ workspace: WorkspaceInfo) -> Bool { lastInfos.contains { $0.groupParentID == workspace.id } + && listsWorkspace(isInactive: workspace.isInactive, isInFoldedGroup: workspace.isInFoldedGroup, isActive: false) } /// Its children, each followed by its own, listed or folded. @@ -15,7 +18,7 @@ extension SidebarView { /// The summary rows under the workspaces of `infos` that have children. func groupTogglesHeight(_ infos: [WorkspaceInfo]) -> CGFloat { - CGFloat(infos.filter(hasGroup).count) * (SidebarExpandedMetrics.groupToggleHeight + SidebarExpandedMetrics.workspaceGap) + CGFloat(infos.filter(showsGroupToggle).count) * (SidebarExpandedMetrics.groupToggleHeight + SidebarExpandedMetrics.workspaceGap) } /// The row under a workspace with children, where its children start: @@ -28,7 +31,8 @@ extension SidebarView { width: bounds.width - metrics.workspaceInsetX - rowX, height: metrics.groupToggleHeight ) let textX = rowX + metrics.cardPaddingX - let label = NSTextField.sidebarLine(Self.groupSummary(members: groupMembers(of: parent), isFolded: parent.isGroupFolded)) + let members = groupMembers(of: parent) + let label = NSTextField.sidebarLine(Self.groupSummary(members: members, isFolded: parent.isGroupFolded)) label.frame = NSRect(x: textX, y: rowFrame.minY + 1, width: rowFrame.maxX - metrics.cardPaddingX - textX, height: 16) label.setAccessibilityElement(false) addSubviewDoc(label) @@ -38,41 +42,47 @@ extension SidebarView { )) let toggle = SidebarSectionToggleView(frame: rowFrame) toggle.toolTip = "\(parent.isGroupFolded ? "Show" : "Hide") the workspaces opened from \(parent.title)" - toggle.setAccessibilityLabel("Workspaces opened from \(parent.title): \(label.stringValue.dropFirst(2))") + toggle.setAccessibilityLabel("Workspaces opened from \(parent.title), \(members.count)") toggle.setAccessibilityExpanded(!parent.isGroupFolded) let index = parent.index - toggle.onPress = { [weak self] in self?.onWorkspaceAction?(.toggleGroup, index) } + toggle.onPress = { [weak self] in self?.toggleGroup(workspaceIndex: index) } addSubviewDoc(toggle) expandedViews.append(toggle) return rowFrame.minY } + /// Keeps the row under the pointer, as `toggleInactiveSection` does. + func toggleGroup(workspaceIndex: Int) { + keepingDocumentTop { onWorkspaceAction?(.toggleGroup, workspaceIndex) } + } + /// "▾ 3 workspaces"; folded, where they stand, most pressing first: /// "▸ 3 · 1 waiting · ✕ 1 · 2 done · 1 working", each count in its /// state's color. ✕ counts open pull requests with red checks; done, /// children whose Mission completed or whose pull request merged. static func groupSummary(members: [WorkspaceInfo], isFolded: Bool) -> NSAttributedString { let font = Theme.Font.caption - func count(_ isCounted: (WorkspaceInfo) -> Bool) -> Int { isFolded ? members.filter(isCounted).count : 0 } - let allCounts: [(word: String, count: Int, color: NSColor)] = [ - ("waiting", count { $0.cardState == .waiting }, Theme.Color.waiting), - ("✕", count { $0.prInfo?.state == "OPEN" && $0.prInfo?.ciStatus == "FAILURE" }, Theme.Color.error), - ("done", count { $0.isMissionCompleted || $0.prInfo?.state == "MERGED" }, Theme.Color.done), - ("working", count { $0.cardState == .working }, Theme.Color.working) + func count(_ isCounted: (WorkspaceInfo) -> Bool) -> Int { members.filter(isCounted).count } + let waiting = count { $0.cardState == .waiting } + let red = count { $0.prInfo?.state == "OPEN" && $0.prInfo?.ciStatus == "FAILURE" } + let done = count { $0.isMissionCompleted || $0.prInfo?.state == "MERGED" } + let working = count { $0.cardState == .working } + let allCounts: [(count: Int, text: String, color: NSColor)] = [ + (waiting, "\(waiting) waiting", Theme.Color.waiting), + (red, "✕ \(red)", Theme.Color.error), + (done, "\(done) done", Theme.Color.done), + (working, "\(working) working", Theme.Color.working) ] - let counts = allCounts.filter { $0.count > 0 } + let counts = isFolded ? allCounts.filter { $0.count > 0 } : [] // The counts say what they are: the noun would push them out. let total = counts.isEmpty ? workspaceCountText(members.count) : "\(members.count)" let text = NSMutableAttributedString( string: "\(isFolded ? "▸" : "▾") \(total)", attributes: [.font: font, .foregroundColor: Theme.Color.textTertiary] ) - for (word, count, color) in counts { + for (_, countText, color) in counts { text.append(NSAttributedString(string: " · ", attributes: [.font: font, .foregroundColor: Theme.Color.textTertiary])) - text.append(NSAttributedString( - string: word == "✕" ? "✕ \(count)" : "\(count) \(word)", - attributes: [.font: font, .foregroundColor: color] - )) + text.append(NSAttributedString(string: countText, attributes: [.font: font, .foregroundColor: color])) } return text } diff --git a/Sources/Nirux/Views/SidebarView+Rendering.swift b/Sources/Nirux/Views/SidebarView+Rendering.swift index 48c2b84b..8817f632 100644 --- a/Sources/Nirux/Views/SidebarView+Rendering.swift +++ b/Sources/Nirux/Views/SidebarView+Rendering.swift @@ -445,7 +445,7 @@ extension SidebarView { for workspace in infos { currentY = buildWorkspaceSection(workspace: workspace, padding: padding, yOffset: currentY) currentY -= SidebarExpandedMetrics.workspaceGap - if hasGroup(workspace) { + if showsGroupToggle(workspace) { currentY = buildGroupToggle(workspace, yOffset: currentY) - SidebarExpandedMetrics.workspaceGap } } diff --git a/Sources/Nirux/Views/SidebarView.swift b/Sources/Nirux/Views/SidebarView.swift index affbefec..6b5edd26 100644 --- a/Sources/Nirux/Views/SidebarView.swift +++ b/Sources/Nirux/Views/SidebarView.swift @@ -52,7 +52,7 @@ final class SidebarView: NSView { /// in a linked worktree, or its folder is gone). Asked on each menu. var offersWorktreeCleanup: ((Int) -> Bool)? /// Drag-reorder drop: (store index of dragged workspace, target - /// position within its active/inactive group). + /// position among its siblings). var onWorkspaceReordered: ((Int, Int) -> Void)? var onProfileClicked: ((String) -> Void)? var onCreateProfile: (() -> Void)? @@ -442,7 +442,7 @@ final class SidebarView: NSView { } else if let workspaceIndex = Self.actionWorkspaceIndex(url, prefix: Self.cleanupActionPrefix) { onWorkspaceAction?(.cleanUpWorktree, workspaceIndex) } else if let workspaceIndex = Self.actionWorkspaceIndex(url, prefix: Self.groupToggleActionPrefix) { - onWorkspaceAction?(.toggleGroup, workspaceIndex) + toggleGroup(workspaceIndex: workspaceIndex) } else if let workspaceID = Self.prFeedbackActionWorkspaceID(url) { prFeedbackMenu?(workspaceID)?.popUp(positioning: nil, at: convert(event.locationInWindow, from: nil), in: self) } else if let (workspaceIndex, url) = Self.openActionTarget(url), @@ -696,6 +696,19 @@ extension SidebarView { var hasInactiveWorkspaces: Bool { lastInfos.contains(where: \.isInactive) } + /// Keeps a toggle's row under the pointer for the next click. Only rows + /// below it change, but the document view isn't flipped, so a rebuild + /// keeps the distance to the bottom: keep the one to the top. + func keepingDocumentTop(_ rebuild: () -> Void) { + let clip = contentScrollView.contentView + let visibleTopFromDocumentTop = contentDocumentView.frame.height - clip.bounds.maxY + rebuild() + let highestOrigin = max(0, contentDocumentView.frame.height - clip.bounds.height) + let originY = contentDocumentView.frame.height - visibleTopFromDocumentTop - clip.bounds.height + clip.scroll(to: NSPoint(x: clip.bounds.origin.x, y: min(max(0, originY), highestOrigin))) + contentScrollView.reflectScrolledClipView(clip) + } + /// The header row's hit area (the rail's toggle tile), in document /// coordinates. var inactiveSectionHeaderFrame: NSRect? { @@ -719,16 +732,7 @@ extension SidebarView { rebuildContent() return } - // Keep the header under the pointer for the next click. Only rows - // below it change, but the document view isn't flipped, so a - // rebuild keeps the distance to the bottom: keep the one to the top. - let clip = contentScrollView.contentView - let visibleTopFromDocumentTop = contentDocumentView.frame.height - clip.bounds.maxY - rebuildContent() - let highestOrigin = max(0, contentDocumentView.frame.height - clip.bounds.height) - let originY = contentDocumentView.frame.height - visibleTopFromDocumentTop - clip.bounds.height - clip.scroll(to: NSPoint(x: clip.bounds.origin.x, y: min(max(0, originY), highestOrigin))) - contentScrollView.reflectScrolledClipView(clip) + keepingDocumentTop { rebuildContent() } // From the menu or ⌘P the header may be out of view. if let header = inactiveSectionHeaderFrame { contentDocumentView.scrollToVisible(header) } // The rebuild read the pointer before the rows moved under it. diff --git a/Tests/NiruxTests/SidebarGroupsTests.swift b/Tests/NiruxTests/SidebarGroupsTests.swift index c7de18e5..190aaa0e 100644 --- a/Tests/NiruxTests/SidebarGroupsTests.swift +++ b/Tests/NiruxTests/SidebarGroupsTests.swift @@ -86,6 +86,55 @@ final class SidebarGroupsTests: XCTestCase { XCTAssertEqual(visibleIDs(store), ["a", "a2", "a1", "b"]) } + // MARK: - Store order + + private func storeIDs(_ store: WorkspaceStore) -> [String] { + store.workspaces.map(\.id) + } + + /// A group dissolves in place: a closed parent leaves its children + /// where it was, not at the bottom. + func testMissionChildIsStoredAfterItsParentsGroup() { + let store = makeStore(["a", "b", "a1", "a2", "a1x"], parents: ["a1": "a", "a2": "a", "a1x": "a1"]) + XCTAssertEqual(storeIDs(store), ["a", "a1", "a1x", "a2", "b"]) + store.removeWorkspace(store.workspaces[0]) + XCTAssertEqual(visibleIDs(store), ["a1", "a1x", "a2", "b"]) + } + + func testNewChildUnfoldsItsParent() { + let store = makeStore(["a", "a1"], parents: ["a1": "a", "a2": "a"]) + store.workspaces[0].isGroupFolded = true + store.appendWorkspace(WorkspaceState(id: "a2", title: "a2", cwd: "/tmp/a2", missionID: "mission-a2")) + XCTAssertFalse(store.workspaces[0].isGroupFolded) + } + + /// State saved before groups: the children at the bottom of the store. + func testMovedGroupIsStoredTogether() { + let store = makeStore(["a", "b", "c"]) + store.missionParentID = { $0 == "mission-a1" ? "a" : nil } + store.replaceWorkspaces(store.workspaces + [WorkspaceState(id: "a1", title: "a1", cwd: "/tmp/a1", missionID: "mission-a1")]) + XCTAssertTrue(store.moveWorkspace(at: index("a", in: store), delta: 1)) + XCTAssertEqual(storeIDs(store), ["b", "a", "a1", "c"]) + XCTAssertTrue(store.moveWorkspace(at: index("c", in: store), delta: -1)) + XCTAssertEqual(storeIDs(store), ["b", "c", "a", "a1"]) + } + + func testClosingAFoldedParentSelectsTheNextCardNotAHiddenChild() { + let store = makeStore(["a", "a1", "b"], parents: ["a1": "a"]) + store.workspaces[index("a", in: store)].isGroupFolded = true + XCTAssertEqual(store.fallbackIndexAfterClosingWorkspace(at: index("a", in: store)), index("b", in: store)) + } + + func testAdjacentWorkspaceSkipsFoldedChildren() { + let store = makeStore(["a", "a1", "b"], parents: ["a1": "a"]) + store.workspaces[index("a", in: store)].isGroupFolded = true + store.selectWorkspace(id: "a") + XCTAssertEqual(store.selectAdjacentWorkspace(delta: 1), index("b", in: store)) + // On screen, a folded child steps out like any card. + store.selectWorkspace(id: "a1") + XCTAssertEqual(store.selectAdjacentWorkspace(delta: 1), index("b", in: store)) + } + // MARK: - Persistence func testFoldedStateRoundTrips() throws { @@ -102,8 +151,8 @@ final class SidebarGroupsTests: XCTestCase { private func info( _ id: String, index: Int, parent: String? = nil, isFolded: Bool = false, inFoldedGroup: Bool = false, - isActive: Bool = false, isWaiting: Bool = false, prState: String? = nil, ciStatus: String? = nil, - isMissionCompleted: Bool = false + isActive: Bool = false, isInactive: Bool = false, isWaiting: Bool = false, prState: String? = nil, + ciStatus: String? = nil, isMissionCompleted: Bool = false ) -> WorkspaceInfo { let waiting = ColumnInfo( index: 0, processName: "claude", abbreviatedCwd: nil, isFocused: false, isWebView: false, @@ -116,7 +165,7 @@ final class SidebarGroupsTests: XCTestCase { ) } return WorkspaceInfo( - id: id, index: index, title: id, profileID: WorkspaceProfile.defaultID, isInactive: false, + id: id, index: index, title: id, profileID: WorkspaceProfile.defaultID, isInactive: isInactive, columnCount: isWaiting ? 1 : 0, focusedColumn: 0, gitBranch: nil, notification: nil, isActive: isActive, columns: isWaiting ? [waiting] : [], prInfo: pullRequest, diffStats: nil, purpose: nil, nextStep: nil, blocker: nil, phase: .active, lastSummary: nil, lastActivityAt: nil, @@ -187,4 +236,25 @@ final class SidebarGroupsTests: XCTestCase { ]) XCTAssertEqual(sidebar.groupMembers(of: sidebar.lastInfos[0]).map(\.id), ["a1", "a1x"]) } + + /// A parent and its children are one drop target; a child's siblings + /// are its parent's other children. + func testDragBlockSpansTheParentItsRowAndItsChildren() throws { + let sidebar = expandedSidebar([info("a", index: 0), info("a1", index: 1, parent: "a"), info("b", index: 2)]) + let cards = cardFrames(sidebar) + let block = try XCTUnwrap(sidebar.blockFrame(of: sidebar.lastInfos[0])) + XCTAssertEqual(block.maxY, try XCTUnwrap(cards[0]).maxY) + XCTAssertEqual(block.minY, try XCTUnwrap(cards[1]).minY) + XCTAssertEqual(sidebar.blockFrame(of: sidebar.lastInfos[2]), cards[2]) + } + + /// An inactive parent listed only for being on screen: its children + /// stay folded with the section, so no row offers them. + func testNoSummaryRowWhenTheChildrenCantShow() { + let parent = info("a", index: 0, isActive: true, isInactive: true) + let sidebar = expandedSidebar([info("b", index: 2), parent, info("a1", index: 1, parent: "a", isInactive: true)]) + XCTAssertFalse(sidebar.showsGroupToggle(parent)) + sidebar.toggleInactiveSection() + XCTAssertTrue(sidebar.showsGroupToggle(parent)) + } } diff --git a/docs/sidebar-groups.md b/docs/sidebar-groups.md index b880fbdc..844eebf2 100644 --- a/docs/sidebar-groups.md +++ b/docs/sidebar-groups.md @@ -25,8 +25,11 @@ close fallback already follow that list, so they agree without changes. workspace is open in the same project and the same section (ACTIVE or INACTIVE). Otherwise it sits at the top level: a parent closed, moved to another project or parked in INACTIVE lets its children go. -- Read from `MissionStore` on each pass, not cached: missions load after - the workspaces are restored. +- Read from `MissionStore` on each pass, not cached. Missions load before + the workspaces are restored, so the first layout is already grouped. +- The store keeps that order: a new Mission child goes after its parent's + group, and a move carries a whole group. A parent closed, parked or + moved away leaves its children in its place, at the top level. - A grandchild follows its own parent (depth first), drawn at the same indent as a child: one level of indent in a 260pt sidebar. @@ -40,9 +43,13 @@ close fallback already follow that list, so they agree without changes. no count, `▸ 3 workspaces`. - A folded group still lists a child on screen or one that asks the user (waiting, broken): the rule the folded INACTIVE section follows - (`listsWorkspace`), without Allow / Deny buttons there too. ⌘↑/↓ walks - into folded children like into folded inactive ones, and the sidebar - then lists the one on screen. + (`listsWorkspace`), without Allow / Deny buttons there too. ⌘↑/↓ steps + over a folded group as over one card (⌘J still reaches a waiting child), + and closing a folded parent selects the next card, not a hidden child. +- A new child unfolds its parent: it would hide under a fold left from + earlier children. +- A parent listed only for being on screen or asking the user, in the + folded INACTIVE section, has no summary row: its children can't show. - Folding is saved in state.json, on the parent (`isGroupFolded`): a parent with many reviews stays folded across launches. - **Rail** (collapsed sidebar, #106): same order, no indent and no toggle