diff --git a/macapp/Sources/GoCodeApp/GoCodeApp.swift b/macapp/Sources/GoCodeApp/GoCodeApp.swift index ba7c0729..3f3a5d46 100644 --- a/macapp/Sources/GoCodeApp/GoCodeApp.swift +++ b/macapp/Sources/GoCodeApp/GoCodeApp.swift @@ -38,12 +38,19 @@ 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, 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/AppShell.swift b/macapp/Sources/GoCodeUI/AppShell.swift index ab08e179..7a257e0d 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 { @@ -129,9 +132,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 +205,21 @@ 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. + // 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.selectedRowForeground : Theme.foregroundSecondary + // 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) } @@ -218,9 +235,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 e50b5fb1..80d6aa60 100644 --- a/macapp/Sources/GoCodeUI/ChatView.swift +++ b/macapp/Sources/GoCodeUI/ChatView.swift @@ -19,7 +19,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,12 @@ struct ChatView: View { .transition(.move(edge: .trailing).combined(with: .opacity)) } } - .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") - } - } + // 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)) + // 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. @@ -101,7 +96,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) } @@ -222,7 +220,9 @@ struct CopyMessageButton: View { Image(systemName: copied ? "checkmark" : "doc.on.doc") .font(Typography.caption) .foregroundStyle( - copied ? AnyShapeStyle(.tint) : AnyShapeStyle(Theme.foregroundQuaternary) + // Only the copied state overrides; the row owns the + // resting ink so all four icons agree. + copied ? AnyShapeStyle(.tint) : AnyShapeStyle(Theme.foregroundSubtle) ) .contentShape(.rect) } @@ -248,7 +248,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") @@ -367,8 +371,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) } @@ -606,7 +616,7 @@ struct ToolRow: View { } Rectangle() .fill(Theme.separator) - .frame(height: Spacing.hairline) + .frame(height: Spacing.toolRuleWeight) } .contentShape(.rect) } @@ -794,7 +804,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) } } } @@ -867,6 +880,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) @@ -875,12 +892,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(action) }) { @@ -900,8 +924,21 @@ struct Composer: View { action.isSteer ? "Steer the running task" : "Send message") } } - .padding(.horizontal, Spacing.large).padding(.vertical, Spacing.inset) + // 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. + // 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.rule, 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, @@ -981,6 +1018,45 @@ enum ComposerAction: Equatable { } } +/// 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) { + // 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] : []) + } +} + struct ModelChip: View { @Bindable var project: ProjectSession @@ -1007,8 +1083,15 @@ struct ModelChip: View { Text(hiddenSummary).font(Typography.caption) } } label: { - Label(project.selectedModel ?? "Server default", systemImage: "cpu") - .font(Typography.caption) + // 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 b9172960..e829c4e9 100644 --- a/macapp/Sources/GoCodeUI/ConversationChrome.swift +++ b/macapp/Sources/GoCodeUI/ConversationChrome.swift @@ -13,36 +13,129 @@ struct ConversationColumn: View { } } -struct ConversationHeader: 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 { - ConversationColumn { - HStack(spacing: Spacing.small) { - Image(systemName: "folder") + 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)) - .foregroundStyle(Theme.foregroundTertiary) - .accessibilityHidden(true) - Text(title) - .font(Typography.body.weight(.medium)) - .lineLimit(1) - Spacer(minLength: Spacing.none) - 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 + /// 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 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) - .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) } - .frame(height: Spacing.conversationHeaderHeight) + .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. + .overlay(alignment: .bottom) { + Rectangle() + .fill(Theme.rule) + .frame(height: Spacing.hairline) } } diff --git a/macapp/Sources/GoCodeUI/ConversationRail.swift b/macapp/Sources/GoCodeUI/ConversationRail.swift index 2b721c04..121b1628 100644 --- a/macapp/Sources/GoCodeUI/ConversationRail.swift +++ b/macapp/Sources/GoCodeUI/ConversationRail.swift @@ -10,6 +10,30 @@ 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) + // 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) // Each header is gated on its own rows. Gating both on "any @@ -33,6 +57,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) @@ -41,6 +72,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") @@ -93,16 +128,16 @@ 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) - .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/Sources/GoCodeUI/DesignSystem/IconSize.swift b/macapp/Sources/GoCodeUI/DesignSystem/IconSize.swift index b3fd5ce8..82f2f6a2 100644 --- a/macapp/Sources/GoCodeUI/DesignSystem/IconSize.swift +++ b/macapp/Sources/GoCodeUI/DesignSystem/IconSize.swift @@ -7,7 +7,12 @@ 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 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/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 e0f12db8..48c247b4 100644 --- a/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift +++ b/macapp/Sources/GoCodeUI/DesignSystem/Spacing.swift @@ -5,10 +5,17 @@ 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 + /// 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 @@ -16,12 +23,30 @@ 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 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 + /// 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/Theme.swift b/macapp/Sources/GoCodeUI/DesignSystem/Theme.swift index d4f1c82c..e1d3affd 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 @@ -154,6 +175,30 @@ 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) + + /// 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/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 9aef4afb..7dba4f75 100644 --- a/macapp/Tests/GoCodeUITests/DesignTokenTests.swift +++ b/macapp/Tests/GoCodeUITests/DesignTokenTests.swift @@ -53,17 +53,21 @@ 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) - #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) } @@ -127,8 +131,12 @@ struct DesignTokenTests { @Test("conversation layout tokens share the Codex column and quieter divider") func conversationLayout() { #expect(Layout.chatContentMaximumWidth == 883) - #expect(Spacing.conversationHeaderHeight == 52) - #expect(Spacing.transcriptTop == 65.5) + // 40, not 52: with padding the band measured 103pt against the + // reference's 55pt. + #expect(Spacing.conversationHeaderHeight == 40) + // 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)) } } @@ -170,3 +178,108 @@ 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 + } +} + +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) + } +} + +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) + } +} 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(