diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Logic/SidebarLayout+EmptyGroup.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Logic/SidebarLayout+EmptyGroup.swift deleted file mode 100644 index 9c56e3eaf56b..000000000000 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Logic/SidebarLayout+EmptyGroup.swift +++ /dev/null @@ -1,28 +0,0 @@ -import CoreGraphics - -// An open group with no members (user 9757): one quiet row under its header, as tall and as far in as a -// one-line member row, so the group reads as open and empty instead of a header with the next group right -// under it. The row is a drop target: the resolver leaves it out (`RustRow.resolvable`), so a pointer over -// it falls on the open header's lower part, which drops into the group at its start. -nonisolated extension SidebarLayout { - /// Whether an open `group` with no shown members lists its empty row: never while filtering (the group - /// hides or shows the path to a match), never for a saved group (a click reopens it), and never for a - /// group holding subgroups (they follow it). - static func listsEmptyRow(_ group: SidebarGroup, filtering: Bool, hasSubgroups: Bool) -> Bool { - !filtering && !group.isPinned && !hasSubgroups - } - - /// Appends `group`'s empty row at `y` (member depth, the group's color for its line) and returns the y after it. - static func appendEmptyRow(_ group: SidebarGroup, section: SectionID, parent: Int, metrics m: SidebarLayoutMetrics, - to rows: inout [SidebarRow], y: CGFloat) -> CGFloat { - let h = m.height(for: WorkspaceRowContent()) - var row = SidebarRow( - key: .emptyGroup(group.id), y: y, height: h, section: section, - group: group.id, siblingIndex: 0, parentIndex: parent, isLastInGroup: true, - isCollapsed: false, childCount: 0, groupColor: group.color - ) - row.depth = group.depth + 1 - rows.append(row) - return y + h + m.rowSpacing - } -} diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Logic/SidebarLayout+Fold.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Logic/SidebarLayout+Fold.swift index 23a8ef2b86eb..59a7b34f0910 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Logic/SidebarLayout+Fold.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Logic/SidebarLayout+Fold.swift @@ -86,11 +86,6 @@ nonisolated extension SidebarLayout { return shown.first(where: { $0.id == member.id }).map { (i, $0) } } for (n, (childIndex, ws)) in kept.enumerated() { workspaceRow(ws, at: childIndex, last: n == kept.count - 1) } - } else if members.isEmpty { - // Subgroups and filters already sent the full layout above; the empty row is `make`'s. - if Self.listsEmptyRow(group, filtering: false, hasSubgroups: false) { - y = Self.appendEmptyRow(group, section: section.id, parent: parent, metrics: m, to: &built, y: y) - } } else { for (childIndex, member) in members.enumerated() { switch member { diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Logic/SidebarLayout.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Logic/SidebarLayout.swift index aecb1adb1e08..76937a1d046f 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Logic/SidebarLayout.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Logic/SidebarLayout.swift @@ -392,12 +392,6 @@ public nonisolated struct SidebarLayout: Hashable, Sendable { } return } - // An open group with nothing in it keeps one quiet member-sized row (user 9757): it reads as open - // and empty, and a drop over it goes into the group. A drag's gap here takes its place. - if members.isEmpty, o.gap?.group != group.id, - Self.listsEmptyRow(group, filtering: filtering, hasSubgroups: subgroupParents.contains(group.id)) { - y = Self.appendEmptyRow(group, section: section.id, parent: index, metrics: m, to: &rows, y: y) - } let fold = SidebarDoneFoldID(section: section.id, group: group.id) let drawn = Self.foldingDone(Self.treeOrder(Self.workspaceIDs(members), parents: o.sessionTree?.parents), workspace: { if case let .workspace(ws) = members[$0] { ws } else { nil } }, fold: fold, options: o) diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Views/SidebarListView+Drag.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Views/SidebarListView+Drag.swift index 494c6944b3ac..635f68a43bb8 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Views/SidebarListView+Drag.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Views/SidebarListView+Drag.swift @@ -496,7 +496,7 @@ extension SidebarListViewComponents { case let (.workspace(id), rowView as WorkspaceRowView): rowView.isDropTarget = drag?.target == .ontoWorkspace(id) || standsForDropGroup(row) case let (.group(id), header as GroupHeaderRowView): - header.isDropTarget = drag?.target == .intoGroup(id) + header.isDropTarget = highlightsDropGroup(id) default: break } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Views/SidebarListView+Layout.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Views/SidebarListView+Layout.swift index 79fd37b96957..8f9e7e624176 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Views/SidebarListView+Layout.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Views/SidebarListView+Layout.swift @@ -442,7 +442,7 @@ extension SidebarListViewComponents { view.configure(group, row: row, details: details, status: subtree.statusRollUp(collapsed: row.isCollapsed), animated: animated || model.followsUserIntent) view.railReach = railReach(for: row) - view.isDropTarget = drag?.target == .intoGroup(id) || external?.proposal == .intoGroup(id) + view.isDropTarget = highlightsDropGroup(id) case let (.section(id), view as SectionHeaderRowView): guard let section = sections[id] else { return } view.configure(section, row: row) diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Views/SidebarListView+PointerEnd.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Views/SidebarListView+PointerEnd.swift index 86ef0f4366aa..4d12284ead28 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Views/SidebarListView+PointerEnd.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Views/SidebarListView+PointerEnd.swift @@ -119,6 +119,15 @@ extension SidebarListViewComponents { return true } + /// An open empty group's insertion position is its header's drop target, too. Keep this + /// visual state tied to the live drag; at rest the header has no empty-state decoration. + func highlightsDropGroup(_ id: GroupID) -> Bool { + if drag?.target == .intoGroup(id) || external?.proposal == .intoGroup(id) { return true } + guard case let .position(position)? = drag?.target, position.group == id, + groups[id]?.members.isEmpty == true else { return false } + return true + } + /// Sets each realized row's selection, multi-selection and drop-target fill from the model and the live drag, as /// `configure` would, without configuring the rows (no content, no layout: a click stays cheap, u2455). A row whose /// fill a gesture left behind clears here. @@ -131,7 +140,7 @@ extension SidebarListViewComponents { row.isDropTarget = external?.proposal == .intoWorkspace(id) || drag?.target == .ontoWorkspace(id) || displayed.row(for: key).map(standsForDropGroup) == true case let (.group(id), header as GroupHeaderRowView): - header.isDropTarget = drag?.target == .intoGroup(id) || external?.proposal == .intoGroup(id) + header.isDropTarget = highlightsDropGroup(id) default: break } diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/SidebarDragKeepsGroupsTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/SidebarDragKeepsGroupsTests.swift index afe06e3e53c1..2b653c5e6b1b 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/SidebarDragKeepsGroupsTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/SidebarDragKeepsGroupsTests.swift @@ -129,8 +129,7 @@ import Testing list.press = SidebarPress(key: dragged, point: start) list.beginDrag(try #require(list.press)) try #require(list.drag != nil, "the member did not lift") - // Past the last row as the drag draws the list: the emptied group shows a placeholder row - // while its member is lifted (0105692a41d), which pushes the rows below it down a row. + // Past the last row as the drag draws the list; the emptied group keeps only its header. @MainActor func bottom() -> CGFloat { (list.displayed.rows.last?.maxY ?? 300) + 10 } var y = start.y, gone: [Int] = [], frame = 0 while y < bottom(), frame < 200 { diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/SidebarEmptyGroupAddButtonTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/SidebarEmptyGroupAddButtonTests.swift index a4519a77c13c..81de541d86dc 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/SidebarEmptyGroupAddButtonTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/SidebarEmptyGroupAddButtonTests.swift @@ -4,15 +4,12 @@ import CmuxNextDesign import CmuxNextTestSupport import Testing -/// User msg 17912 ("empty workspace needs plus button", a shot of an open, empty "Sleep alarm" group): the -/// empty group's row read only "No workspaces · drag one here", and its header hid the + an empty group needs -/// most. The row shows a + at rest, and the header offers its + on hover as a full group's does; both run -/// New Workspace in Group, the one action path the group editor, menu, palette and CLI share. +/// Empty groups show only their header; its hover + keeps New Workspace in Group reachable. @MainActor @Suite(.serialized, .closesItsWindows) struct SidebarEmptyGroupAddButtonTests { - private static let group = GroupID("grp-empty") + static let group = GroupID("grp-empty") /// A sidebar in a window with one open, empty group; `ran` collects the group actions it runs. - private static func sidebar() -> (SidebarView, NSWindow, Ran) { + static func sidebar() -> (SidebarView, NSWindow, Ran) { let machine = SidebarMachine(id: .local, name: "This Mac", kind: .local) let empty = SidebarGroup(id: group, name: "Sleep alarm", color: .purple, workspaces: []) let loose = SidebarWorkspace(id: WorkspaceID("loose"), title: "loose") @@ -39,21 +36,40 @@ import Testing !view.isHidden && view.alphaValue > 0 && view.frame.width > 0 && row.bounds.contains(view.frame) } - @Test func theEmptyRowShowsAPlusAtRestThatMakesAWorkspaceInTheGroup() throws { - let (sidebar, window, ran) = Self.sidebar() + @Test func theEmptyGroupShowsOnlyItsHeaderAtRest() throws { + let (sidebar, window, _) = Self.sidebar() + defer { window.close() } + let header = try #require(sidebar.list.rowViews[.group(Self.group)] as? GroupHeaderRowView) + header.isHovered = false + header.layoutSubtreeIfNeeded() + #expect(sidebar.list.rowViews[.emptyGroup(Self.group)] == nil) + #expect(header.addButton.isHidden) + #expect(!header.isDropTarget) + } + + @Test func theEmptyHeaderHighlightsOnlyDuringADropIntoIt() throws { + let (sidebar, window, _) = Self.sidebar() defer { window.close() } - let row = try #require(sidebar.list.rowViews[.emptyGroup(Self.group)], "the open empty group shows its row") - sidebar.list.wire(row, key: .emptyGroup(Self.group)) - row.isHovered = false - row.layoutSubtreeIfNeeded() - let plus = try #require(Self.views(in: row).first { $0.accessibilityIdentifier() == "cmux.sidebar.group.grp-empty.emptyAdd" } as? NSButton, - "the empty group's row has no + button") - #expect(Self.drawn(plus, in: row), "the + does not show at rest (user 17912)") - #expect(plus.accessibilityLabel() == Strings.newWorkspace) - let center = row.convert(NSPoint(x: plus.frame.midX, y: plus.frame.midY), to: row.superview) - #expect(row.hitTest(center) === plus, "the + takes no click") - plus.performClick(nil) - #expect(ran.items == ["grp-empty workspaceGroup.newWorkspace"], "the + runs New Workspace in Group") + let list = sidebar.list + let header = try #require(list.rowViews[.group(Self.group)] as? GroupHeaderRowView) + let source = try #require(list.displayed.row(for: .workspace(WorkspaceID("loose")))) + let press = SidebarPress(key: source.key, point: NSPoint(x: 80, y: source.y + source.height / 2)) + list.beginDrag(press) + let drag = try #require(list.drag) + defer { + list.cancelDrag() + if let landing = list.landing { list.finishLand(landing, then: nil) } + } + let row = try #require(list.displayed.row(for: .group(Self.group))) + let y = row.y + row.height / 2 + drag.target = DropResolver.resolve(y: y, payload: .workspaces([WorkspaceID("loose")]), + base: list.displayed, sections: list.model.sections) + list.syncRowStates() + #expect(header.isDropTarget) + drag.target = nil + list.syncRowStates() + #expect(!header.isDropTarget) + #expect(list.rowViews[.emptyGroup(Self.group)] == nil) } @Test func theEmptyGroupsHeaderOffersItsPlusOnHover() throws { diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/SidebarEmptyGroupGalleryTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/SidebarEmptyGroupGalleryTests.swift new file mode 100644 index 000000000000..5fbe56693c78 --- /dev/null +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/SidebarEmptyGroupGalleryTests.swift @@ -0,0 +1,100 @@ +import AppKit +@testable import CmuxNextDesign +@testable import CmuxNextSidebar +import CmuxNextTestSupport +import CmuxTheme +import Foundation +import Testing + +/// Native design comparison, with synthetic data. The chosen header-only version is the live list; +/// the other two are gallery-only reconstructions, never product settings. Artifacts are 2x. +@MainActor @Suite(.serialized, .closesItsWindows) struct SidebarEmptyGroupGalleryTests { + enum Version: String, CaseIterable { case placeholder, headerOnly, alwaysAdd } + enum State: String, CaseIterable { case rest, hover, drop } + + static func render(_ version: Version, state: State, dark: Bool) throws -> CGImage { + Motion.reduceMotionOverride = true + defer { Motion.reduceMotionOverride = nil } + let (sidebar, window, _) = SidebarEmptyGroupAddButtonTests.sidebar() + defer { window.close() } + let theme = dark ? "Soft Blue" : "Soft Blue Light" + window.appearance = NSAppearance(named: dark ? .darkAqua : .aqua) + let root = NSView(frame: sidebar.frame) + root.wantsLayer = true + root.appearance = window.appearance + window.contentView = root + root.addSubview(sidebar) + let input = try AANestedGroupsGalleryTests.input(theme) + _ = ThemeScope.app.store.apply(input) + let scope = ThemeScope(level: .room) + scope.setOverride(ThemeSpec(theme), input: input, animated: false) + scope.root(root) + root.layer?.backgroundColor = scope.perform { Palette.windowBackground.withAlphaComponent(1) }.cgColor + window.orderFrontRegardless() + sidebar.renderNow() + let list = sidebar.list + let group = SidebarEmptyGroupAddButtonTests.group + let header = try #require(list.rowViews[.group(group)] as? GroupHeaderRowView) + header.isHovered = state == .hover + header.isDropTarget = state == .drop + for view in AANestedGroupsGalleryTests.all(root) { + view.layoutSubtreeIfNeeded() + view.needsDisplay = true + view.displayIfNeeded() + } + if version == .placeholder { + // Reconstruct the previous native member-sized row without restoring it to product layout. + let height = SidebarLayoutMetrics.standard.height(for: WorkspaceRowContent()) + let gap = SidebarLayoutMetrics.standard.rowSpacing + for (key, row) in list.rowViews where key != .group(group) && row.frame.minY >= header.frame.maxY { + row.frame.origin.y += height + gap + } + let row = EmptyGroupRowView(key: .emptyGroup(group)) + row.onAdd = {} + row.configure(groupID: group, groupName: "Sleep alarm") + row.frame = NSRect(x: header.frame.minX, y: header.frame.maxY + gap, width: header.frame.width, height: height) + list.addSubview(row) + row.layoutSubtreeIfNeeded() + row.displayIfNeeded() + } else if version == .alwaysAdd { + // Compare a permanent header + against the chosen reveal-on-hover control. + header.addButton.isHidden = false + header.addButton.alphaValue = 1 + } + CATransaction.flush() + let rep = try #require(NSBitmapImageRep(bitmapDataPlanes: nil, pixelsWide: Int(root.bounds.width * 2), + pixelsHigh: Int(root.bounds.height * 2), bitsPerSample: 8, + samplesPerPixel: 4, hasAlpha: true, isPlanar: false, + colorSpaceName: .deviceRGB, bytesPerRow: 0, bitsPerPixel: 0)) + rep.size = root.bounds.size + root.cacheDisplay(in: root.bounds, to: rep) + return try #require(rep.cgImage) + } + + @Test(.enabled(if: ProcessInfo.processInfo.environment["CMUX_TEST_REQUIRE_GUI"] == "1")) + func renderThreeVersions() throws { + let root = AANestedGroupsGalleryTests.packageRoot + let out = root.appending(path: "sidebar-empty-group-gallery") + try FileManager.default.createDirectory(at: out, withIntermediateDirectories: true) + var html = "Empty group designs

