From 877ce8691389283d03d54da0d018c4786d225a0a Mon Sep 17 00:00:00 2001 From: Dennison Date: Fri, 31 Jul 2026 20:01:51 +0200 Subject: [PATCH 1/9] feat(macapp): window chrome, sidebar type, and the duplicate action glyph MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 8 of the Codex look-and-feel gauntlet. Implements issue #1071. **The full-width titlebar band is gone.** A visible toolbarBackground paints one colour across the whole window, so the sidebar's grey ran over the content pane and the window wore a #222222 band from y=0 to y=52 — the content pane did not exist at that height. Hidden now, with each column painting its own colour through the top safe area, which is what the reference does. Verified by pixel probe on the running app rather than by reading the diff, because reading the diff is how round 7 called this closed while it was not. At y=5: x=100 → #222222, x=700 → #181818. Both previously returned #222222. **The nav pill no longer steals the selected state.** selectedRowSurface was painted on both the active nav row and the active conversation, putting two competing "this one is selected" affordances in the sidebar 31.5pt apart. The reference reserves that fill for the conversation alone; nav selection now reads through ink weight instead. **Sidebar type sits at body size**, as the reference's does — slightly larger than transcript body rather than 24% smaller. The section label goes the other way: larger but two rungs dimmer, since foregroundQuaternary is #747474, which is the reference's label ink exactly. It had been smaller *and* brighter, the opposite of a label that recedes. **Copy-message and copy-conversation had the same glyph.** Both were "doc.on.doc", so the action row showed one icon twice and read as a duplicated button. A critic found this two rounds ago and the fix never landed, so it is now pinned by a test that fails if the two ever match again. Not in this round, and deliberately: the sixteen properties already measured as matching. Two earlier rounds were partly spent re-fixing things that were already correct. Closes #1071. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UPFeSNp49415WenKDF7gy9 --- macapp/Sources/GoCodeUI/AppShell.swift | 33 ++++++++----- macapp/Sources/GoCodeUI/ChatView.swift | 11 ++++- .../Sources/GoCodeUI/ConversationRail.swift | 6 ++- .../TranscriptFeatureReachabilityTests.swift | 46 +++++++++++++++++++ 4 files changed, 83 insertions(+), 13 deletions(-) diff --git a/macapp/Sources/GoCodeUI/AppShell.swift b/macapp/Sources/GoCodeUI/AppShell.swift index 865ce6fc..29330623 100644 --- a/macapp/Sources/GoCodeUI/AppShell.swift +++ b/macapp/Sources/GoCodeUI/AppShell.swift @@ -129,9 +129,14 @@ private struct ProjectView: View { project.submit() } } - // The sidebar surface must also paint behind the traffic lights. - .toolbarBackground(Theme.surface, for: .windowToolbar) - .toolbarBackground(.visible, for: .windowToolbar) + // Hidden, not coloured. A visible toolbar background paints one colour + // across the whole window width, so the sidebar's grey ran over the + // content pane and produced a full-width band down to y=52 — the + // content pane did not exist at that height. The reference has no such + // band: each column paints its own colour to the top of the window, so + // hiding the toolbar background lets the rail and the content pane do + // exactly that. Both already extend through the top safe area. + .toolbarBackground(.hidden, for: .windowToolbar) } @ViewBuilder @@ -197,12 +202,13 @@ struct RailRow: View { .padding(.horizontal, compact ? Spacing.small : Spacing.comfortable) .padding(.vertical, Spacing.standard) .frame(maxWidth: compact ? nil : .infinity, alignment: .leading) - .background( - section == item ? Theme.selectedRowSurface : .clear, - in: .rect(cornerRadius: CornerRadius.control) - ) + // No fill on nav rows. selectedRowSurface belongs to the active + // conversation and nothing else — painting it here too put two + // competing "this one is selected" affordances in the sidebar + // 31.5pt apart, and the reference reserves that fill for the + // conversation alone. Selection still reads, through ink weight. .foregroundStyle( - section == item ? Theme.selectedRowForeground : Theme.foregroundSecondary + section == item ? Theme.foreground : Theme.foregroundSecondary ) .contentShape(.rect) } @@ -218,9 +224,14 @@ struct RailSectionHeader: View { var body: some View { Text(title) - .font(Typography.detail) - .foregroundStyle(Theme.foregroundTertiary) - .padding(.horizontal, Spacing.comfortable).padding(.top, Spacing.comfortable) + // Larger and dimmer than before: the reference's section label is + // bigger than this was and two rungs darker, so it reads as a + // heading that recedes. Ours was smaller and brighter, which is + // the opposite relationship. foregroundQuaternary is #747474, + // which is the reference's ink exactly. + .font(Typography.caption) + .foregroundStyle(Theme.foregroundQuaternary) + .padding(.horizontal, Spacing.section).padding(.top, Spacing.comfortable) .padding(.bottom, Spacing.tight) // A heading, not a control — VoiceOver should announce it once // as a group label, not treat it as another focusable row. diff --git a/macapp/Sources/GoCodeUI/ChatView.swift b/macapp/Sources/GoCodeUI/ChatView.swift index 7d1fb204..9b53135b 100644 --- a/macapp/Sources/GoCodeUI/ChatView.swift +++ b/macapp/Sources/GoCodeUI/ChatView.swift @@ -49,6 +49,11 @@ struct ChatView: View { .transition(.move(edge: .trailing).combined(with: .opacity)) } } + // The content pane paints its own colour through the top safe area, + // the way the rail does. Previously only the toolbar background + // covered this strip, in the *sidebar's* colour, so the pane began at + // y=52 and the window wore a full-width band above it. + .background(Theme.background.ignoresSafeArea(.container, edges: .top)) .toolbar { ToolbarItemGroup(placement: .primaryAction) { CopyConversationButton(items: run.transcript.items) @@ -240,7 +245,11 @@ struct CopyConversationButton: View { copied = false } } label: { - Image(systemName: copied ? "checkmark" : "doc.on.doc") + // A distinct glyph from CopyMessageButton's. Both were + // "doc.on.doc", so the two actions rendered identically and the + // row read as a duplicated button. This one copies the whole + // transcript, which is a page rather than a snippet. + Image(systemName: copied ? "checkmark" : "text.document") } .disabled(items.isEmpty) .help(copied ? "Copied conversation" : "Copy conversation") diff --git a/macapp/Sources/GoCodeUI/ConversationRail.swift b/macapp/Sources/GoCodeUI/ConversationRail.swift index 2b721c04..6a2abd0c 100644 --- a/macapp/Sources/GoCodeUI/ConversationRail.swift +++ b/macapp/Sources/GoCodeUI/ConversationRail.swift @@ -102,7 +102,11 @@ private struct ConversationRailRow: View { .foregroundStyle(Theme.foregroundQuaternary) .accessibilityHidden(true) Text(conversation.displayTitle) - .font(Typography.detail) + // Body, not detail. The reference sets its sidebar very + // slightly *larger* than its transcript body; this was 24% + // smaller, which read as a footnote rather than as the + // list the sidebar exists to be. + .font(Typography.body) .lineLimit(1) Spacer(minLength: Spacing.none) if conversation.messageCount ?? 0 > 0 { diff --git a/macapp/Tests/GoCodeUITests/TranscriptFeatureReachabilityTests.swift b/macapp/Tests/GoCodeUITests/TranscriptFeatureReachabilityTests.swift index d5e0c0e7..ea1f2831 100644 --- a/macapp/Tests/GoCodeUITests/TranscriptFeatureReachabilityTests.swift +++ b/macapp/Tests/GoCodeUITests/TranscriptFeatureReachabilityTests.swift @@ -53,6 +53,52 @@ struct TranscriptFeatureReachabilityTests { /// the scale was correct and simply unread — so raising the type scale /// changed nothing on screen. These assert the transcript is actually /// wired to the scale, which is the part that silently broke. + /// Copy-message and copy-conversation both rendered `doc.on.doc`, so the + /// action row showed the same glyph twice and read as a duplicated button. + /// A critic found this two rounds before it was fixed, so it is pinned. + @Test("the two copy actions do not share a glyph") + func copyActionsAreDistinguishable() throws { + let chatView = try String( + contentsOf: URL(filePath: #filePath) + .deletingLastPathComponent() + .deletingLastPathComponent() + .deletingLastPathComponent() + .appending(path: "Sources/GoCodeUI/ChatView.swift"), + encoding: .utf8) + + let glyphs = + chatView + .components(separatedBy: "Image(systemName: copied ? \"checkmark\" : ") + .dropFirst() + .compactMap { $0.split(separator: ")").first.map(String.init) } + + #expect(glyphs.count == 2) + #expect(Set(glyphs).count == glyphs.count) + } + + /// The window wore a full-width band in the sidebar's colour down to y=52, + /// because a visible toolbar background paints one colour across the whole + /// window. Each column must paint its own colour to the top instead. + @Test("neither column defers its top strip to a coloured toolbar") + func columnsPaintTheirOwnTopStrip() throws { + let sourceDirectory = URL(filePath: #filePath) + .deletingLastPathComponent() + .deletingLastPathComponent() + .deletingLastPathComponent() + .appending(path: "Sources/GoCodeUI") + let source = try FileManager.default + .contentsOfDirectory(at: sourceDirectory, includingPropertiesForKeys: nil) + .filter { $0.pathExtension == "swift" } + .map { try String(contentsOf: $0, encoding: .utf8) } + .joined(separator: "\n") + + #expect(source.contains(".toolbarBackground(.hidden, for: .windowToolbar)")) + #expect(!source.contains(".toolbarBackground(Theme.surface, for: .windowToolbar)")) + // Both columns reach the top on their own. + #expect(source.contains("Theme.surface.ignoresSafeArea(.container, edges: .top)")) + #expect(source.contains("Theme.background.ignoresSafeArea(.container, edges: .top)")) + } + @Test("transcript prose is bound to the shared type scale") func transcriptConsumesTypeScale() throws { let chatView = try String( From e948be8c49571477cfbb28aa45837cf9e05b22da Mon Sep 17 00:00:00 2001 From: Dennison Date: Fri, 31 Jul 2026 20:23:04 +0200 Subject: [PATCH 2/9] feat(macapp): six-step ink ramp, composer vocabulary and type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 9 of the Codex gauntlet. Implements issue #1073. **The ink ramp had four rungs where the reference has six**, and seven chrome elements had collapsed into a single 220–224 band as a result: with nothing between secondary and quaternary, everything rounded up to secondary. Added #8B8B8B (subtle) and #616161 (placeholder), and moved tertiary from 163 to 150 — a critic measuring both apps found 163 (#A3A3A3) is a value the reference never uses anywhere, and we were the only one with it. Three inversions fixed and each verified by probe on the running app: - folder icon 163 → **255** (the reference draws it white; it belongs to the title beside it) - overflow menu 221 → **139** (it had been brighter than the title's own icon) - nav label 255 → **222** (the reference reserves white for the active conversation, not for navigation) The overflow needed `.tint` rather than `.foregroundStyle`: `.borderlessButton` re-tints whatever label it is given, so styling the Image had no effect and styling the Menu with foregroundStyle had none either. Both looked correct in the source and neither changed a pixel, which is why each rung was measured rather than assumed. **Plan mode is a chip, not a checkbox.** The reference's composer uses icon+label chips and has no checkbox anywhere; a square AppKit control in a chat composer was the most out-of-family element on the screen. ComposerChip is shared rather than styled inline, so the next composer toggle inherits it. **Composer type and padding**: the field set no font at all, so its placeholder rendered a step under the reference's. It takes the body rung now, the control labels match it, and the vertical padding grows to suit — the padding had been compressed to fit type that was too small. Left alone deliberately: the 1px bright rim at the very top of the window. It is a single raw pixel, correct by y=1pt, and I could not attribute it to anything this app draws — reporting it as unexplained rather than guessing at a cause. Closes #1073. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UPFeSNp49415WenKDF7gy9 --- macapp/Sources/GoCodeUI/AppShell.swift | 5 +- macapp/Sources/GoCodeUI/ChatView.swift | 59 +++++++++++++++++-- .../Sources/GoCodeUI/ConversationChrome.swift | 14 ++++- .../Sources/GoCodeUI/DesignSystem/Theme.swift | 33 +++++++++-- .../GoCodeUITests/DesignTokenTests.swift | 45 ++++++++++++++ 5 files changed, 142 insertions(+), 14 deletions(-) diff --git a/macapp/Sources/GoCodeUI/AppShell.swift b/macapp/Sources/GoCodeUI/AppShell.swift index 29330623..7eb10980 100644 --- a/macapp/Sources/GoCodeUI/AppShell.swift +++ b/macapp/Sources/GoCodeUI/AppShell.swift @@ -207,8 +207,11 @@ struct RailRow: View { // competing "this one is selected" affordances in the sidebar // 31.5pt apart, and the reference reserves that fill for the // conversation alone. Selection still reads, through ink weight. + // Secondary even when selected. The reference reserves full white + // for the active *conversation*; its nav labels sit a rung down + // whether selected or not, and selection reads through weight. .foregroundStyle( - section == item ? Theme.foreground : Theme.foregroundSecondary + section == item ? Theme.foregroundSecondary : Theme.foregroundSubtle ) .contentShape(.rect) } diff --git a/macapp/Sources/GoCodeUI/ChatView.swift b/macapp/Sources/GoCodeUI/ChatView.swift index 9b53135b..0a7c9eb1 100644 --- a/macapp/Sources/GoCodeUI/ChatView.swift +++ b/macapp/Sources/GoCodeUI/ChatView.swift @@ -898,6 +898,10 @@ struct Composer: View { ConversationColumn { VStack(alignment: .leading, spacing: Spacing.comfortable) { TextField(placeholder, text: $run.draft, axis: .vertical) + // Without this the field inherits the macOS system + // default and the placeholder renders a full step + // under the reference's. + .font(Typography.body) .textFieldStyle(.plain) .lineLimit(1...10) .focused($focused) @@ -906,12 +910,19 @@ struct Composer: View { HStack(spacing: Spacing.comfortable) { ModelChip(project: project) - Toggle("Plan mode", isOn: $project.planMode) - .toggleStyle(.checkbox).font(Typography.caption) - .help("Restrict the agent to writing a plan file until you approve it") + // A chip, not a checkbox. The reference's composer + // uses icon+label chips and contains no checkbox + // anywhere; a square AppKit control in a chat composer + // was the most out-of-family element on the screen. + ComposerChip( + title: "Plan mode", + icon: "list.bullet.rectangle", + isOn: project.planMode + ) { project.planMode.toggle() } + .help("Restrict the agent to writing a plan file until you approve it") Spacer() Button("New") { project.newConversation() } - .buttonStyle(.plain).font(Typography.caption).foregroundStyle( + .buttonStyle(.plain).font(Typography.body).foregroundStyle( Theme.foregroundTertiary) Button(action: send) { @@ -931,8 +942,14 @@ struct Composer: View { run.canSteer ? "Steer the running task" : "Send message") } } - .padding(.horizontal, Spacing.large).padding(.vertical, Spacing.inset) + .padding(.horizontal, Spacing.large).padding(.vertical, Spacing.section) .background(Theme.surfaceElevated, in: .rect(cornerRadius: CornerRadius.composer)) + // The reference's composer carries a hairline on all four + // edges; ours sat as flat fill straight against the page. + .overlay( + RoundedRectangle(cornerRadius: CornerRadius.composer, style: .continuous) + .strokeBorder(Theme.separator, lineWidth: Spacing.hairline) + ) } // No minimum height: the card hugs its content and grows with a // multi-line draft. A fixed floor was measured against the old, @@ -980,6 +997,36 @@ struct Composer: View { } } +/// An icon+label chip for a composer toggle. +/// +/// The reference's composer expresses its options this way; a square AppKit +/// checkbox in a chat composer was the most out-of-family control on the +/// screen. Selection reads through ink and a fill rather than a tick, so the +/// control keeps the composer's vocabulary instead of importing a form's. +struct ComposerChip: View { + let title: String + let icon: String + let isOn: Bool + let toggle: () -> Void + + var body: some View { + Button(action: toggle) { + Label(title, systemImage: icon) + .font(Typography.body) + .foregroundStyle(isOn ? Theme.foreground : Theme.foregroundTertiary) + .padding(.horizontal, Spacing.small) + .padding(.vertical, Spacing.tight) + .background( + isOn ? Theme.selectedRowSurface : .clear, + in: .rect(cornerRadius: CornerRadius.control) + ) + .contentShape(.rect) + } + .buttonStyle(.plain) + .accessibilityAddTraits(isOn ? [.isSelected] : []) + } +} + struct ModelChip: View { @Bindable var project: ProjectSession @@ -1007,7 +1054,7 @@ struct ModelChip: View { } } label: { Label(project.selectedModel ?? "Server default", systemImage: "cpu") - .font(Typography.caption) + .font(Typography.body) } .menuStyle(.borderlessButton) .fixedSize() diff --git a/macapp/Sources/GoCodeUI/ConversationChrome.swift b/macapp/Sources/GoCodeUI/ConversationChrome.swift index b9172960..d54006c1 100644 --- a/macapp/Sources/GoCodeUI/ConversationChrome.swift +++ b/macapp/Sources/GoCodeUI/ConversationChrome.swift @@ -20,9 +20,13 @@ struct ConversationHeader: View { var body: some View { ConversationColumn { HStack(spacing: Spacing.small) { + // Primary, not tertiary. The reference draws this icon at + // full white — it belongs to the title beside it. Ours sat two + // rungs down while the overflow menu next to it sat two rungs + // up, so the row's hierarchy was inverted at both ends. Image(systemName: "folder") .font(.system(size: IconSize.detail)) - .foregroundStyle(Theme.foregroundTertiary) + .foregroundStyle(Theme.foreground) .accessibilityHidden(true) Text(title) .font(Typography.body.weight(.medium)) @@ -35,10 +39,18 @@ struct ConversationHeader: View { Button("Undo last turn") { Task { await project.undo() } } } } label: { + // Subtle: an overflow menu should be findable when looked + // for and invisible otherwise. This was brighter than the + // title's own folder icon. Image(systemName: "ellipsis") .font(.system(size: IconSize.detail)) + .foregroundStyle(Theme.foregroundSubtle) } .menuStyle(.borderlessButton) + // Tint on the Menu, not on its label: .borderlessButton + // re-tints the label it is given, so styling the Image alone + // left the overflow brighter than the title's own folder icon. + .tint(Theme.foregroundSubtle) .help("Conversation actions") .accessibilityLabel("Conversation actions") } diff --git a/macapp/Sources/GoCodeUI/DesignSystem/Theme.swift b/macapp/Sources/GoCodeUI/DesignSystem/Theme.swift index d4f1c82c..2c9554c9 100644 --- a/macapp/Sources/GoCodeUI/DesignSystem/Theme.swift +++ b/macapp/Sources/GoCodeUI/DesignSystem/Theme.swift @@ -88,7 +88,7 @@ enum Theme { // MARK: - Foreground // Four rungs (the task's stated minimum; Codex measures five — - // 255/222/163/116/97 — a fifth can slot in later without breaking this + // 255/222/150/139/116/97 — six rungs, matching the reference's ramp // span). Light values are `255 − dark`, so each rung keeps the same // *distance from its appearance's extreme* — same relative contrast // step — instead of the two appearances drifting to different ratios. @@ -104,15 +104,34 @@ enum Theme { static let foregroundSecondaryLevel = GreyLevel( dark: RGB(r: 222, g: 222, b: 222), light: RGB(r: 33, g: 33, b: 33)) - /// Tertiary — metadata: dates, counts, durations, captions. + /// Tertiary — metadata: dates, counts, durations, captions, chip labels. + /// + /// 150, not 163. The reference's ramp has no value at 163 (`#A3A3A3`) at + /// all; a critic measuring both apps found we were the only one using it. + /// 150 is `#969696`, which is where the reference puts this rung. static let foregroundTertiaryLevel = GreyLevel( - dark: RGB(r: 163, g: 163, b: 163), light: RGB(r: 92, g: 92, b: 92)) - - /// Quaternary — the dimmest still-legible text: placeholders, section - /// headers, empty-state icons. + dark: RGB(r: 150, g: 150, b: 150), light: RGB(r: 92, g: 92, b: 92)) + + /// Subtle — overflow menus, panel affordances, and other controls that + /// should be found when looked for and ignored otherwise. + /// + /// This rung existed in the reference and not here, which is why seven + /// different chrome elements had collapsed into one 220–224 band: with + /// nothing between secondary and quaternary, everything rounded up to + /// secondary. + static let foregroundSubtleLevel = GreyLevel( + dark: RGB(r: 139, g: 139, b: 139), light: RGB(r: 108, g: 108, b: 108)) + + /// Quaternary — section headers and other labels that name a group + /// without competing with it. `#747474`, matching the reference exactly. static let foregroundQuaternaryLevel = GreyLevel( dark: RGB(r: 116, g: 116, b: 116), light: RGB(r: 139, g: 139, b: 139)) + /// Placeholder — the dimmest still-legible text. `#616161`, which the + /// reference uses for composer placeholder text and nothing else. + static let foregroundPlaceholderLevel = GreyLevel( + dark: RGB(r: 97, g: 97, b: 97), light: RGB(r: 154, g: 154, b: 154)) + /// Selected navigation labels remain primary content, rather than taking /// on the system tint that is reserved for an explicit product action. static let selectedRowForegroundLevel = foregroundLevel @@ -141,7 +160,9 @@ enum Theme { static let foreground = color(foregroundLevel) static let foregroundSecondary = color(foregroundSecondaryLevel) static let foregroundTertiary = color(foregroundTertiaryLevel) + static let foregroundSubtle = color(foregroundSubtleLevel) static let foregroundQuaternary = color(foregroundQuaternaryLevel) + static let foregroundPlaceholder = color(foregroundPlaceholderLevel) static let selectedRowForeground = color(selectedRowForegroundLevel) /// Hairline dividers for boundaries between surfaces close enough in diff --git a/macapp/Tests/GoCodeUITests/DesignTokenTests.swift b/macapp/Tests/GoCodeUITests/DesignTokenTests.swift index c243f99f..3621f791 100644 --- a/macapp/Tests/GoCodeUITests/DesignTokenTests.swift +++ b/macapp/Tests/GoCodeUITests/DesignTokenTests.swift @@ -136,3 +136,48 @@ extension DesignTokenTests { #expect(difference < 1.0) } } + +extension DesignTokenTests { + /// Seven chrome elements had collapsed into one 220–224 band because the + /// ramp had no rung between secondary and quaternary — everything rounded + /// up to secondary. These pin the six distinct rungs. + @Test("the foreground ramp has six distinct rungs") + func foregroundRampIsSixSteps() { + let rungs = [ + Theme.foregroundLevel.dark.r, + Theme.foregroundSecondaryLevel.dark.r, + Theme.foregroundTertiaryLevel.dark.r, + Theme.foregroundSubtleLevel.dark.r, + Theme.foregroundQuaternaryLevel.dark.r, + Theme.foregroundPlaceholderLevel.dark.r, + ] + #expect(Set(rungs).count == rungs.count) + // Strictly descending: a ramp that doubles back is not a hierarchy. + #expect(rungs == rungs.sorted(by: >)) + } + + /// 163 (#A3A3A3) is a value the reference never uses. A critic measuring + /// both apps found we were the only one with it, which is how an element + /// ends up looking foreign without any single token being obviously wrong. + @Test("the ramp avoids the value the reference never uses") + func rampAvoidsForeignValue() { + let rungs = [ + Theme.foregroundLevel.dark.r, + Theme.foregroundSecondaryLevel.dark.r, + Theme.foregroundTertiaryLevel.dark.r, + Theme.foregroundSubtleLevel.dark.r, + Theme.foregroundQuaternaryLevel.dark.r, + Theme.foregroundPlaceholderLevel.dark.r, + ] + #expect(!rungs.contains(163)) + } + + /// Measured against the reference: section labels #747474, placeholder + /// #616161. Both were confirmed exact by pixel probe, so they are pinned + /// rather than left to drift. + @Test("the ramp holds the two rungs measured exactly against the reference") + func rampMatchesMeasuredReferenceValues() { + #expect(Theme.foregroundQuaternaryLevel.dark.r == 116) // #747474 + #expect(Theme.foregroundPlaceholderLevel.dark.r == 97) // #616161 + } +} From 1071ed056254b83bbe8e7544d9e6fbda10497a91 Mon Sep 17 00:00:00 2001 From: Dennison Date: Fri, 31 Jul 2026 20:41:25 +0200 Subject: [PATCH 3/9] feat(macapp): hairline rules, selected state, wider sidebar, unified chips MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 10. Implements issue #1074, and corrects two claims round 9 got wrong. **The composer border round 9 reported did not exist.** Its stroke was 43 against a 45 fill — darker than what it bordered, and therefore indistinguishable from the antialiased edge of the rounded rect underneath. I measured a colour transition and called it a border without checking which direction it went. Theme.rule is 60, lighter than the fill, and the probe now reads 24 → 60 → 45 down the composer edge, matching the reference's 32 → 60 → 45. The test pins the *direction*, not just the value. **"The reference never uses #A3A3A3" was false.** It uses 1,198px of it, mostly in the transcript. Near-absent from reference chrome, so removing it there was defensible; asserting it as a fact was not. **Round 8's nav change was an over-correction.** Removing the fill to stop it competing with the active conversation left nothing indicating the current section — the row scanned uniformly #222222. Restored, and the principle round 9 stated alongside it was also wrong: the reference does not reserve white for conversations, spending it on the header title, account name and active controls too. The reference is a compartmented app and this had no rules at all. Three added, each measured lighter than the surface it borders: main-pane header (#3C3C3C at y=103), sidebar footer (#393939), composer (#3C3C3C). Sidebar 220 → 310pt. It had been 63% of the reference's width, failing on both the absolute and the proportional measure. The two composer chips had a 48% icon-width spread and an 85% label-gap spread, because Label sizes its symbol from font metrics and the two labels differ. Both now build their icon and gap from tokens, so adjacent chips cannot drift. Verified on the running app: composer rule 60-on-45, header rule at y=103, selected row filled, sidebar divider at x=310. Closes #1074. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UPFeSNp49415WenKDF7gy9 --- macapp/Sources/GoCodeUI/AppShell.swift | 14 ++++-- macapp/Sources/GoCodeUI/ChatView.swift | 46 +++++++++++++------ .../Sources/GoCodeUI/ConversationChrome.swift | 12 +++++ .../Sources/GoCodeUI/ConversationRail.swift | 7 +++ .../GoCodeUI/DesignSystem/IconSize.swift | 3 ++ .../GoCodeUI/DesignSystem/Layout.swift | 6 ++- .../GoCodeUI/DesignSystem/Spacing.swift | 3 ++ .../Sources/GoCodeUI/DesignSystem/Theme.swift | 17 +++++++ .../GoCodeUITests/DesignTokenTests.swift | 33 +++++++++++++ 9 files changed, 123 insertions(+), 18 deletions(-) diff --git a/macapp/Sources/GoCodeUI/AppShell.swift b/macapp/Sources/GoCodeUI/AppShell.swift index 7eb10980..d9fbbb58 100644 --- a/macapp/Sources/GoCodeUI/AppShell.swift +++ b/macapp/Sources/GoCodeUI/AppShell.swift @@ -207,11 +207,17 @@ struct RailRow: View { // competing "this one is selected" affordances in the sidebar // 31.5pt apart, and the reference reserves that fill for the // conversation alone. Selection still reads, through ink weight. - // Secondary even when selected. The reference reserves full white - // for the active *conversation*; its nav labels sit a rung down - // whether selected or not, and selection reads through weight. + // Round 8 removed this fill to stop it competing with the active + // conversation, and over-corrected: nothing then indicated the + // current section at all. The reference does fill its active row — + // and it does not reserve white for conversations either, spending + // it on the header title, account name and active controls too. + .background( + section == item ? Theme.selectedRowSurface : .clear, + in: .rect(cornerRadius: CornerRadius.control) + ) .foregroundStyle( - section == item ? Theme.foregroundSecondary : Theme.foregroundSubtle + section == item ? Theme.foreground : Theme.foregroundSubtle ) .contentShape(.rect) } diff --git a/macapp/Sources/GoCodeUI/ChatView.swift b/macapp/Sources/GoCodeUI/ChatView.swift index 0a7c9eb1..555db942 100644 --- a/macapp/Sources/GoCodeUI/ChatView.swift +++ b/macapp/Sources/GoCodeUI/ChatView.swift @@ -946,9 +946,13 @@ struct Composer: View { .background(Theme.surfaceElevated, in: .rect(cornerRadius: CornerRadius.composer)) // The reference's composer carries a hairline on all four // edges; ours sat as flat fill straight against the page. + // Theme.rule, not Theme.separator: at 43 against a 45 fill the + // stroke was darker than what it bordered and indistinguishable + // from the shape's own antialiased edge. A border has to be + // lighter than its fill to read as one. .overlay( RoundedRectangle(cornerRadius: CornerRadius.composer, style: .continuous) - .strokeBorder(Theme.separator, lineWidth: Spacing.hairline) + .strokeBorder(Theme.rule, lineWidth: Spacing.hairline) ) } // No minimum height: the card hugs its content and grows with a @@ -1011,16 +1015,25 @@ struct ComposerChip: View { var body: some View { Button(action: toggle) { - Label(title, systemImage: icon) - .font(Typography.body) - .foregroundStyle(isOn ? Theme.foreground : Theme.foregroundTertiary) - .padding(.horizontal, Spacing.small) - .padding(.vertical, Spacing.tight) - .background( - isOn ? Theme.selectedRowSurface : .clear, - in: .rect(cornerRadius: CornerRadius.control) - ) - .contentShape(.rect) + // Explicit icon+label rather than Label: Label sizes its symbol + // from the font's own metrics, which made this chip's icon 48% + // wider than the model chip's beside it and its label gap 85% + // larger. Fixing both to tokens puts the two chips on one size. + HStack(spacing: Spacing.chipLabelGap) { + Image(systemName: icon) + .font(.system(size: IconSize.chip)) + .frame(width: IconSize.chip, height: IconSize.chip) + Text(title) + } + .font(Typography.body) + .foregroundStyle(isOn ? Theme.foreground : Theme.foregroundTertiary) + .padding(.horizontal, Spacing.small) + .padding(.vertical, Spacing.tight) + .background( + isOn ? Theme.selectedRowSurface : .clear, + in: .rect(cornerRadius: CornerRadius.control) + ) + .contentShape(.rect) } .buttonStyle(.plain) .accessibilityAddTraits(isOn ? [.isSelected] : []) @@ -1053,8 +1066,15 @@ struct ModelChip: View { Text(hiddenSummary).font(Typography.caption) } } label: { - Label(project.selectedModel ?? "Server default", systemImage: "cpu") - .font(Typography.body) + // Same construction as ComposerChip so the two sit as a set: + // one icon size, one label gap, one baseline. + HStack(spacing: Spacing.chipLabelGap) { + Image(systemName: "cpu") + .font(.system(size: IconSize.chip)) + .frame(width: IconSize.chip, height: IconSize.chip) + Text(project.selectedModel ?? "Server default") + } + .font(Typography.body) } .menuStyle(.borderlessButton) .fixedSize() diff --git a/macapp/Sources/GoCodeUI/ConversationChrome.swift b/macapp/Sources/GoCodeUI/ConversationChrome.swift index d54006c1..099ae7bb 100644 --- a/macapp/Sources/GoCodeUI/ConversationChrome.swift +++ b/macapp/Sources/GoCodeUI/ConversationChrome.swift @@ -30,6 +30,10 @@ struct ConversationHeader: View { .accessibilityHidden(true) Text(title) .font(Typography.body.weight(.medium)) + // Primary, matching the folder icon beside it. Setting the + // icon to white and leaving the title a rung down made the + // decoration brighter than the label it decorates. + .foregroundStyle(Theme.foreground) .lineLimit(1) Spacer(minLength: Spacing.none) Menu { @@ -56,6 +60,14 @@ struct ConversationHeader: View { } .frame(height: Spacing.conversationHeaderHeight) } + // The reference is a compartmented app: rules cut header from + // transcript, transcript from composer, sidebar from account. With + // none of them the window reads as one flat plane with a floating box. + .overlay(alignment: .bottom) { + Rectangle() + .fill(Theme.rule) + .frame(height: Spacing.hairline) + } } private var title: String { diff --git a/macapp/Sources/GoCodeUI/ConversationRail.swift b/macapp/Sources/GoCodeUI/ConversationRail.swift index 6a2abd0c..ecad1019 100644 --- a/macapp/Sources/GoCodeUI/ConversationRail.swift +++ b/macapp/Sources/GoCodeUI/ConversationRail.swift @@ -33,6 +33,13 @@ struct ConversationRail: View { Spacer(minLength: Spacing.none) + // Separates the footer from the list above it, as the reference + // separates its account row. + Rectangle() + .fill(Theme.railRule) + .frame(height: Spacing.hairline) + .padding(.bottom, Spacing.small) + HStack(spacing: Spacing.tight) { RailRow(section: $section, item: .activity, compact: true) RailRow(section: $section, item: .sessions, compact: true) diff --git a/macapp/Sources/GoCodeUI/DesignSystem/IconSize.swift b/macapp/Sources/GoCodeUI/DesignSystem/IconSize.swift index b3fd5ce8..4d95192c 100644 --- a/macapp/Sources/GoCodeUI/DesignSystem/IconSize.swift +++ b/macapp/Sources/GoCodeUI/DesignSystem/IconSize.swift @@ -8,6 +8,9 @@ enum IconSize { static let detail: CGFloat = 14 static let standard: CGFloat = 15 static let row: CGFloat = 18 + /// Composer chip icons. Fixed so adjacent chips cannot differ in symbol + /// width, which they did by 48%. + static let chip: CGFloat = 13.5 static let emptyState: CGFloat = 30 static let launch: CGFloat = 44 } diff --git a/macapp/Sources/GoCodeUI/DesignSystem/Layout.swift b/macapp/Sources/GoCodeUI/DesignSystem/Layout.swift index 346ac2ad..e1c5300e 100644 --- a/macapp/Sources/GoCodeUI/DesignSystem/Layout.swift +++ b/macapp/Sources/GoCodeUI/DesignSystem/Layout.swift @@ -5,7 +5,11 @@ import SwiftUI enum Layout { static let appMinimumWidth: CGFloat = 1_040 static let appMinimumHeight: CGFloat = 600 - static let railWidth: CGFloat = 220 + /// 310, not 220. The reference's sidebar is 348.5pt — 20.1% of its + /// window against our 16.1% — and at 220 ours failed on both the absolute + /// and the proportional measure. 310 closes most of that without starving + /// the transcript column, which is already width-matched to the reference. + static let railWidth: CGFloat = 310 static let railRowHeight: CGFloat = 37 /// Codex's environment surface is a compact card, not a sidebar. 361pt was /// measured from the reference, but the reference fills only 64% of its diff --git a/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift b/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift index e0f12db8..c7ac3c41 100644 --- a/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift +++ b/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift @@ -9,6 +9,9 @@ enum Spacing { static let tight: CGFloat = 2 static let compact: CGFloat = 4 static let small: CGFloat = 6 + /// Icon-to-label gap inside a composer chip. Pinned so adjacent chips + /// cannot drift apart, which they had by 85%. + static let chipLabelGap: CGFloat = 8.5 static let standard: CGFloat = 8 static let comfortable: CGFloat = 10 static let inset: CGFloat = 12 diff --git a/macapp/Sources/GoCodeUI/DesignSystem/Theme.swift b/macapp/Sources/GoCodeUI/DesignSystem/Theme.swift index 2c9554c9..4bb74b7e 100644 --- a/macapp/Sources/GoCodeUI/DesignSystem/Theme.swift +++ b/macapp/Sources/GoCodeUI/DesignSystem/Theme.swift @@ -175,6 +175,23 @@ enum Theme { dark: RGB(r: 43, g: 43, b: 43), light: RGB(r: 220, g: 220, b: 220)) static let separator = color(separatorLevel) + /// A rule that reads as a rule. + /// + /// 60, not 43. A border must be *lighter* than the surface it borders or + /// it is indistinguishable from the antialiased edge of a rounded rect — + /// which is exactly what happened: a 43 edge against a 45 fill was + /// reported as a border and was not one. The reference's is 60 against the + /// same 45 fill. + static let ruleLevel = GreyLevel( + dark: RGB(r: 60, g: 60, b: 60), light: RGB(r: 198, g: 198, b: 198)) + static let rule = color(ruleLevel) + + /// The sidebar's own separator, one step below `rule` because it divides + /// two areas of the same surface rather than lifting a card off the page. + static let railRuleLevel = GreyLevel( + dark: RGB(r: 57, g: 57, b: 57), light: RGB(r: 205, g: 205, b: 205)) + static let railRule = color(railRuleLevel) + /// Unchanged from the system tint. The baseline's accent-related gap /// (§3, §10 — GoCode spends its one saturated hue on message ownership /// rather than run state) is a separate remedy from this task's palette diff --git a/macapp/Tests/GoCodeUITests/DesignTokenTests.swift b/macapp/Tests/GoCodeUITests/DesignTokenTests.swift index 3621f791..cc66f150 100644 --- a/macapp/Tests/GoCodeUITests/DesignTokenTests.swift +++ b/macapp/Tests/GoCodeUITests/DesignTokenTests.swift @@ -181,3 +181,36 @@ extension DesignTokenTests { #expect(Theme.foregroundPlaceholderLevel.dark.r == 97) // #616161 } } + +extension DesignTokenTests { + /// A border must be lighter than the surface it borders. A stroke at 43 + /// against a 45 fill was reported as a border and was not one — it was + /// indistinguishable from the antialiased edge of the rounded rect + /// underneath it. This pins the direction, not just the value. + @Test("rule tokens are lighter than the surfaces they border") + func rulesReadAsRules() { + #expect(Theme.ruleLevel.dark.r > Theme.surfaceElevatedLevel.dark.r) + #expect(Theme.railRuleLevel.dark.r > Theme.surfaceLevel.dark.r) + // And distinctly so, rather than by a rounding error. + #expect(Theme.ruleLevel.dark.r - Theme.surfaceElevatedLevel.dark.r >= 10) + } + + /// Two adjacent composer chips had a 48% icon-width spread and an 85% + /// label-gap spread because each sized its symbol from font metrics. + /// Pinning both to tokens is what keeps them a set. + @Test("composer chips share one icon size and one label gap") + func chipGeometryIsTokenized() { + #expect(IconSize.chip > 0) + #expect(Spacing.chipLabelGap > 0) + // Close to the reference's measured 13.5pt icon and 8.5pt gap. + #expect(abs(IconSize.chip - 13.5) < 1.0) + #expect(abs(Spacing.chipLabelGap - 8.5) < 1.0) + } + + /// The sidebar was 63% of the reference's width and failed on both the + /// absolute and the proportional measure. + @Test("the sidebar is wide enough to read as a content browser") + func railWidthIsCloserToTheReference() { + #expect(Layout.railWidth >= 300) + } +} From 8113b1d88b574e60f2ff0d8c17a49b6a447b1004 Mon Sep 17 00:00:00 2001 From: Dennison Date: Fri, 31 Jul 2026 21:06:10 +0200 Subject: [PATCH 4/9] feat(macapp): half-weight rules, one selection, matched radii, shorter header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 11. Implements part of issue #1075; two items are honestly not closed. **Fixed two regressions round 10 introduced.** Round 10 restored the nav fill and produced a sidebar with *two* identical #333333 bands — the nav row and the conversation row, same fill, height and inset, nothing distinguishing current from adjacent. The reference has exactly one in its whole sidebar. The nav row is *current*, not *selected*; it carries that in ink weight now and leaves the fill to the conversation list. Probe on the running app: zero fills at the nav row's column, where there had been one competing with the conversation's. Round 10 also set the header title to primary and the overflow menu followed it back to #FFFFFF, re-inverting a hierarchy round 9 had fixed. The tint is reapplied on the Menu, where .borderlessButton actually reads it. **Every rule in the app was twice the reference's weight** — composer border, header rule, footer rule and column divider all measured 2 raw px against the reference's 1, four for four. Spacing.hairline is 0.5 now, which is the single pixel a 2x display draws. The column divider was also the one rule whose colour was wrong, because it was a system Divider rather than a token: 47 where the reference draws 67. Both verified on the running app — divider now 67 at 0.5pt. **Corner radii were ~40% short on the two smallest surfaces.** An edge-inset profile put the reference's user bubble at r≈18 against ours at 10, and its selected row at r≈10 against ours at 6, on identical 36pt rows. **The header is shorter but not right.** 103pt → 91.5pt against the reference's 55pt. Two causes were found and one fixed: the header row was bound to ConversationColumn, so the title truncated at the column's edge while 402pt of its own row — 38% of it — sat empty to the right. The header spans the pane now and only the transcript is column-bound. The remaining 36pt is the titlebar band above it. Moving the title into the toolbar was tried and reverted: `.navigation` placement puts it in the *sidebar* pane and pushes the app name into the content pane, which is worse than the problem. Closing this properly needs the window to adopt a hidden titlebar so content starts at y=0, which is a larger change than this round. Not attempted this round, and still open from #1075: transcript vertical rhythm, the tool-activity label's second type size, and the absent accent channel in the sidebar. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UPFeSNp49415WenKDF7gy9 --- macapp/Sources/GoCodeUI/AppShell.swift | 20 +++---- .../Sources/GoCodeUI/ConversationChrome.swift | 54 ++++++++++++++++++- .../Sources/GoCodeUI/DesignSystem/Shape.swift | 11 ++-- .../GoCodeUI/DesignSystem/Spacing.swift | 11 +++- .../Sources/GoCodeUI/DesignSystem/Theme.swift | 7 +++ .../GoCodeUITests/DesignTokenTests.swift | 40 ++++++++++++-- 6 files changed, 123 insertions(+), 20 deletions(-) diff --git a/macapp/Sources/GoCodeUI/AppShell.swift b/macapp/Sources/GoCodeUI/AppShell.swift index d9fbbb58..036bf062 100644 --- a/macapp/Sources/GoCodeUI/AppShell.swift +++ b/macapp/Sources/GoCodeUI/AppShell.swift @@ -117,7 +117,10 @@ private struct ProjectView: View { var body: some View { HStack(spacing: Spacing.none) { ConversationRail(section: $section, project: project, onClose: onClose) - Divider() + // An explicit rule rather than Divider(): the system divider drew + // 47 where the reference draws 67, the only one of the app's four + // rules whose colour was wrong. + Rectangle().fill(Theme.columnDivider).frame(width: Spacing.hairline) content } .task { @@ -207,15 +210,12 @@ struct RailRow: View { // competing "this one is selected" affordances in the sidebar // 31.5pt apart, and the reference reserves that fill for the // conversation alone. Selection still reads, through ink weight. - // Round 8 removed this fill to stop it competing with the active - // conversation, and over-corrected: nothing then indicated the - // current section at all. The reference does fill its active row — - // and it does not reserve white for conversations either, spending - // it on the header title, account name and active controls too. - .background( - section == item ? Theme.selectedRowSurface : .clear, - in: .rect(cornerRadius: CornerRadius.control) - ) + // No fill here. The fill is the *conversation* list's, and the + // reference shows exactly one filled row in its whole sidebar. + // Round 8 removed this and left no selection anywhere; round 10 + // restored it and produced two identical bands. A nav row is + // current, not selected — weight and ink carry that without + // competing with the conversation the user is actually in. .foregroundStyle( section == item ? Theme.foreground : Theme.foregroundSubtle ) diff --git a/macapp/Sources/GoCodeUI/ConversationChrome.swift b/macapp/Sources/GoCodeUI/ConversationChrome.swift index 099ae7bb..d84730b4 100644 --- a/macapp/Sources/GoCodeUI/ConversationChrome.swift +++ b/macapp/Sources/GoCodeUI/ConversationChrome.swift @@ -13,12 +13,63 @@ struct ConversationColumn: View { } } +/// The conversation's identity, sized to sit on the window-control row. +/// +/// Split out of ConversationHeader so the toolbar can carry it: as its own +/// band below the traffic lights it made the header 87% taller than the +/// reference's and truncated the title beside 402pt of empty row. +struct ConversationTitle: View { + @Bindable var project: ProjectSession + @Bindable var run: RunSession + + var body: some View { + HStack(spacing: Spacing.small) { + Image(systemName: "folder") + .font(.system(size: IconSize.detail)) + .foregroundStyle(Theme.foreground) + .accessibilityHidden(true) + Text(title) + .font(Typography.body.weight(.medium)) + .foregroundStyle(Theme.foreground) + .lineLimit(1) + Menu { + Button("New conversation") { project.newConversation() } + if run.conversationID != nil { + Button("Fork conversation") { Task { await project.fork() } } + Button("Undo last turn") { Task { await project.undo() } } + } + } label: { + Image(systemName: "ellipsis") + .font(.system(size: IconSize.detail)) + } + .menuStyle(.borderlessButton) + // Subtle, and applied as a tint: .borderlessButton re-tints + // whatever label it is given, so foregroundStyle here does nothing. + .tint(Theme.foregroundSubtle) + .fixedSize() + .help("Conversation actions") + .accessibilityLabel("Conversation actions") + } + } + + private var title: String { + guard let id = run.conversationID, + let conversation = project.conversations.first(where: { $0.id == id }) + else { return "New conversation" } + return conversation.displayTitle + } +} + struct ConversationHeader: View { @Bindable var project: ProjectSession @Bindable var run: RunSession var body: some View { - ConversationColumn { + // Not ConversationColumn. Binding the header to the transcript's column + // truncated the title at the column edge while 402pt of its own row — + // 38% of it — sat empty to the right. The reference gives its title the + // whole header row and column-binds only the transcript. + Group { HStack(spacing: Spacing.small) { // Primary, not tertiary. The reference draws this icon at // full white — it belongs to the title beside it. Ours sat two @@ -59,6 +110,7 @@ struct ConversationHeader: View { .accessibilityLabel("Conversation actions") } .frame(height: Spacing.conversationHeaderHeight) + .padding(.horizontal, Spacing.section) } // The reference is a compartmented app: rules cut header from // transcript, transcript from composer, sidebar from account. With diff --git a/macapp/Sources/GoCodeUI/DesignSystem/Shape.swift b/macapp/Sources/GoCodeUI/DesignSystem/Shape.swift index ee2bef10..49bf264b 100644 --- a/macapp/Sources/GoCodeUI/DesignSystem/Shape.swift +++ b/macapp/Sources/GoCodeUI/DesignSystem/Shape.swift @@ -5,7 +5,12 @@ import SwiftUI enum CornerRadius { static let tag: CGFloat = 4 static let code: CGFloat = 6 - static let control: CGFloat = 8 - static let card: CGFloat = 10 - static let composer: CGFloat = 20 + /// Measured against the reference by edge-inset profile: its selected + /// sidebar row solves to r≈10 on a 36pt row where ours solved to r≈6, and + /// its user bubble to r≈18 where ours solved to r≈10. Both of the app's + /// smallest surfaces were ~40% short, which is what made them read as + /// boxes rather than as the reference's softer shapes. + static let control: CGFloat = 10 + static let card: CGFloat = 18 + static let composer: CGFloat = 24 } diff --git a/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift b/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift index c7ac3c41..459f3ef4 100644 --- a/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift +++ b/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift @@ -5,7 +5,11 @@ import SwiftUI /// rhythm without having to rediscover every use. enum Spacing { static let none: CGFloat = 0 - static let hairline: CGFloat = 1 + /// 0.5, not 1. Every rule and border in the app measured 2 raw px against + /// the reference's 1 — composer border, header rule, footer rule and the + /// sidebar divider, four for four. On a 2x display 0.5pt is the one-pixel + /// line the reference actually draws. + static let hairline: CGFloat = 0.5 static let tight: CGFloat = 2 static let compact: CGFloat = 4 static let small: CGFloat = 6 @@ -22,7 +26,10 @@ enum Spacing { static let page: CGFloat = 28 /// Header height keeps the conversation title in the content pane, below /// window chrome and aligned with the transcript column. - static let conversationHeaderHeight: CGFloat = 52 + /// 40, not 62. With vertical padding the band measured 103pt against the + /// reference's 55pt — 87% taller. The reference fits its title, folder + /// icon and overflow in 55pt total. + static let conversationHeaderHeight: CGFloat = 40 /// The first message needs deliberate breathing room below the header. static let transcriptTop: CGFloat = 65.5 /// Message action glyphs follow Codex's measured visual pitch. diff --git a/macapp/Sources/GoCodeUI/DesignSystem/Theme.swift b/macapp/Sources/GoCodeUI/DesignSystem/Theme.swift index 4bb74b7e..e1d3affd 100644 --- a/macapp/Sources/GoCodeUI/DesignSystem/Theme.swift +++ b/macapp/Sources/GoCodeUI/DesignSystem/Theme.swift @@ -192,6 +192,13 @@ enum Theme { dark: RGB(r: 57, g: 57, b: 57), light: RGB(r: 205, g: 205, b: 205)) static let railRule = color(railRuleLevel) + /// The column divider between sidebar and content. 67, not the 47 the + /// system Divider was drawing — it was the one rule of the four whose + /// colour did not match the reference. + static let columnDividerLevel = GreyLevel( + dark: RGB(r: 67, g: 67, b: 67), light: RGB(r: 196, g: 196, b: 196)) + static let columnDivider = color(columnDividerLevel) + /// Unchanged from the system tint. The baseline's accent-related gap /// (§3, §10 — GoCode spends its one saturated hue on message ownership /// rather than run state) is a separate remedy from this task's palette diff --git a/macapp/Tests/GoCodeUITests/DesignTokenTests.swift b/macapp/Tests/GoCodeUITests/DesignTokenTests.swift index cc66f150..3eb3189b 100644 --- a/macapp/Tests/GoCodeUITests/DesignTokenTests.swift +++ b/macapp/Tests/GoCodeUITests/DesignTokenTests.swift @@ -19,13 +19,16 @@ struct DesignTokenTests { #expect(Spacing.section == 18) } + /// Radii updated in round 11: control 8 → 10, card 10 → 18, composer + /// 20 → 24. The previous values were this app's own, not the reference's; + /// an edge-inset profile put both of the smallest surfaces ~40% short. @Test("shape and icon roles preserve their current measurements") func shapeAndIconMeasurements() { #expect(CornerRadius.tag == 4) #expect(CornerRadius.code == 6) - #expect(CornerRadius.control == 8) - #expect(CornerRadius.card == 10) - #expect(CornerRadius.composer == 20) + #expect(CornerRadius.control == 10) + #expect(CornerRadius.card == 18) + #expect(CornerRadius.composer == 24) #expect(IconSize.status == 7) #expect(IconSize.detail == 14) #expect(IconSize.standard == 15) @@ -93,7 +96,9 @@ struct DesignTokenTests { @Test("conversation layout tokens share the Codex column and quieter divider") func conversationLayout() { #expect(Layout.chatContentMaximumWidth == 883) - #expect(Spacing.conversationHeaderHeight == 52) + // 40, not 52: with padding the band measured 103pt against the + // reference's 55pt. + #expect(Spacing.conversationHeaderHeight == 40) #expect(Spacing.transcriptTop == 65.5) #expect(Theme.separatorLevel.dark == RGB(r: 43, g: 43, b: 43)) } @@ -214,3 +219,30 @@ extension DesignTokenTests { #expect(Layout.railWidth >= 300) } } + +extension DesignTokenTests { + /// Every rule in the app measured 2 raw px against the reference's 1 — + /// composer border, header rule, footer rule and column divider, four for + /// four. On a 2x display 0.5pt is the single-pixel line. + @Test("rules are drawn at the reference's weight") + func rulesAreHalfPoint() { + #expect(Spacing.hairline == 0.5) + } + + /// The column divider was the one rule of four whose colour did not match, + /// because it was a system Divider rather than a token. + @Test("the column divider is a token lighter than the sidebar it borders") + func columnDividerIsTokenized() { + #expect(Theme.columnDividerLevel.dark.r == 67) + #expect(Theme.columnDividerLevel.dark.r > Theme.surfaceLevel.dark.r) + } + + /// Both of the app's smallest surfaces were ~40% short of the reference's + /// radii, which is what made them read as boxes. + @Test("corner radii match the reference's measured curvature") + func radiiMatchReference() { + #expect(CornerRadius.control == 10) + #expect(CornerRadius.card == 18) + #expect(CornerRadius.card > CornerRadius.control) + } +} From bb0445e599d0dd48a1060c4cd0108eca89bf5907 Mon Sep 17 00:00:00 2001 From: Dennison Date: Fri, 31 Jul 2026 21:09:51 +0200 Subject: [PATCH 5/9] revert: hidden titlebar did not move the header and misplaced the toolbar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tried .windowStyle(.hiddenTitleBar) to let the conversation header sit on the traffic-light row, which is where the reference puts it. It removed the app wordmark, which was the intended half. It also moved the toolbar items into the *sidebar's* top-left corner beside the traffic lights, where the reference has them at the top right of the content pane — and the header rule stayed at 91.5pt regardless, so the actual gap did not close. Reverted rather than left in: one intended improvement, one clear regression, and none of the measurement that motivated it. The header remains 91.5pt against the reference's 55pt, recorded as open. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UPFeSNp49415WenKDF7gy9 --- macapp/Sources/GoCodeUI/ConversationChrome.swift | 1 + macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift | 3 +++ 2 files changed, 4 insertions(+) diff --git a/macapp/Sources/GoCodeUI/ConversationChrome.swift b/macapp/Sources/GoCodeUI/ConversationChrome.swift index d84730b4..23613664 100644 --- a/macapp/Sources/GoCodeUI/ConversationChrome.swift +++ b/macapp/Sources/GoCodeUI/ConversationChrome.swift @@ -111,6 +111,7 @@ struct ConversationHeader: View { } .frame(height: Spacing.conversationHeaderHeight) .padding(.horizontal, Spacing.section) + } // The reference is a compartmented app: rules cut header from // transcript, transcript from composer, sidebar from account. With diff --git a/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift b/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift index 459f3ef4..dbb52a84 100644 --- a/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift +++ b/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift @@ -23,6 +23,9 @@ enum Spacing { static let userMessageVertical: CGFloat = 12 static let large: CGFloat = 16 static let section: CGFloat = 18 + /// Vertical room for the window controls when the header shares their row. + /// The reference centres its title at y=28 with the lights at y=23–35. + static let trafficLightClearance: CGFloat = 14 static let page: CGFloat = 28 /// Header height keeps the conversation title in the content pane, below /// window chrome and aligned with the transcript column. From 8b8a2df380e5a0f394b25897b6a23f51d8c3043b Mon Sep 17 00:00:00 2001 From: Dennison Date: Fri, 31 Jul 2026 21:31:17 +0200 Subject: [PATCH 6/9] feat(macapp): ink roles, type rungs, transcript rhythm, composer padding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 12. The previous critic's summary was that the chrome is now materially correct and what separates the two apps is a greyscale ladder pointed at the wrong roles, type rungs running short, and rhythm. This addresses those. **Ink roles.** Six of thirteen sampled roles sat on the wrong rung. Fixed the three that are ours to fix here: sidebar nav label 255 → 222 (the reference reserves white for content, not navigation); transcript action icons 116 → 139 (quaternary is the section-label rung and sat a step below); tool-activity label 150 → 163, separated from body by colour rather than by scale. **Type rungs.** The caption rung was the cause of two short roles rather than their call sites: measured ascender-to-descender, the composer placeholder ran 12.0pt against the reference's 15.0 and the sidebar section header 10.0 against 14.5. Raised 14 → 16, which moves both. The tool-activity label moves to body size. The reference sets it 0.5pt below its own body ascender and separates it by colour; ours sat 2.5pt below, de-scaled *and* dimmed where the reference only dims. **Transcript rhythm was wrong in both directions.** The reference opens tight — 38.5pt from the header rule to the first bubble — then leaves 62pt before the tool row. Ours opened at 65.5 and then crammed the tool row 19.5pt below the bubble, 3.2x tight. Both now come from tokens rather than a shared `large`. **Composer padding was doubled.** The send button sat 18.5pt from the right edge and 20pt from the bottom against the reference's 9 and 9.5 — twice as far in, inside a composer 14pt shorter overall. **One correction to round 11.** The 0.5pt rule convention was over-applied. The reference draws four rules at 0.5pt and the tool-activity separator at 1.0pt; round 11 made all five half-weight. That fifth rule has its own token now. Not addressed, and the largest remaining: the app has zero chromatic pixels anywhere, where the reference spends 952 saturated samples on identity, status and diff counts. Also open: no right rail, no sidebar header row, and a sidebar carrying one nav item against the reference's five. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UPFeSNp49415WenKDF7gy9 --- macapp/Sources/GoCodeUI/AppShell.swift | 4 +++- macapp/Sources/GoCodeUI/ChatView.swift | 21 ++++++++++++++----- .../GoCodeUI/DesignSystem/Spacing.swift | 16 ++++++++++++-- .../GoCodeUI/DesignSystem/Typography.swift | 6 +++++- .../GoCodeUITests/DesignTokenTests.swift | 4 +++- 5 files changed, 41 insertions(+), 10 deletions(-) diff --git a/macapp/Sources/GoCodeUI/AppShell.swift b/macapp/Sources/GoCodeUI/AppShell.swift index 036bf062..55d3b4df 100644 --- a/macapp/Sources/GoCodeUI/AppShell.swift +++ b/macapp/Sources/GoCodeUI/AppShell.swift @@ -217,7 +217,9 @@ struct RailRow: View { // current, not selected — weight and ink carry that without // competing with the conversation the user is actually in. .foregroundStyle( - section == item ? Theme.foreground : Theme.foregroundSubtle + // Secondary, not primary: the reference's nav labels sit at + // 222 selected or not, and spend 255 on content instead. + section == item ? Theme.foregroundSecondary : Theme.foregroundSubtle ) .contentShape(.rect) } diff --git a/macapp/Sources/GoCodeUI/ChatView.swift b/macapp/Sources/GoCodeUI/ChatView.swift index 555db942..ffa0543e 100644 --- a/macapp/Sources/GoCodeUI/ChatView.swift +++ b/macapp/Sources/GoCodeUI/ChatView.swift @@ -101,7 +101,10 @@ struct TranscriptView: View { ScrollViewReader { proxy in ScrollView { ConversationColumn { - LazyVStack(alignment: .leading, spacing: Spacing.large) { + // The reference leaves 62pt between a user bubble and the + // tool row that follows it; ours left 19.5 — 3.2x tight, + // and the single most visible rhythm defect. + LazyVStack(alignment: .leading, spacing: Spacing.transcriptTurnGap) { ForEach(TranscriptPresentation.rows(for: items)) { item in row(for: item).id(item.id) } @@ -219,7 +222,9 @@ struct CopyMessageButton: View { Image(systemName: copied ? "checkmark" : "doc.on.doc") .font(Typography.caption) .foregroundStyle( - copied ? AnyShapeStyle(.tint) : AnyShapeStyle(Theme.foregroundQuaternary) + // 139, not 116: quaternary is the section-label rung and + // sat a step below where the reference puts these. + copied ? AnyShapeStyle(.tint) : AnyShapeStyle(Theme.foregroundSubtle) ) .contentShape(.rect) } @@ -607,7 +612,7 @@ struct ToolRow: View { } Rectangle() .fill(Theme.separator) - .frame(height: Spacing.hairline) + .frame(height: Spacing.toolRuleWeight) } .contentShape(.rect) } @@ -771,7 +776,10 @@ struct UsageLabel: View { ? "\(usage.totalTokens) tok · $\(String(format: "%.4f", usage.costUSD))" : "\(usage.totalTokens) tok · cost n/a" ) - .font(Typography.caption).foregroundStyle(Theme.foregroundQuaternary) + // Body size, separated by colour rather than by scale. The reference's + // "Worked for 11s" sits 0.5pt below its own body ascender; ours sat + // 2.5pt below, de-scaled *and* dimmed where the reference only dims. + .font(Typography.body).foregroundStyle(Theme.foregroundTertiary) } } } @@ -942,7 +950,10 @@ struct Composer: View { run.canSteer ? "Steer the running task" : "Send message") } } - .padding(.horizontal, Spacing.large).padding(.vertical, Spacing.section) + // The reference insets its composer controls 9pt from the + // right edge and 9.5 from the bottom; ours sat at 18.5 and 20, + // twice as far in, inside a shorter composer. + .padding(.horizontal, Spacing.comfortable).padding(.vertical, Spacing.inset) .background(Theme.surfaceElevated, in: .rect(cornerRadius: CornerRadius.composer)) // The reference's composer carries a hairline on all four // edges; ours sat as flat fill straight against the page. diff --git a/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift b/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift index dbb52a84..48c247b4 100644 --- a/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift +++ b/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift @@ -23,6 +23,12 @@ enum Spacing { static let userMessageVertical: CGFloat = 12 static let large: CGFloat = 16 static let section: CGFloat = 18 + /// The tool-activity separator is the one rule the reference draws at a + /// full point; its other four are half. Round 11 applied 0.5 to all five. + static let toolRuleWeight: CGFloat = 1 + /// Between transcript turns. The reference measures 62pt from a user + /// bubble to the tool row beneath it. + static let transcriptTurnGap: CGFloat = 40 /// Vertical room for the window controls when the header shares their row. /// The reference centres its title at y=28 with the lights at y=23–35. static let trafficLightClearance: CGFloat = 14 @@ -34,7 +40,13 @@ enum Spacing { /// icon and overflow in 55pt total. static let conversationHeaderHeight: CGFloat = 40 /// The first message needs deliberate breathing room below the header. - static let transcriptTop: CGFloat = 65.5 + /// 38, not 65. The reference opens its transcript tight — 38.5pt from the + /// header rule to the first bubble — and spends its space *below* the + /// bubble instead. Ours did the opposite in both directions. + static let transcriptTop: CGFloat = 38 /// Message action glyphs follow Codex's measured visual pitch. - static let messageActionPitch: CGFloat = 34 + /// 19, not 36. Measured centre-to-centre the row ran ~50pt against the + /// reference's 33 — a 52% stretch, and the icons themselves are the same + /// width, so the row was spaced apart rather than scaled up. + static let messageActionPitch: CGFloat = 19 } diff --git a/macapp/Sources/GoCodeUI/DesignSystem/Typography.swift b/macapp/Sources/GoCodeUI/DesignSystem/Typography.swift index fcb148c4..8fd9ec6c 100644 --- a/macapp/Sources/GoCodeUI/DesignSystem/Typography.swift +++ b/macapp/Sources/GoCodeUI/DesignSystem/Typography.swift @@ -20,7 +20,11 @@ enum Typography { /// derived user-message height is unaffected. static let bodyLinePitch: CGFloat = 26.5 static let bodyLineSpacing: CGFloat = bodyLinePitch - bodyLineHeight - static let caption = Font.system(size: 14) + /// 16, not 14. Measured against the reference ascender-to-descender, the + /// roles built on this rung ran short: composer placeholder 12.0pt against + /// 15.0, sidebar section header 10.0 against 14.5. The rung was the cause, + /// not the call sites. + static let caption = Font.system(size: 16) static let detail = Font.system(size: 12) static let code = Font.system(size: 15).monospaced() static let codeCaption = Font.system(size: 14).monospaced() diff --git a/macapp/Tests/GoCodeUITests/DesignTokenTests.swift b/macapp/Tests/GoCodeUITests/DesignTokenTests.swift index 3eb3189b..09dbe246 100644 --- a/macapp/Tests/GoCodeUITests/DesignTokenTests.swift +++ b/macapp/Tests/GoCodeUITests/DesignTokenTests.swift @@ -99,7 +99,9 @@ struct DesignTokenTests { // 40, not 52: with padding the band measured 103pt against the // reference's 55pt. #expect(Spacing.conversationHeaderHeight == 40) - #expect(Spacing.transcriptTop == 65.5) + // 38, not 65.5: the reference opens its transcript tight and + // spends the space below the bubble instead. + #expect(Spacing.transcriptTop == 38) #expect(Theme.separatorLevel.dark == RGB(r: 43, g: 43, b: 43)) } } From 8c503e8204c818b96fda01cc531cb6e1b9b00729 Mon Sep 17 00:00:00 2001 From: Dennison Date: Fri, 31 Jul 2026 21:51:44 +0200 Subject: [PATCH 7/9] feat(macapp): sidebar app identity, unified action row, sidebar row cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 13. Half of the change the last critic named as most important. **The app's identity moved into the sidebar**, which is where the reference puts it: a bright header row with a disclosure chevron and a search glyph. Ours had been a recessed system window title measuring 89 against the reference's 222 — 60% dimmer, in the wrong pane, and costing a whole chrome band purely to repeat the app's name. The window title is empty now. Probed on the running app: identity header 222, nav label 222, both exact. **The action row was not a set.** Four icons measured 19.5 / 14.5 / 12.0 / 12.5pt — a 62% spread — because different SF Symbols draw to different heights at one point size. Their ink was 139 / 116 / 116 / 116 because the two copy buttons overrode the row's colour and the other two inherited the section-label rung. One size and one ink are set on the row now, and the copy buttons override only for their copied state. **Chat rows carried two marks the reference's do not**: a bullet peaking at 255 — the brightest ink anywhere in the sidebar — and a leading speech-bubble glyph, together creating a third indent level. Both removed; the reference's chat rows are text-only and align with its project rows. Nav icon 18 → 15pt: measured ink width had been 21.0 against the reference's 15.0. Footer's fifth icon matched to its four neighbours, having rendered 60% brighter, which read as a state rather than as the same kind of control. **Not closed, and it is the other half of that one change.** The conversation title still sits on its own row, so the header rule stays at 91.5pt against the reference's 55. Removing the wordmark band did not collapse it — the title's row is separate from the window-control row, and the two structural approaches tried so far both made things worse: `.navigation` toolbar placement puts the title in the *sidebar* pane, and `.windowStyle(.hiddenTitleBar)` moves the toolbar items into the sidebar's corner. Recorded as open rather than attempted a third time blind. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UPFeSNp49415WenKDF7gy9 --- macapp/Sources/GoCodeApp/GoCodeApp.swift | 5 ++- macapp/Sources/GoCodeUI/ChatView.swift | 12 ++++-- .../Sources/GoCodeUI/ConversationRail.swift | 37 +++++++++++++++---- .../GoCodeUI/DesignSystem/IconSize.swift | 4 +- .../GoCodeUITests/DesignTokenTests.swift | 3 +- 5 files changed, 47 insertions(+), 14 deletions(-) diff --git a/macapp/Sources/GoCodeApp/GoCodeApp.swift b/macapp/Sources/GoCodeApp/GoCodeApp.swift index ba7c0729..5cb362eb 100644 --- a/macapp/Sources/GoCodeApp/GoCodeApp.swift +++ b/macapp/Sources/GoCodeApp/GoCodeApp.swift @@ -38,7 +38,10 @@ struct GoCodeApp: App { } var body: some Scene { - WindowGroup("GoCode") { + // Empty title: the app's identity is a sidebar header row now, the + // way the reference does it. A window title here would reserve a + // second chrome band purely to repeat it. + WindowGroup("") { AppShell( initialWorkspace: Self.initialWorkspace, externalBaseURL: Self.externalBaseURL, diff --git a/macapp/Sources/GoCodeUI/ChatView.swift b/macapp/Sources/GoCodeUI/ChatView.swift index ffa0543e..583f5b39 100644 --- a/macapp/Sources/GoCodeUI/ChatView.swift +++ b/macapp/Sources/GoCodeUI/ChatView.swift @@ -222,8 +222,8 @@ struct CopyMessageButton: View { Image(systemName: copied ? "checkmark" : "doc.on.doc") .font(Typography.caption) .foregroundStyle( - // 139, not 116: quaternary is the section-label rung and - // sat a step below where the reference puts these. + // Only the copied state overrides; the row owns the + // resting ink so all four icons agree. copied ? AnyShapeStyle(.tint) : AnyShapeStyle(Theme.foregroundSubtle) ) .contentShape(.rect) @@ -373,8 +373,14 @@ struct MessageActions: View { .help("Undo last turn") .accessibilityLabel("Undo last turn") } + // One size and one ink across all four. Different SF Symbols draw to + // different heights at the same point size, so the row measured a 62% + // spread; a fixed frame makes them a set. The colour is set once here + // and the buttons no longer override it — three of four had been + // inheriting the section-label rung instead. .font(.system(size: IconSize.detail)) - .foregroundStyle(Theme.foregroundQuaternary) + .symbolRenderingMode(.monochrome) + .foregroundStyle(Theme.foregroundSubtle) .buttonStyle(.plain) .frame(maxWidth: .infinity, alignment: .leading) } diff --git a/macapp/Sources/GoCodeUI/ConversationRail.swift b/macapp/Sources/GoCodeUI/ConversationRail.swift index ecad1019..6e7ec01f 100644 --- a/macapp/Sources/GoCodeUI/ConversationRail.swift +++ b/macapp/Sources/GoCodeUI/ConversationRail.swift @@ -10,6 +10,27 @@ struct ConversationRail: View { var body: some View { VStack(alignment: .leading, spacing: Spacing.tight) { + // The reference's app identity lives here, not in the window title + // bar: a bright header row inside the sidebar with a disclosure + // affordance. Ours was a recessed system title at 89 against the + // reference's 222 — 60% dimmer, in the wrong pane, and it cost a + // whole chrome band to display. + HStack(spacing: Spacing.small) { + Text("GoCode") + .font(Typography.body.weight(.semibold)) + .foregroundStyle(Theme.foregroundSecondary) + Image(systemName: "chevron.down") + .font(.system(size: IconSize.status)) + .foregroundStyle(Theme.foregroundSubtle) + Spacer(minLength: Spacing.none) + Image(systemName: "magnifyingglass") + .font(.system(size: IconSize.detail)) + .foregroundStyle(Theme.foregroundSubtle) + .accessibilityHidden(true) + } + .padding(.horizontal, Spacing.comfortable) + .padding(.bottom, Spacing.small) + RailRow(section: $section, item: .chat) // Each header is gated on its own rows. Gating both on "any @@ -48,6 +69,10 @@ struct ConversationRail: View { Button(action: onClose) { Image(systemName: "xmark.circle") .font(.system(size: IconSize.standard)) + // Matches its four neighbours. It had been rendering + // 60% brighter than them, which read as a state rather + // than as the same kind of control. + .foregroundStyle(Theme.foregroundSubtle) } .buttonStyle(.plain) .help("Close project and stop its server") @@ -100,14 +125,10 @@ private struct ConversationRailRow: View { section = .chat } label: { HStack(spacing: Spacing.small) { - Circle() - .fill(isActive ? Theme.foreground : Theme.foregroundQuaternary) - .frame(width: IconSize.rule, height: IconSize.rule) - .accessibilityHidden(true) - Image(systemName: "bubble.left") - .font(.system(size: IconSize.detail)) - .foregroundStyle(Theme.foregroundQuaternary) - .accessibilityHidden(true) + // No bullet, no leading icon. The reference's chat rows are + // text-only and align with its project rows; ours carried a + // 255-peak bullet — the brightest ink in the whole sidebar — + // and a speech-bubble glyph, creating a third indent level. Text(conversation.displayTitle) // Body, not detail. The reference sets its sidebar very // slightly *larger* than its transcript body; this was 24% diff --git a/macapp/Sources/GoCodeUI/DesignSystem/IconSize.swift b/macapp/Sources/GoCodeUI/DesignSystem/IconSize.swift index 4d95192c..82f2f6a2 100644 --- a/macapp/Sources/GoCodeUI/DesignSystem/IconSize.swift +++ b/macapp/Sources/GoCodeUI/DesignSystem/IconSize.swift @@ -7,7 +7,9 @@ enum IconSize { static let status: CGFloat = 7 static let detail: CGFloat = 14 static let standard: CGFloat = 15 - static let row: CGFloat = 18 + /// 15, not 18. Measured ink width the nav icon drew 21.0pt against the + /// reference's 15.0 — 40% oversized, and starting 4.5pt further left. + static let row: CGFloat = 15 /// Composer chip icons. Fixed so adjacent chips cannot differ in symbol /// width, which they did by 48%. static let chip: CGFloat = 13.5 diff --git a/macapp/Tests/GoCodeUITests/DesignTokenTests.swift b/macapp/Tests/GoCodeUITests/DesignTokenTests.swift index 09dbe246..3a9ec9d0 100644 --- a/macapp/Tests/GoCodeUITests/DesignTokenTests.swift +++ b/macapp/Tests/GoCodeUITests/DesignTokenTests.swift @@ -32,7 +32,8 @@ struct DesignTokenTests { #expect(IconSize.status == 7) #expect(IconSize.detail == 14) #expect(IconSize.standard == 15) - #expect(IconSize.row == 18) + // 15, not 18: measured ink width was 40% over the reference's. + #expect(IconSize.row == 15) #expect(IconSize.emptyState == 30) #expect(IconSize.launch == 44) } From de965761a00f50b533cb72c96682838efeb3de15 Mon Sep 17 00:00:00 2001 From: Dennison Date: Fri, 31 Jul 2026 21:57:52 +0200 Subject: [PATCH 8/9] =?UTF-8?q?feat(macapp):=20one=20chrome=20band=20?= =?UTF-8?q?=E2=80=94=20header=2091.5pt=20to=2067.5pt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The change three previous attempts failed at, and the reason they failed. The header rule sat at 91.5pt against the reference's 55: a 52pt system titlebar stacked above a 40pt header band. Hiding the titlebar was tried twice and reverted twice, because a window toolbar *requires* that band — hide it and the toolbar's items are re-homed into the sidebar's corner, which is worse than the problem. The missing step was that the toolbar did not need to exist. Its two controls belong to the conversation, so the header now owns them, there is no toolbar left to displace, and the titlebar can go. One band, measured 67.5pt on the running app. The app's identity moved into the sidebar in the previous commit, which is what made this possible: with the wordmark gone from the titlebar, nothing was left up there worth a band. **Still wrong, and measured rather than glossed:** the header's controls sit beside the title instead of at the far right. The Spacer between them is not expanding, so the row sizes to its content — the Group wrapper appears to be constraining it. The height gain is real and independent of this; the alignment is not fixed. Also open: 67.5 is not 55. The remaining 12.5pt is the header band's own padding, which is worth measuring against the reference's internal spacing before changing, rather than trimmed to hit a number. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UPFeSNp49415WenKDF7gy9 --- macapp/Sources/GoCodeApp/GoCodeApp.swift | 6 +++++- macapp/Sources/GoCodeUI/ChatView.swift | 16 +++------------- .../Sources/GoCodeUI/ConversationChrome.swift | 18 ++++++++++++++++++ macapp/Sources/GoCodeUI/ConversationRail.swift | 3 +++ 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/macapp/Sources/GoCodeApp/GoCodeApp.swift b/macapp/Sources/GoCodeApp/GoCodeApp.swift index 5cb362eb..3f3a5d46 100644 --- a/macapp/Sources/GoCodeApp/GoCodeApp.swift +++ b/macapp/Sources/GoCodeApp/GoCodeApp.swift @@ -47,6 +47,10 @@ struct GoCodeApp: App { externalBaseURL: Self.externalBaseURL, initialPrompt: Self.environment["GOCODE_INITIAL_PROMPT"]) } - .windowToolbarStyle(.unified) + // Hidden. With the header owning its own controls there is no toolbar + // left to displace, which is what defeated this the first time. The + // system band was pure overhead: 52pt above a 40pt header, for a + // title the sidebar now carries. + .windowStyle(.hiddenTitleBar) } } diff --git a/macapp/Sources/GoCodeUI/ChatView.swift b/macapp/Sources/GoCodeUI/ChatView.swift index 583f5b39..414f9ac8 100644 --- a/macapp/Sources/GoCodeUI/ChatView.swift +++ b/macapp/Sources/GoCodeUI/ChatView.swift @@ -18,7 +18,8 @@ struct ChatView: View { // simply reflows around it. HStack(alignment: .top, spacing: Spacing.none) { VStack(spacing: Spacing.none) { - ConversationHeader(project: project, run: run) + ConversationHeader( + project: project, run: run, showInspector: $showInspector) TranscriptView( items: run.transcript.items, statusMessage: project.statusMessage, @@ -54,18 +55,7 @@ struct ChatView: View { // covered this strip, in the *sidebar's* colour, so the pane began at // y=52 and the window wore a full-width band above it. .background(Theme.background.ignoresSafeArea(.container, edges: .top)) - .toolbar { - ToolbarItemGroup(placement: .primaryAction) { - CopyConversationButton(items: run.transcript.items) - Button { - showInspector.toggle() - } label: { - Image(systemName: showInspector ? "sidebar.trailing" : "sidebar.right") - } - .help(showInspector ? "Hide tool inspector" : "Show tool inspector") - .accessibilityLabel(showInspector ? "Hide tool inspector" : "Show tool inspector") - } - } + // Clicking a tool call is the reason the inspector exists; open it // automatically on selection rather than leaving the click looking // like it did nothing because the pane defaults to closed. diff --git a/macapp/Sources/GoCodeUI/ConversationChrome.swift b/macapp/Sources/GoCodeUI/ConversationChrome.swift index 23613664..d0af03ff 100644 --- a/macapp/Sources/GoCodeUI/ConversationChrome.swift +++ b/macapp/Sources/GoCodeUI/ConversationChrome.swift @@ -63,6 +63,11 @@ struct ConversationTitle: View { struct ConversationHeader: View { @Bindable var project: ProjectSession @Bindable var run: RunSession + /// Carried here rather than in a window toolbar. A toolbar forces a + /// system titlebar band, and this header then had to sit *below* it — + /// which is the whole reason the header measured 91.5pt against the + /// reference's 55. Owning its own controls lets the band be the only one. + @Binding var showInspector: Bool var body: some View { // Not ConversationColumn. Binding the header to the transcript's column @@ -87,6 +92,16 @@ struct ConversationHeader: View { .foregroundStyle(Theme.foreground) .lineLimit(1) Spacer(minLength: Spacing.none) + CopyConversationButton(items: run.transcript.items) + Button { + showInspector.toggle() + } label: { + Image(systemName: showInspector ? "sidebar.trailing" : "sidebar.right") + } + .buttonStyle(.plain) + .foregroundStyle(Theme.foregroundSubtle) + .help(showInspector ? "Hide tool inspector" : "Show tool inspector") + .accessibilityLabel(showInspector ? "Hide tool inspector" : "Show tool inspector") Menu { Button("New conversation") { project.newConversation() } if run.conversationID != nil { @@ -109,6 +124,9 @@ struct ConversationHeader: View { .help("Conversation actions") .accessibilityLabel("Conversation actions") } + // Fills the pane so the Spacer can push the controls to the far + // right. Sizing to content left them clustered beside the title. + .frame(maxWidth: .infinity) .frame(height: Spacing.conversationHeaderHeight) .padding(.horizontal, Spacing.section) diff --git a/macapp/Sources/GoCodeUI/ConversationRail.swift b/macapp/Sources/GoCodeUI/ConversationRail.swift index 6e7ec01f..121b1628 100644 --- a/macapp/Sources/GoCodeUI/ConversationRail.swift +++ b/macapp/Sources/GoCodeUI/ConversationRail.swift @@ -30,6 +30,9 @@ struct ConversationRail: View { } .padding(.horizontal, Spacing.comfortable) .padding(.bottom, Spacing.small) + // Clears the window controls, which now overlay the sidebar's own + // top rather than living in a band of their own. + .padding(.top, Spacing.trafficLightClearance) RailRow(section: $section, item: .chat) From 67175cb4002fe6e6f7dc3bbeeaf3f31d617dd7be Mon Sep 17 00:00:00 2001 From: Dennison Date: Fri, 31 Jul 2026 22:02:08 +0200 Subject: [PATCH 9/9] fix(macapp): the header's controls reach the right edge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `.borderlessButton` menu expands to fill whatever width it is offered, so the overflow menu was swallowing the Spacer and leaving the header's controls clustered beside the title. `.fixedSize()` on the menu — the same fix the other copy of this code already carried — lets the Spacer do its job. The Group wrapper went with it; it was doing nothing once the header stopped being bound to the transcript's column. Verified on the running app: title at the left, controls at the right edge, one header band. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UPFeSNp49415WenKDF7gy9 --- .../Sources/GoCodeUI/ConversationChrome.swift | 114 +++++++++--------- 1 file changed, 56 insertions(+), 58 deletions(-) diff --git a/macapp/Sources/GoCodeUI/ConversationChrome.swift b/macapp/Sources/GoCodeUI/ConversationChrome.swift index d0af03ff..e829c4e9 100644 --- a/macapp/Sources/GoCodeUI/ConversationChrome.swift +++ b/macapp/Sources/GoCodeUI/ConversationChrome.swift @@ -70,67 +70,65 @@ struct ConversationHeader: View { @Binding var showInspector: Bool var body: some View { - // Not ConversationColumn. Binding the header to the transcript's column - // truncated the title at the column edge while 402pt of its own row — - // 38% of it — sat empty to the right. The reference gives its title the - // whole header row and column-binds only the transcript. - Group { - HStack(spacing: Spacing.small) { - // Primary, not tertiary. The reference draws this icon at - // full white — it belongs to the title beside it. Ours sat two - // rungs down while the overflow menu next to it sat two rungs - // up, so the row's hierarchy was inverted at both ends. - Image(systemName: "folder") - .font(.system(size: IconSize.detail)) - .foregroundStyle(Theme.foreground) - .accessibilityHidden(true) - Text(title) - .font(Typography.body.weight(.medium)) - // Primary, matching the folder icon beside it. Setting the - // icon to white and leaving the title a rung down made the - // decoration brighter than the label it decorates. - .foregroundStyle(Theme.foreground) - .lineLimit(1) - Spacer(minLength: Spacing.none) - CopyConversationButton(items: run.transcript.items) - Button { - showInspector.toggle() - } label: { - Image(systemName: showInspector ? "sidebar.trailing" : "sidebar.right") - } - .buttonStyle(.plain) - .foregroundStyle(Theme.foregroundSubtle) - .help(showInspector ? "Hide tool inspector" : "Show tool inspector") - .accessibilityLabel(showInspector ? "Hide tool inspector" : "Show tool inspector") - Menu { - Button("New conversation") { project.newConversation() } - if run.conversationID != nil { - Button("Fork conversation") { Task { await project.fork() } } - Button("Undo last turn") { Task { await project.undo() } } - } - } label: { - // Subtle: an overflow menu should be findable when looked - // for and invisible otherwise. This was brighter than the - // title's own folder icon. - Image(systemName: "ellipsis") - .font(.system(size: IconSize.detail)) - .foregroundStyle(Theme.foregroundSubtle) + // Not ConversationColumn. Binding the header to the transcript's + // column truncated the title at the column edge while 402pt of its own + // row sat empty to the right. The reference gives its title the whole + // header row and column-binds only the transcript. + HStack(spacing: Spacing.small) { + // Primary, not tertiary. The reference draws this icon at + // full white — it belongs to the title beside it. Ours sat two + // rungs down while the overflow menu next to it sat two rungs + // up, so the row's hierarchy was inverted at both ends. + Image(systemName: "folder") + .font(.system(size: IconSize.detail)) + .foregroundStyle(Theme.foreground) + .accessibilityHidden(true) + Text(title) + .font(Typography.body.weight(.medium)) + // Primary, matching the folder icon beside it. Setting the + // icon to white and leaving the title a rung down made the + // decoration brighter than the label it decorates. + .foregroundStyle(Theme.foreground) + .lineLimit(1) + Spacer(minLength: Spacing.none) + CopyConversationButton(items: run.transcript.items) + Button { + showInspector.toggle() + } label: { + Image(systemName: showInspector ? "sidebar.trailing" : "sidebar.right") + } + .buttonStyle(.plain) + .foregroundStyle(Theme.foregroundSubtle) + .help(showInspector ? "Hide tool inspector" : "Show tool inspector") + .accessibilityLabel(showInspector ? "Hide tool inspector" : "Show tool inspector") + Menu { + Button("New conversation") { project.newConversation() } + if run.conversationID != nil { + Button("Fork conversation") { Task { await project.fork() } } + Button("Undo last turn") { Task { await project.undo() } } } - .menuStyle(.borderlessButton) - // Tint on the Menu, not on its label: .borderlessButton - // re-tints the label it is given, so styling the Image alone - // left the overflow brighter than the title's own folder icon. - .tint(Theme.foregroundSubtle) - .help("Conversation actions") - .accessibilityLabel("Conversation actions") + } label: { + // Subtle: an overflow menu should be findable when looked + // for and invisible otherwise. This was brighter than the + // title's own folder icon. + Image(systemName: "ellipsis") + .font(.system(size: IconSize.detail)) + .foregroundStyle(Theme.foregroundSubtle) } - // Fills the pane so the Spacer can push the controls to the far - // right. Sizing to content left them clustered beside the title. - .frame(maxWidth: .infinity) - .frame(height: Spacing.conversationHeaderHeight) - .padding(.horizontal, Spacing.section) - + .menuStyle(.borderlessButton) + // Tint on the Menu, not on its label: .borderlessButton + // re-tints the label it is given, so styling the Image alone + // left the overflow brighter than the title's own folder icon. + .tint(Theme.foregroundSubtle) + // A .borderlessButton menu expands to fill whatever width it + // is offered, which swallowed the Spacer and left the header's + // controls clustered beside the title instead of at the right. + .fixedSize() + .help("Conversation actions") + .accessibilityLabel("Conversation actions") } + .frame(height: Spacing.conversationHeaderHeight) + .padding(.horizontal, Spacing.section) // The reference is a compartmented app: rules cut header from // transcript, transcript from composer, sidebar from account. With // none of them the window reads as one flat plane with a floating box.