Empty group designs

Chosen: headerOnly. No extra row; + on hover; group highlight during a drop.

" + for dark in [false, true] { + for state in State.allCases { + html += "

\(dark ? "Dark" : "Light") — \(state.rawValue)

" + for version in Version.allCases { + let name = "\(version.rawValue)-\(state.rawValue)-\(dark ? "dark" : "light").png" + try AAGroupHeaderDensityGalleryTests.write(try Self.render(version, state: state, dark: dark), to: out.appending(path: name)) + html += "
\(version.rawValue)
" + } + html += "
" + } + } + try html.write(to: out.appending(path: "index.html"), atomically: true, encoding: .utf8) + let tar = Process() + tar.executableURL = URL(fileURLWithPath: "/usr/bin/tar") + tar.currentDirectoryURL = root + tar.arguments = ["cf", "sidebar-empty-group-gallery.tar", "sidebar-empty-group-gallery"] + try tar.run() + tar.waitUntilExit() + #expect(tar.terminationStatus == 0) + } +} diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/SidebarEmptyGroupTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/SidebarEmptyGroupTests.swift index 43e870e0ff45..5e636e5477b0 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/SidebarEmptyGroupTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/SidebarEmptyGroupTests.swift @@ -3,10 +3,8 @@ import CmuxNextDesign import CoreGraphics import Testing -/// An expanded group with no workspaces (user 9757, "if group has no workspaces, we need better empty -/// space"): under its header the list keeps one quiet row, as tall and as far in as a member row, so -/// the group reads as open and empty, and a workspace dragged over that row drops into the group. -/// A collapsed empty group stays a header alone. +/// An empty group keeps only its header. Creation stays on the header's hover +, +/// and dropping a workspace on that header still inserts it into the group. @MainActor struct SidebarEmptyGroupTests { private let scratch = GroupID("g_scratch"), perf = GroupID("g_perf") private let machine = SidebarMachine(id: .local, name: "This Mac", kind: .local) @@ -34,18 +32,13 @@ import Testing return next.group == group ? next : nil } - @Test func anOpenEmptyGroupKeepsOneMemberSizedRowUnderItsHeader() throws { + @Test func anOpenEmptyGroupKeepsOnlyItsHeader() throws { let layout = layout(sections()) let header = try #require(layout.row(for: .group(scratch))) - let placeholder = try #require(rowUnderHeader(layout, scratch), "the empty group shows a row under its header") - let memberHeight = SidebarLayoutMetrics.standard.height(for: WorkspaceRowContent()) - #expect(placeholder.height == memberHeight, "as tall as a one-line member row") - #expect(placeholder.depth == header.depth + 1, "one level in, as a member") - #expect(placeholder.y == header.maxY + SidebarLayoutMetrics.standard.rowSpacing) - #expect(placeholder.isLastInGroup) - // The next group starts one row gap after the empty row, not right under the header. + #expect(rowUnderHeader(layout, scratch) == nil) + #expect(layout.row(for: .emptyGroup(scratch)) == nil) let next = try #require(layout.row(for: .group(perf))) - #expect(next.y == placeholder.maxY + SidebarLayoutMetrics.standard.rowSpacing) + #expect(next.y == header.maxY + SidebarLayoutMetrics.standard.rowSpacing) } @Test func aCollapsedEmptyGroupStaysAHeaderAlone() { @@ -93,12 +86,12 @@ import Testing #expect(unfolded.totalHeight == open.totalHeight) } - /// A loose workspace dragged over the empty row drops into the empty group, at its start. - @Test func aDropOnTheEmptyRowGoesIntoTheGroup() throws { + /// A loose workspace dragged over the empty header drops into the group, at its start. + @Test func aDropOnTheEmptyHeaderGoesIntoTheGroup() throws { let sections = sections() let layout = layout(sections) - let placeholder = try #require(rowUnderHeader(layout, scratch)) - let y = placeholder.y + placeholder.height / 2 + let header = try #require(layout.row(for: .group(scratch))) + let y = header.y + header.height / 2 let resolved = try #require(DropResolver.resolveDrag(centreY: y, leadingY: y, gapY: nil, gapHeight: 0, payload: .workspaces([WorkspaceID("loose")]), base: layout, sections: sections)) let section = try #require(sections.first?.id) diff --git a/plans/cmux-next/sidebar-owner.md b/plans/cmux-next/sidebar-owner.md index 74ab14817091..0b5697fde234 100644 --- a/plans/cmux-next/sidebar-owner.md +++ b/plans/cmux-next/sidebar-owner.md @@ -906,3 +906,9 @@ Commits logged without a trailer (behavior fixes, no visual change; the card kee to 14 pt (~13 pt visible ring), centered within the same trailing cell. Blue dots, working spinners, other status glyphs, row spacing and the shared trailing column stay unchanged. The row-badge regression measures the error frame and its unchanged cell center (`SidebarRowCornerBadgeTests`). + +## Empty groups keep just their header (2026-11-01) + +Leo: "what do you feel about this placeholder situation? idk doesn't that plceholder text seem a bit wack?" + +An empty group now keeps only its header at rest. The header's existing + appears on hover or keyboard focus, and New Workspace in Group remains available through accessibility and the group menu. A workspace dragged into the empty group highlights its header; no permanent "No workspaces · drag one here" row is projected by either full layout or fast fold. This replaces the placeholder and always-visible row + from 0105692a41d / user 17912; creation and drop capabilities remain. `SidebarEmptyGroupTests` and `SidebarEmptyGroupAddButtonTests` cover layout, folding, drop targeting and creation. `SidebarEmptyGroupGalleryTests` compares the old placeholder, the chosen header-only design, and a permanent header +, each at rest, hover and drop in light/dark at 2x. Chosen: header-only, because it keeps group names easy to scan and reveals the action when needed. Design spec: leo-workspace `design/sidebar.md`.