diff --git a/Kanban/LiveStore/StyleModel.swift b/Kanban/LiveStore/StyleModel.swift index c18b35f..ab731f8 100644 --- a/Kanban/LiveStore/StyleModel.swift +++ b/Kanban/LiveStore/StyleModel.swift @@ -24,8 +24,9 @@ import Foundation /// /// **Hex does arrive here from the app now**, which this comment used to deny — 03-board-ui.md's /// "custom hex is not pickable in-app but stays fully honored from disk" stopped being true the day -/// `ColorComboView` shipped an **Other…** row onto `NSColorPanel.shared`, and the 2026-08-09 rework -/// put that door on the Style… popover and the symbol popover's tint row as well. The write path +/// the card sidebar's background control shipped an **Other…** row onto `NSColorPanel.shared`, and +/// the 2026-08-09 rework put that door on the Style… popover and the symbol popover's tint row as +/// well. The write path /// always handled it (`FrontmatterValue.emitScalar` quotes a `#` because YAML would otherwise read it /// as a comment; `BackgroundField.flowText` quotes unconditionally) and now says so out loud: /// `CustomColorRoundTripTests` asserts the whole chain. diff --git a/Kanban/UI/Card/CardSidebarSections.swift b/Kanban/UI/Card/CardSidebarSections.swift index 53701a7..94f745c 100644 --- a/Kanban/UI/Card/CardSidebarSections.swift +++ b/Kanban/UI/Card/CardSidebarSections.swift @@ -1,15 +1,20 @@ import AppKit import SwiftUI -// MARK: - Style +// MARK: - Appearance -/// The sidebar's **Style** section: the **Background color combo** over the compact **`SymbolPicker`** -/// (05-card-window.md ▸ Style; 03-board-ui.md ▸ Styling ▸ Controls, its 2026-08-06 anchor-ownership -/// rule for the background half, extended to the symbol half on 2026-08-08). Card styling is -/// discoverable here without a context menu — the picker is the same reusable control the board -/// popover's glyph picker beside the rename field already uses (`BoardInfoPopover`). +/// The sidebar's **Appearance** section (renamed from **Style**, owner's ruling 2026-08-10, Pipeline +/// card 5004c540 — the sidebar-only rename; the board context menu's **Style…** item and +/// `StyleEditorView`'s own naming are unaffected, per that ruling's own scope): the **`SymbolPicker`** +/// rectangle beside the **`ColorSwatchPicker`** rectangle, side by side — Symbol left, Background +/// right, each captioned *above* rather than leading (05-card-window.md ▸ Style; 03-board-ui.md ▸ +/// Styling ▸ Controls, its 2026-08-06 anchor-ownership rule for the background half, extended to the +/// symbol half on 2026-08-08). Card styling is discoverable here without a context menu — both +/// rectangles are the same reusable controls the board popover's glyph picker beside the rename field +/// already uses (`BoardInfoPopover`) and the Style… popover's own grid reuses (`ColorSwatchPicker`'s +/// own header). /// -/// ### Why the curated grid left +/// ### Why the curated grid left (history, unaffected by this rename) /// /// `StyleEditorView`'s curated grid used to draw **whole, uncollapsed** in this column /// (`StyleEditorLayout.sidebar`'s `symbolGridMaximumHeight: nil` — "the sidebar is already a scroll @@ -17,18 +22,18 @@ import SwiftUI /// grid, permanently open, ahead of every other section the sidebar carries. `SymbolPicker` is the /// general-purpose control built for precisely this shape of problem (its own header: "a control any /// caller can aim at one symbol field without wiring up a `BoardStore`, a `StyleTarget`, or the -/// two-dimension batch machinery `StyleEditorView` carries") — one small well at rest, the grid only -/// while its popover is open. This section is the first caller to aim it at a store-backed field +/// two-dimension batch machinery `StyleEditorView` carries") — one small rectangle at rest, the grid +/// only while its popover is open. This section is the first caller to aim it at a store-backed field /// rather than a plain binding, which is why it does the store plumbing below that `SymbolPicker` /// itself deliberately knows nothing about. /// /// **`StyleEditorView` is no longer instantiated here at all.** It was already `showsBackground: -/// false` — the color combo has owned the background half since 2026-08-06 — so the curated symbol -/// grid was its only remaining job in this section, and that job is what moved. The shared -/// component's `.sidebar` layout variant and `showsBackground` toggle stay in `StyleEditor.swift` -/// regardless: they are still correct and still tested (`StyleEditorLayoutTests`), and that file's own -/// stance is to be anchor-agnostic about callers it does not currently have. Retiring them as dead -/// code is a separate, larger cut this card does not make. +/// false` — the background rectangle has owned that half since 2026-08-06 (as `ColorComboView`, now +/// `ColorSwatchPicker`) — so the curated symbol grid was its only remaining job in this section, and +/// that job moved out to `SymbolPicker`. The shared component's `.sidebar` layout variant and +/// `showsBackground` toggle stay in `StyleEditor.swift` regardless: they are still correct and still +/// tested (`StyleEditorLayoutTests`), and that file's own stance is to be anchor-agnostic about +/// callers it does not currently have. /// /// ### The curated set does not shrink /// @@ -43,14 +48,23 @@ import SwiftUI /// /// ### Writes still ride the one funnel, on this window's own stack /// -/// `onSelect`/`onSelectColor` below call `StyleCommand.apply(icon:iconColor:to:in:store:recents:on:)` -/// — the same funnel `StyleEditorView`'s wells rode, `on: undo` passed through exactly as before -/// (13-native-undo.md ▸ Rules ▸ two levels: a colour or symbol chosen here is a gesture *issued in -/// this window*, so its step joins the window's session and reaches board history only inside the -/// coarse close step — this section's own note on `undo` below, unchanged by the control swap). -/// Nothing about `BoardStore.applyStyle`'s batch semantics, its no-op skipping, or its `HistoryAnchor` -/// choice moved; only the view that calls it did — `CardSessionUndoTests.sidebarStylingIsASessionGesture` -/// calls that same funnel directly and needed no change. +/// `onSelect`/`onSelectColor`/`onChange` below call `StyleCommand.apply(icon:iconColor:to:in:store: +/// recents:on:)` — the same funnel `StyleEditorView`'s wells rode, `on: undo` passed through exactly +/// as before (13-native-undo.md ▸ Rules ▸ two levels: a colour or symbol chosen here is a gesture +/// *issued in this window*, so its step joins the window's session and reaches board history only +/// inside the coarse close step). Nothing about `BoardStore.applyStyle`'s batch semantics, its no-op +/// skipping, or its `HistoryAnchor` choice moved; only the views that call it did — +/// `CardSessionUndoTests.sidebarStylingIsASessionGesture` calls that same funnel directly and needed +/// no change. +/// +/// **The background rectangle's Colors-panel door no longer needs this section's own debounce.** +/// `ColorComboView`'s old panel handoff was a per-view-owned `SystemColorPanel`, undebounced at the +/// source, so this section carried its own trailing-debounce `Task` (`backgroundPanelCommit`) around +/// it. `ColorSwatchPicker`'s **Other…** row opens `SharedColorPanelSession` instead — the same +/// pre-debounced (~400ms) shared session the Style… popover's own background section and the symbol +/// popover's tint row already use — so the settled value arrives already debounced, and this section +/// only has to write it (`applyPanelBackground(_:)`), raw, never through the recents funnel, exactly +/// as `StyleEditorView.openBackgroundPanel` writes its own. /// /// ### The target is fixed, and that is the whole difference in behavior /// @@ -58,10 +72,10 @@ import SwiftUI /// re-resolved against every snapshot, dismissing when it empties (`StyleEditorSession`). This /// section tracks nothing. Its target is this window's card, always — "the two embedded anchors need /// none of this and get none: the card sidebar dismisses with its card's window, and the board -/// popover's target is the board itself" (`StyleEditorSession`'s own note). `SymbolPicker`'s own -/// popover presentation is plain view-local `@State`, needing no session for the same reason: the -/// window's card cannot vanish out from under its own window, and when it does the window goes with -/// it (`CardWindowFate`). +/// popover's target is the board itself" (`StyleEditorSession`'s own note). `SymbolPicker`'s and +/// `ColorSwatchPicker`'s own popover presentations are plain view-local `@State`, needing no session +/// for the same reason: the window's card cannot vanish out from under its own window, and when it +/// does the window goes with it (`CardWindowFate`). struct CardStyleSection: View { let store: BoardStore @@ -69,16 +83,10 @@ struct CardStyleSection: View { let cardID: ItemID /// **This window's undo stack** (13-native-undo.md ▸ Rules ▸ two levels): a colour or symbol /// chosen here is a gesture *issued in this window*, so its step joins the window's session and - /// reaches board history only inside the coarse close step. The background combo below and the - /// symbol picker both take it as a parameter, for the same reason. + /// reaches board history only inside the coarse close step. The background rectangle below and + /// the symbol rectangle both take it as a parameter, for the same reason. let undo: CardWindowUndo - /// The trailing debounce on a live colour-panel drag (`ColorComboView`'s `onPanelChange`, - /// opened from the combo's **Other…** row): cancelled and replaced on every tick, so only the - /// value the user is still on ~400ms after the last one actually reaches disk. One task for the - /// section's one combo. - @State private var backgroundPanelCommit: Task? - /// The live body metric, read here rather than passed in — `CardAttachmentsSection`'s pattern, /// so every section in this sidebar derives its geometry the same way. private var pointSize: CGFloat { CardWindowMetrics.bodyPointSize } @@ -96,102 +104,32 @@ struct CardStyleSection: View { var body: some View { VStack(alignment: .leading, spacing: CardWindowMetrics.sidebarRowSpacing(bodyPointSize: pointSize)) { - CardSidebarSectionHeader(title: "Style") - backgroundComboRow - symbolRow - } - .frame(maxWidth: .infinity, alignment: .leading) - } - - // MARK: - Background combo - - /// The labeled **Background** row, above the well grid — a narrower, single-value alternative - /// to it (`ColorCombo.swift`'s own doc comment): an inspector row, caption leading and a - /// compact combo trailing, the arrangement every Xcode inspector uses for exactly this control. - /// - /// **No explicit width since 2026-08-09** — the combo used to be stretched to just over half the - /// row (`CardWindowMetrics.sidebarContentWidth(...) * 0.55`), which was the wide-bar shape the - /// owner's iteration undid. `ComboFieldMetrics` now carries its own taller, narrower intrinsic - /// width, so the row hands the combo no frame at all and lets it size itself; the leading - /// `Spacer(minLength: 8)` still pins it to the row's trailing edge. - private var backgroundComboRow: some View { - HStack(spacing: 0) { - Text("Background") - .font(.caption) - .foregroundStyle(.secondary) - Spacer(minLength: 8) - ColorComboView( - role: .background, - value: currentBackground, - isEnabled: !store.isReadOnly, - onChange: { commitBackground($0) }, - onPanelChange: { debounceBackground($0) } - ) - } - .frame(maxWidth: .infinity, alignment: .leading) - } - - /// The card's `background` field, exactly as written — malformed reads as its raw text, missing - /// reads `nil`, both `StyleFieldState.written`'s own rule (`StyleModel.swift`). The **raw** - /// string, never a resolved colour: `ColorComboModel`'s matching needs the bytes, not what they - /// render as. - private var currentBackground: String? { - StyleFieldState.written(currentSubject?.background ?? .missing) - } - - /// A discrete pick — commits immediately. `nil` removes; a name from `Palette.backgrounds` goes - /// through `StyleCommand.apply` so it feeds `StyleRecents` exactly like a well click would - /// ("updated on every background application from any anchor", `StyleEditor.swift`); anything - /// else — the dynamic current-value row re-affirming a foreign name or a custom hex — writes - /// directly, since it is not the "palette pick" recents was ever meant to remember. - private func commitBackground(_ newValue: String?) { - let target = Self.target(forCard: cardID) - guard let newValue else { - store.applyStyle(to: target, background: .remove, icon: .keep, on: undo) - return - } - if Palette.backgrounds.contains(where: { $0.name == newValue }) { - StyleCommand.apply(background: .set(newValue), to: target, in: store, recents: recents, on: undo) - } else { - store.applyStyle(to: target, background: .set(newValue), icon: .keep, on: undo) - } - } - - /// One tick of a live colour-panel drag: cancels whatever commit was pending and schedules a new - /// one ~400ms out, so a drag writes once it settles rather than on every pixel it passes through. - /// Never routed through `StyleCommand.apply` — a drag that passes through a palette-exact hex - /// mid-gesture must not spam the recents row the way a deliberate pick would. - private func debounceBackground(_ newValue: String?) { - backgroundPanelCommit?.cancel() - let target = Self.target(forCard: cardID) - backgroundPanelCommit = Task { @MainActor in - try? await Task.sleep(for: .milliseconds(400)) - guard !Task.isCancelled else { return } - if let newValue { - store.applyStyle(to: target, background: .set(newValue), icon: .keep, on: undo) - } else { - store.applyStyle(to: target, background: .remove, icon: .keep, on: undo) + CardSidebarSectionHeader(title: "Appearance") + // Side by side, Symbol left of Background — the owner's sketch (2026-08-10 ruling): + // "Symbol Background" over the two rectangles. `.top` alignment keeps the two + // captions, and the two rectangles below them, on one shared baseline even though + // neither column reserves a fixed width for the other. + HStack(alignment: .top, spacing: CardWindowMetrics.sidebarRowSpacing(bodyPointSize: pointSize)) { + symbolColumn + backgroundColumn } + .frame(maxWidth: .infinity, alignment: .leading) } + .frame(maxWidth: .infinity, alignment: .leading) } // MARK: - Symbol - /// The labeled **Symbol** row, below the background combo — `backgroundComboRow`'s own - /// inspector-row shape, restated: caption leading, the combo trailing, **at the same size**. - /// - /// That size used to be the difference between the two rows. The picker was a 20pt bordered - /// square with one hit zone sitting under a wide two-zone colour combo, and this is the sidebar - /// where the mismatch was most visible — two adjacent rows setting two adjacent keys, looking - /// like different kinds of control. Since the 2026-08-09 rework `SymbolPicker` *is* a combo - /// (`ComboField.swift`), so the two rows need no frame of their own to agree — both controls read - /// their geometry off the identical `ComboFieldMetrics`, `backgroundComboRow`'s own note. - private var symbolRow: some View { - HStack(spacing: 0) { + /// The **Symbol** column: its caption *above* the rectangle rather than leading it — the owner's + /// sketch, and `CardDetailsSection.row`'s own caption-over-value shape (`CardDetailsSection. + /// swift`) applied here instead of `CardSidebarSectionHeader`'s title-and-divider shape, which is + /// a section-level header rather than a per-control caption. `4` is `CardSidebarSectionHeader`'s + /// own literal spacing between its title and its divider, restated for the same small gap here. + private var symbolColumn: some View { + VStack(alignment: .leading, spacing: 4) { Text("Symbol") .font(.caption) .foregroundStyle(.secondary) - Spacer(minLength: 8) SymbolPicker( current: currentSymbol, fallback: ItemSymbol.card, @@ -202,18 +140,10 @@ struct CardStyleSection: View { ) // The same lock `StyleEditorView`'s whole body disabled under // (`.disabled(!store.acceptsBoardMutations)`, `StyleEditor.swift`) — the read-only lock - // and the board's inline-editing rule alike, preserved exactly across the control swap - // rather than narrowed to `backgroundComboRow`'s plainer `!store.isReadOnly`. + // and the board's inline-editing rule alike, unchanged across the layout reshape and the + // control's own two-zone-to-one-zone change. .disabled(!store.acceptsBoardMutations) } - .frame(maxWidth: .infinity, alignment: .leading) - } - - /// One `styleSubjects` lookup behind `currentBackground` above and the two properties below — - /// a card window names exactly one target, so there is exactly one subject to read for all three - /// fields. - private var currentSubject: BoardStore.StyleSubject? { - store.styleSubjects(of: Self.target(forCard: cardID)).first } /// The card's `icon` field, exactly as written — `currentBackground`'s rule, restated for the @@ -257,4 +187,71 @@ struct CardStyleSection: View { on: undo ) } + + // MARK: - Background + + /// The **Background** column, `symbolColumn`'s own caption-above shape mirrored — both + /// rectangles read their geometry off the identical `PickerRectMetrics` + /// (`PickerRect.swift`), so the two columns line up at the same height without either one + /// naming a size of its own. + private var backgroundColumn: some View { + VStack(alignment: .leading, spacing: 4) { + Text("Background") + .font(.caption) + .foregroundStyle(.secondary) + ColorSwatchPicker( + role: .background, + value: currentBackground, + isEnabled: !store.isReadOnly, + onChange: { commitBackground($0) }, + onPanelChange: { applyPanelBackground($0) } + ) + } + } + + /// The card's `background` field, exactly as written — malformed reads as its raw text, missing + /// reads `nil`, both `StyleFieldState.written`'s own rule (`StyleModel.swift`). The **raw** + /// string, never a resolved colour: the grid's selection matching needs the bytes, not what they + /// render as (`ColorSwatchPicker`'s own header on why that matching is now literal-name only). + private var currentBackground: String? { + StyleFieldState.written(currentSubject?.background ?? .missing) + } + + /// A discrete pick — commits immediately. `nil` removes; a name from `Palette.backgrounds` goes + /// through `StyleCommand.apply` so it feeds `StyleRecents` exactly like a well click would + /// ("updated on every background application from any anchor", `StyleEditor.swift`); anything + /// else writes directly, since it is not the "palette pick" recents was ever meant to remember. + private func commitBackground(_ newValue: String?) { + let target = Self.target(forCard: cardID) + guard let newValue else { + store.applyStyle(to: target, background: .remove, icon: .keep, on: undo) + return + } + if Palette.backgrounds.contains(where: { $0.name == newValue }) { + StyleCommand.apply(background: .set(newValue), to: target, in: store, recents: recents, on: undo) + } else { + store.applyStyle(to: target, background: .set(newValue), icon: .keep, on: undo) + } + } + + /// The Colors panel's settled value, from `ColorSwatchPicker`'s **Other…** door + /// (`SharedColorPanelSession`, already debounced ~400ms at the source — this section carries no + /// debounce of its own any more, `CardStyleSection`'s own header). Written raw, **never** through + /// `StyleCommand.apply` — a panel pick must not enter `StyleRecents`, the same rule + /// `StyleEditorView.openBackgroundPanel` applies to its own **Other…** row. + private func applyPanelBackground(_ newValue: String?) { + let target = Self.target(forCard: cardID) + if let newValue { + store.applyStyle(to: target, background: .set(newValue), icon: .keep, on: undo) + } else { + store.applyStyle(to: target, background: .remove, icon: .keep, on: undo) + } + } + + /// One `styleSubjects` lookup behind `currentBackground`, `currentSymbol` and `currentIconColor` + /// above — a card window names exactly one target, so there is exactly one subject to read for + /// all three fields. + private var currentSubject: BoardStore.StyleSubject? { + store.styleSubjects(of: Self.target(forCard: cardID)).first + } } diff --git a/Kanban/UI/Card/CardWindowMetrics.swift b/Kanban/UI/Card/CardWindowMetrics.swift index d086c52..14fc89f 100644 --- a/Kanban/UI/Card/CardWindowMetrics.swift +++ b/Kanban/UI/Card/CardWindowMetrics.swift @@ -70,9 +70,9 @@ enum CardWindowMetrics { /// /// Named because a section can need a number rather than a proposal — the embedded style editor's /// grids, a fixed count of fixed-size wells per row whose count had to be decided before the - /// layout ran (`StyleEditorLayout.sidebar(contentWidth:)`). The Style section's combos - /// (`CardStyleSection`'s background and symbol rows) no longer size off this figure — since - /// 2026-08-09 they carry their own intrinsic width (`ComboFieldMetrics`) and simply fill what + /// layout ran (`StyleEditorLayout.sidebar(contentWidth:)`). The Appearance section's rectangles + /// (`CardStyleSection`'s symbol and background columns) no longer size off this figure — since + /// 2026-08-09 they carry their own intrinsic width (`PickerRectMetrics`) and simply fill what /// they are proposed, like everything else in the sidebar. static func sidebarContentWidth(bodyPointSize: CGFloat) -> CGFloat { sidebarWidth(bodyPointSize: bodyPointSize) - 2 * gutter(bodyPointSize: bodyPointSize) diff --git a/Kanban/UI/ColorCombo.swift b/Kanban/UI/ColorCombo.swift deleted file mode 100644 index 25713b4..0000000 --- a/Kanban/UI/ColorCombo.swift +++ /dev/null @@ -1,450 +0,0 @@ -import AppKit -import SwiftUI - -/// A reusable colour-picker combo: a collapsed face split into **two zones**, Xcode's inspector -/// colour combo's own shape — a flat swatch of the current value filling almost the whole control, -/// and a narrow chevron trigger at the trailing edge. Clicking the swatch opens -/// `NSColorPanel.shared` directly; clicking the trigger pops a dropdown of **None**, the role's own -/// palette colours, an off-palette current value stated verbatim when there is one, and -/// **Other…**, which hands off to that same panel — the swatch and **Other…** are two doors onto -/// one panel takeover (`ColorComboView.Coordinator.openColorPanel()`). -/// -/// It is the second surface `background`/`iconColor` can be set from, beside the well grid -/// (`StyleEditor.swift`'s `StyleEditorView`) — the well grid stays exactly as it is; this is a -/// narrower, single-value control for a context where a whole grid would not fit (`CardStyleSection`'s -/// own labeled row). -/// -/// ### Two halves, the same split every other file here draws -/// -/// `ColorComboRole`, `ColorComboItem`, `ColorComboMatch` and `ColorComboModel` are the **pure model** -/// — item lists, selection matching, hex normalization, display-name casing — every rule a test can -/// hold without an `NSView` in sight. `ColorComboView` is the thin AppKit bridge that draws it and -/// answers clicks, exactly the `StyleEditorLayout`/`StyleEditorView` split in `StyleEditor.swift`. -/// (Its collapsed face has its *own*, unrelated two-zone split — swatch versus trigger, -/// `ColorComboControl`'s own doc comment — which has nothing to do with this pure-model/view one.) - -// MARK: - Role - -/// Which of the two palettes a combo offers — `Palette.backgrounds` for `background`, -/// `Palette.foregrounds` for `iconColor`/icon tints. Both tables already answer either field -/// (`Palette.nsColor(for:)`), so a combo's *role* is only about which table it lists, never about -/// which values it can resolve. -enum ColorComboRole: Sendable, Equatable { - case background - case foreground - - /// The rows this picker offers — one per entry in the role's own table (sixteen since - /// 2026-08-09; the count is the palette's business, not this type's). - var palette: [PaletteColor] { - switch self { - case .background: Palette.backgrounds - case .foreground: Palette.foregrounds - } - } - - /// The *other* picker's table — consulted only to name a foreign palette value in the dynamic - /// current-value row (`ColorComboModel.match`). Never offered as a row of this picker's own, - /// which is what keeps "background lists backgrounds" true even though `Palette.nsColor(for:)` - /// itself would happily resolve a foreground name. - var otherPalette: [PaletteColor] { - switch self { - case .background: Palette.foregrounds - case .foreground: Palette.backgrounds - } - } -} - -// MARK: - Rows and matching - -/// One row of a `ColorComboView`'s dropdown, in display order. -enum ColorComboItem: Equatable { - /// Clears the field — the well grid's own leading None well, same removal. - case none - case separator - /// One of `role`'s own entries, by name. `ColorComboModel.displayName(_:)` is its title; the row - /// is never built with anything the role's own palette doesn't list. - case palette(String) - /// The live value's own row — present only when the stored value matches neither `.none` nor a - /// `.palette` row (`ColorComboModel.match` decides). `swatchValue` is the raw stored string a - /// swatch draws from (`Palette.nsColor(for:)`, lenient exactly like `PaletteSwatch`); `title` is - /// the display text `ColorComboModel.match` already worked out. - case current(swatchValue: String, title: String) - /// Opens `NSColorPanel.shared`. - case other -} - -/// Which row a stored value checks — computed once and shared by the item list (`ColorComboModel. -/// menu`) and by anything that just wants to know "what does this resolve to" without building -/// rows, which is most of what a test wants to assert. -enum ColorComboMatch: Equatable { - case none - case palette(String) - case current(swatchValue: String, title: String) -} - -/// The full dropdown for one role at one value: its rows, and the index of the checked one. -struct ColorComboMenu: Equatable { - let items: [ColorComboItem] - /// Always a valid index into `items` — the None row exists in every menu, so there is always at - /// least one candidate to fall back to. - let selectedIndex: Int -} - -// MARK: - The pure model - -/// The whole of what a `ColorComboView` shows, as pure functions of `role` and a stored value — -/// no `NSView`, no store, nothing a `ColorComboTests` case can't hold still. -enum ColorComboModel { - - // MARK: Display - - /// Kebab-case palette name → Title Case with hyphens as spaces: `"light-cayenne"` → - /// `"Light Cayenne"`, `"smokey-rich-eggplant"` → `"Smokey Rich Eggplant"` — the one place a - /// palette name becomes a row's title rather than its stored spelling. - static func displayName(_ name: String) -> String { - name.split(separator: "-") - .map { $0.isEmpty ? "" : $0.prefix(1).uppercased() + $0.dropFirst() } - .joined(separator: " ") - } - - // MARK: Matching - - /// Which row `value` checks, given `role`: - /// - `nil` → `.none`. - /// - a name in `role`'s own palette → `.palette(name)`, matched exactly — `Palette`'s own - /// case-sensitive rule, unchanged here. - /// - a hex that, normalized, equals one of `role`'s palette hexes → that colour's `.palette` - /// match, **by name** — a panel pick landing exactly on a palette colour selects the name, so - /// picking it again from the panel later re-emits the name rather than drifting to a hex. - /// - anything else (a foreign palette name, a custom hex, or unresolvable garbage) → `.current`, - /// titled with the other picker's display name when `value` is one of *its* own, else - /// `value` itself, uppercased when it looks like hex and left verbatim otherwise. - static func match(role: ColorComboRole, value: String?) -> ColorComboMatch { - guard let value else { return .none } - if role.palette.contains(where: { $0.name == value }) { - return .palette(value) - } - if let normalized = normalizedHex(value), - let hit = role.palette.first(where: { normalizedHex($0.hex) == normalized }) { - return .palette(hit.name) - } - return .current(swatchValue: value, title: currentTitle(role: role, value: value)) - } - - /// The dynamic current-value row's title — the other table's display name when `value` is one - /// of its own, the raw string otherwise (hex shown uppercase, matching `NSColor. - /// paletteHexString`'s own casing so a stored value and a freshly panel-picked one read alike). - private static func currentTitle(role: ColorComboRole, value: String) -> String { - if let foreign = role.otherPalette.first(where: { $0.name == value }) { - return displayName(foreign.name) - } - return value.hasPrefix("#") ? value.uppercased() : value - } - - // MARK: Item list - - /// The dropdown's full row list and which row is checked, for `role` at `value`: **None**, - /// separator, the role's own entries, then — only when `match` lands on `.current` — that dynamic row, - /// separator, **Other…**. - static func menu(role: ColorComboRole, value: String?) -> ColorComboMenu { - var items: [ColorComboItem] = [.none, .separator] - items.append(contentsOf: role.palette.map { .palette($0.name) }) - - let selectedIndex: Int - switch match(role: role, value: value) { - case .none: - selectedIndex = 0 - case let .palette(name): - selectedIndex = items.firstIndex(of: .palette(name)) ?? 0 - case let .current(swatchValue, title): - items.append(.current(swatchValue: swatchValue, title: title)) - selectedIndex = items.count - 1 - } - - items.append(.separator) - items.append(.other) - return ColorComboMenu(items: items, selectedIndex: selectedIndex) - } - - // MARK: Hex normalization - - /// `#RRGGBB`/`#RRGGBBAA` → uppercase, alpha-`FF` collapsed to six digits — the string-side half - /// of the round trip `NSColor.paletteHexString` builds (Palette.swift), used here purely for - /// **comparison**: two spellings of the same opaque colour normalize to the same string, so a - /// stored `#b6071eff` matches a palette entry's `#B6071E` exactly as a bare `#b6071e` would. - /// `nil` for anything that isn't `#` followed by six or eight hex digits, so a malformed value - /// never accidentally matches a palette colour by coincidence. - static func normalizedHex(_ value: String) -> String? { - var upper = value.uppercased() - guard upper.hasPrefix("#") else { return nil } - let digits = upper.dropFirst() - guard digits.count == 6 || digits.count == 8, digits.allSatisfy(\.isHexDigit) else { return nil } - if digits.count == 8, digits.hasSuffix("FF") { - upper.removeLast(2) - } - return upper - } -} - -// MARK: - View - -/// The AppKit bridge: a two-zone collapsed face (`ColorComboControl`) whose dropdown is -/// `ColorComboModel.menu(role:value:)` — built exactly as it always was, just handed to the control -/// to pop instead of being assigned as an `NSPopUpButton`'s own `menu`. Two click targets sharing one -/// menu-plus-panel contract is the one thing `NSPopUpButton` cannot do on its own: it has exactly one -/// hit zone for exactly one action. -struct ColorComboView: NSViewRepresentable { - - let role: ColorComboRole - /// The raw stored value — a palette name or a hand-written hex, exactly as the frontmatter field - /// carries it. Never a resolved `Color`: matching needs the string, not what it renders as. - let value: String? - let isEnabled: Bool - /// One discrete row picked — **None**, one of the palette rows, or the dynamic current-value row. - /// Fired once, synchronously; the call site commits it immediately. - var onChange: @MainActor (String?) -> Void - /// One tick of a live `NSColorPanel` drag opened from **Other…** or the swatch zone — fires - /// repeatedly while the user is still adjusting the colour. Kept separate from `onChange` - /// because the two halves of this control's contract differ at the call site - /// (`CardStyleSection`): a discrete pick commits immediately, a panel tick is the caller's to - /// debounce and never feeds style recents. - var onPanelChange: @MainActor (String?) -> Void - - func makeNSView(context: Context) -> ColorComboControl { - let control = ColorComboControl(frame: .zero) - // The two zones' jobs, wired once (`ComboFieldControl`'s grammar): the face opens the same - // panel takeover **Other…** does, the trigger pops the dropdown. Closures, not target/action - // pairs — there is exactly one caller each and no `NSMenuItem`-style Objective-C boundary to - // cross for them. - control.onFaceClick = { [weak coordinator = context.coordinator] in - coordinator?.openColorPanel() - } - control.onTriggerClick = { [weak control] in - control?.popUpMenu() - } - return control - } - - func updateNSView(_ control: ColorComboControl, context: Context) { - context.coordinator.role = role - context.coordinator.onChange = onChange - context.coordinator.onPanelChange = onPanelChange - control.metrics = .current - control.isEnabled = isEnabled - context.coordinator.rebuild(control, value: value) - } - - /// Obeys an explicit finite proposal when SwiftUI hands one over — never the widest menu item, - /// which is what the old caller-supplied `width` input existed to work around. Falls back to the - /// control's own intrinsic width (`ComboFieldControl.intrinsicContentSize`, the 2026-08-09 - /// iteration's taller-narrower field) on an unconstrained measuring pass, which is the normal case - /// now that the sidebar row no longer forces a wider frame of its own (`CardStyleSection`). - func sizeThatFits(_ proposal: ProposedViewSize, nsView: ColorComboControl, context: Context) -> CGSize? { - let width: CGFloat - if let proposed = proposal.width, proposed.isFinite { - width = proposed - } else { - width = nsView.intrinsicContentSize.width - } - return CGSize(width: width, height: nsView.intrinsicContentSize.height) - } - - /// Detaches the colour panel's target/action if this coordinator still holds them — "last-writer - /// wins" for any control that took the panel over afterward (this view's own doc comment). - static func dismantleNSView(_ control: ColorComboControl, coordinator: Coordinator) { - coordinator.detachColorPanel() - } - - func makeCoordinator() -> Coordinator { - Coordinator(role: role, onChange: onChange, onPanelChange: onPanelChange) - } - - // MARK: Coordinator - - /// The one object every menu action and the colour panel's action target — a class because - /// `NSColorPanel.setTarget(_:)` needs something with reference identity to detach from later, - /// and `@MainActor` because every AppKit call it makes has to be. - @MainActor - final class Coordinator: NSObject { - fileprivate var role: ColorComboRole - fileprivate var currentValue: String? - fileprivate var onChange: @MainActor (String?) -> Void - fileprivate var onPanelChange: @MainActor (String?) -> Void - - /// ~44×14pt — a menu row's swatch, wide enough beside its title to read as a colour sample - /// rather than a bullet. - private static let menuSwatchSize = NSSize(width: 44, height: 14) - - /// This coordinator's handle on `NSColorPanel.shared` — the takeover, its continuous action - /// and its last-writer-wins detach all live in `SystemColorPanel` now, shared with the two - /// surfaces that gained an **Other…** on 2026-08-09. - private let colorPanel = SystemColorPanel() - - init( - role: ColorComboRole, - onChange: @escaping @MainActor (String?) -> Void, - onPanelChange: @escaping @MainActor (String?) -> Void - ) { - self.role = role - self.onChange = onChange - self.onPanelChange = onPanelChange - } - - /// Rebuilds the dropdown for `value` and hands the control the menu, its checked item (the - /// popup anchor `ColorComboControl.popUpMenu()` positions against, and the source of its - /// accessibility value), and the value its swatch zone should draw. Cheap enough — a dozen - /// rows, a fistful of small menu-row images — to redo wholesale on every SwiftUI update - /// rather than diffing against what was there before. - func rebuild(_ control: ColorComboControl, value: String?) { - currentValue = value - let menu = NSMenu() - let built = ColorComboModel.menu(role: role, value: value) - var checkedItem: NSMenuItem? - for (index, item) in built.items.enumerated() { - if item == .separator { - menu.addItem(.separator()) - continue - } - let menuItem = self.menuItem(for: item) - let isChecked = index == built.selectedIndex - menuItem.state = isChecked ? .on : .off - menu.addItem(menuItem) - if isChecked { checkedItem = menuItem } - } - control.comboMenu = menu - control.checkedItem = checkedItem - control.accessibilityValueText = checkedItem?.title - control.swatchValue = value - } - - /// See `ColorComboView.dismantleNSView(_:coordinator:)`. - func detachColorPanel() { - colorPanel.detach() - } - - private func menuItem(for item: ColorComboItem) -> NSMenuItem { - switch item { - case .none: - let menuItem = NSMenuItem(title: "None", action: #selector(selectNone), keyEquivalent: "") - menuItem.target = self - menuItem.image = PaletteSwatch.rectImage(for: nil, size: Self.menuSwatchSize) - return menuItem - - case let .palette(name): - let menuItem = NSMenuItem( - title: ColorComboModel.displayName(name), - action: #selector(selectValue(_:)), - keyEquivalent: "" - ) - menuItem.target = self - menuItem.representedObject = name - menuItem.image = PaletteSwatch.rectImage(for: name, size: Self.menuSwatchSize) - return menuItem - - case let .current(swatchValue, title): - let menuItem = NSMenuItem(title: title, action: #selector(selectValue(_:)), keyEquivalent: "") - menuItem.target = self - menuItem.representedObject = swatchValue - menuItem.image = PaletteSwatch.rectImage(for: swatchValue, size: Self.menuSwatchSize) - return menuItem - - case .other: - let menuItem = NSMenuItem(title: "Other…", action: #selector(openColorPanel), keyEquivalent: "") - menuItem.target = self - return menuItem - - case .separator: - // Unreached: `rebuild` handles `.separator` before calling this. Kept so the switch - // stays total against a case list a future row could still grow. - return NSMenuItem.separator() - } - } - - @objc private func selectNone() { - onChange(nil) - } - - @objc private func selectValue(_ sender: NSMenuItem) { - onChange(sender.representedObject as? String) - } - - /// Opens the shared Colors panel seeded with the current resolved colour, streaming what it - /// picks into `onPanelChange` — the palette name when the colour lands exactly on one of - /// `role`'s own entries, the hex otherwise. Every rule of that takeover is - /// `SystemColorPanel`'s; this is the seed and the destination. - /// - /// Two callers, one takeover: the dropdown's own **Other…** row (`#selector` target above) - /// and `ColorComboControl`'s face-zone click (wired in `ColorComboView.makeNSView`) — - /// Xcode's own two-zone combo opens the same panel from either half, and this is the one - /// place that happens. - @objc func openColorPanel() { - colorPanel.present( - seed: currentValue.flatMap(Palette.nsColor(for:)), - matching: role.palette - ) { [weak self] value in - self?.onPanelChange(value) - } - } - } -} - -// MARK: - The control - -/// The collapsed face: `ComboFieldControl` with a **swatch** in its face zone. -/// -/// Everything that makes it a two-zone combo — the field, the hairline, the trailing chevron square, -/// the hit split, the disabled compositing, the geometry — is the base class's now (ComboField.swift), -/// shared with the symbol combo so the two cannot drift apart. What is left here is the one thing that -/// is actually about *colour*: the swatch, and the menu the trigger pops. -/// -/// Plain internal, not `private`/`fileprivate`, even though nothing outside this file constructs one -/// directly: it is `ColorComboView`'s `NSViewType`, an associated-type witness the compiler requires -/// to be at least as visible as `ColorComboView` itself (internal, usable module-wide) — same -/// reasoning as the un-modified-access `Coordinator` a few lines up. -final class ColorComboControl: ComboFieldControl { - - /// The value the swatch zone currently draws — a palette name or a hand-written hex, exactly as - /// `ColorComboView.Coordinator.rebuild(_:value:)` hands it over on every SwiftUI update. - var swatchValue: String? { - didSet { - guard swatchValue != oldValue else { return } - needsDisplay = true - } - } - - /// The dropdown the trigger zone pops, and the row within it that should read as checked — both - /// `Coordinator.rebuild(_:value:)`'s to hand over on every rebuild, always together (`checkedItem` - /// is always one of `comboMenu`'s own items). This control never builds a row itself; it only - /// positions and pops what it is given. - var comboMenu: NSMenu? - var checkedItem: NSMenuItem? - - /// The colour rect, drawn exactly like `PaletteSwatch.rectImage`: a `textBackgroundColor` - /// underlay so a translucent stored colour composites the same way in light and dark, the - /// resolved colour on top, a `separatorColor` hairline stroke last. `nil`/unresolvable value → - /// underlay + stroke only, the same "there is no colour, so show none" rule. - /// - /// **No inset, since 2026-08-09** — the swatch fills the whole face zone rather than sitting in a - /// padded ring inside it (the owner's "remove padding from the combo"). - override func drawFace(in rect: NSRect) { - guard rect.width > 0, rect.height > 0 else { return } - let path = NSBezierPath(roundedRect: rect, xRadius: metrics.cornerRadius, yRadius: metrics.cornerRadius) - NSColor.textBackgroundColor.setFill() - path.fill() - if let swatchValue, let color = Palette.nsColor(for: swatchValue) { - color.setFill() - path.fill() - } - NSColor.separatorColor.setStroke() - path.lineWidth = 1 - path.stroke() - } - - /// Standard popup placement: `comboMenu` is asked to land `checkedItem` at the control's own top - /// edge, the same non-pulldown anchor `NSPopUpButton` itself uses so the checked row appears - /// where the control's own face is rather than wherever the pointer happened to be. - func popUpMenu() { - guard let comboMenu else { return } - comboMenu.popUp(positioning: checkedItem, at: NSPoint(x: 0, y: bounds.height), in: self) - } -} diff --git a/Kanban/UI/ColorSwatchPicker.swift b/Kanban/UI/ColorSwatchPicker.swift new file mode 100644 index 0000000..3786eb5 --- /dev/null +++ b/Kanban/UI/ColorSwatchPicker.swift @@ -0,0 +1,389 @@ +import AppKit +import SwiftUI + +/// **The colour rectangle** — a plain clickable swatch (`ColorSwatchControl`, `PickerRect.swift`'s +/// shared chrome) that opens a SwiftUI popover holding the background palette grid: **None**, the +/// role's own sixteen wells, and an **Other…** row onto the system Colors panel. +/// +/// ### The owner's reversal (2026-08-10, Pipeline card 5004c540, verbatim) +/// +/// "...lets do same for color, just a 4:6 ratio rectangle for color, no trigger, a grid of colors, and +/// a choice for 'Other' or 'More' to pull up generic color picker." +/// +/// This retires `ColorComboView` and its `NSMenu` dropdown outright — the swatch-faced two-zone combo +/// this file used to hold, whose trigger popped **None**, the role's own entries, an off-palette +/// "current" row and **Other…** as menu items. The dropdown's whole pure model (`ColorComboItem`, +/// `ColorComboMatch`, `ColorComboMenu`, `ColorComboModel.match`/`.menu`/`.displayName`/ +/// `.normalizedHex`) went with it: nothing left calls a menu into being, so nothing here builds one. +/// +/// `ColorSwatchRole` is the one surviving piece of the old pure model — which of the two palettes a +/// swatch offers. The new grid's wells are `role.palette`, matched by **literal name**, exactly the +/// rule `StyleEditorView.backgroundWells` already applies to the Style… popover's own background grid +/// — reusing that grid's own layout and matching convention rather than the retired dropdown's hex- +/// normalizing cleverness, so the app's two background grids agree about what "selected" means. An +/// off-palette hex that happens to equal a palette colour numerically no longer highlights that well; +/// it did in the old dropdown and does not in the Style… popover's grid either. +/// +/// ### The "Other…" row and the debounce +/// +/// Opens `SharedColorPanelSession` — the same shared, pre-debounced (~400ms) Colors-panel takeover +/// `StyleEditorView.openBackgroundPanel` and `SymbolPicker.openTintPanel` already use, because this +/// swatch's popover is transient exactly like theirs: opening the panel dismisses the popover, so a +/// per-view-owned panel session (what `ColorComboView`'s coordinator held) would have nothing left to +/// talk to. The write always goes through **`onPanelChange`, never `onChange`'s funnel** — "no +/// StyleRecents feed on drag ticks" (`StyleCommand.apply`'s recents feed is keyed to `background` +/// specifically, and a Colors-panel pick must never enter it, even the one settled value the debounce +/// finally delivers) — `StyleEditorView.openBackgroundPanel`'s own rule, restated here since this door +/// opens the identical session. + +// MARK: - Role + +/// Which of the two palettes a swatch offers — `Palette.backgrounds` for `background`, the only role +/// wired to a caller today; `Palette.foregrounds` stays for symmetry with `ColorSwatchRole`'s own +/// pre-2026-08-10 shape, unused but free. +enum ColorSwatchRole: Sendable, Equatable { + case background + case foreground + + var palette: [PaletteColor] { + switch self { + case .background: Palette.backgrounds + case .foreground: Palette.foregrounds + } + } +} + +// MARK: - Geometry + +/// The popover's own grid geometry — `SymbolPickerLayout`'s pattern, restated for a plain colour +/// grid: the well side and spacing read off `StyleEditorLayout`'s statics, scaled up by the same +/// `gridScale` the symbol popover's grid uses, and **seven columns** — `StyleEditorLayout.popover`'s +/// own settled fit for None + sixteen wells (7 + 7 + 3), reused here rather than re-derived, since it +/// is exactly this count of wells again. +struct ColorSwatchPopoverLayout: Equatable { + + static let columns = 7 + /// The grid's enlargement over the style editor's well size — `SymbolPickerLayout.gridScale`'s + /// own figure, so the two new-since-2026-08-10 popovers read at the same scale. + static let gridScale: CGFloat = 1.3 + + var wellSide: CGFloat + var wellSpacing: CGFloat + /// The popover's own inset, on all four sides, and the gap between the grid and the **Other…** + /// row below it. + var contentPadding: CGFloat + var gridWidth: CGFloat + /// The grid's width plus its padding on both sides — the popover's fixed width. + var popoverWidth: CGFloat + + static func metrics(bodyPointSize: CGFloat) -> ColorSwatchPopoverLayout { + let side = (StyleEditorLayout.wellSide(bodyPointSize: bodyPointSize) * gridScale).rounded() + let spacing = StyleEditorLayout.wellSpacing(bodyPointSize: bodyPointSize) + let padding = StyleEditorLayout.sectionSpacing(bodyPointSize: bodyPointSize) + let gridWidth = (side * CGFloat(columns) + spacing * CGFloat(columns - 1)).rounded() + return ColorSwatchPopoverLayout( + wellSide: side, + wellSpacing: spacing, + contentPadding: padding, + gridWidth: gridWidth, + popoverWidth: (gridWidth + padding * 2).rounded() + ) + } +} + +// MARK: - The picker + +/// The public surface: a colour rectangle plus its popover, `SymbolPicker`'s own shape one dimension +/// over. No store, no undo stack, no target — `onChange`/`onPanelChange` are the whole contract, the +/// same split `ColorComboView`'s `onChange`/`onPanelChange` offered, so `CardStyleSection` keeps its +/// two-closure wiring across the control swap. +struct ColorSwatchPicker: View { + + let role: ColorSwatchRole + /// The raw stored value — a palette name or a hand-written hex, exactly as the frontmatter field + /// carries it. + let value: String? + var isEnabled: Bool = true + /// A discrete pick — a grid well or **None**. Fired once, synchronously; the caller commits it + /// immediately and decides for itself whether it feeds `StyleRecents`. + let onChange: (String?) -> Void + /// A value settled from the Colors panel's **Other…** door — always raw, never through the + /// recents funnel (this file's own header). + let onPanelChange: (String?) -> Void + + /// The popover's presented flag — view-local `@State`, `SymbolPicker.isPopoverPresented`'s own + /// reasoning: a plain SwiftUI `.popover` nests correctly inside another one (the board popover + /// mounts a symbol rectangle the identical way), where a hand-driven `NSPopover` would not. + @State private var isPopoverPresented = false + + var body: some View { + ColorSwatchView(value: value, isEnabled: isEnabled, onClick: { isPopoverPresented = true }) + .help(helpText) + .accessibilityLabel(helpText) + .popover(isPresented: $isPopoverPresented, arrowEdge: .bottom) { + ColorSwatchPopoverContent( + current: value, + palette: role.palette, + layout: .metrics(bodyPointSize: CardWindowMetrics.bodyPointSize), + onSelect: { picked in + isPopoverPresented = false + onChange(picked) + }, + onPickOther: { + isPopoverPresented = false + openColorPanel() + } + ) + } + } + + private var helpText: String { + role == .background ? "Background" : "Color" + } + + /// **Other…** — the shared, debounced Colors panel, for the same outlives-the-popover reason + /// `SymbolPicker.openTintPanel` opens it: dismissing this popover would take a view-owned + /// coordinator with it before the panel finished streaming. + private func openColorPanel() { + SharedColorPanelSession.present( + seed: value.flatMap(Palette.nsColor(for:)), + matching: role.palette + ) { picked in + onPanelChange(picked) + } + } +} + +// MARK: - The AppKit bridge + +/// `ColorSwatchPicker`'s face: a `ColorSwatchControl` with its one zone wired to the caller's +/// closure. Deliberately dumb, `SymbolPicker`'s own `SymbolGlyphView` split: every decision about +/// what a click means lives in SwiftUI, where the popover's presented state already does. +private struct ColorSwatchView: NSViewRepresentable { + + let value: String? + let isEnabled: Bool + let onClick: () -> Void + + func makeNSView(context: Context) -> ColorSwatchControl { + ColorSwatchControl(frame: .zero) + } + + func updateNSView(_ control: ColorSwatchControl, context: Context) { + control.metrics = .current + control.isEnabled = isEnabled + control.swatchValue = value + control.accessibilityValueText = value ?? "None" + control.onClick = onClick + } + + /// Obeys an explicit finite proposal when SwiftUI hands one over, else falls back to the + /// control's own intrinsic width — `SymbolGlyphView.sizeThatFits`'s rule, restated so the two + /// rectangles answer a proposal identically. + func sizeThatFits(_ proposal: ProposedViewSize, nsView: ColorSwatchControl, context: Context) -> CGSize? { + let width: CGFloat + if let proposed = proposal.width, proposed.isFinite { + width = proposed + } else { + width = nsView.intrinsicContentSize.width + } + return CGSize(width: width, height: nsView.intrinsicContentSize.height) + } +} + +// MARK: - The control + +/// `PickerRectControl` with a **swatch** filling the whole face — the colour half of the shared +/// chrome, and the whole of what is specific to colour about it. +final class ColorSwatchControl: PickerRectControl { + + /// The value the face currently draws — a palette name or a hand-written hex, handed over on + /// every SwiftUI update. + var swatchValue: String? { + didSet { + guard swatchValue != oldValue else { return } + needsDisplay = true + } + } + + /// The colour rect: a `textBackgroundColor` underlay so a translucent stored colour composites + /// the same way in light and dark, the resolved colour on top, a `separatorColor` hairline + /// stroke last. `nil`/unresolvable value → underlay + stroke only, the same "there is no colour, + /// so show none" rule the retired combo's face drew — straight into the control's own graphics + /// context, exactly as before, rather than through `PaletteSwatch.rectImage`'s intermediate + /// `NSImage` (that function had exactly one caller, the retired dropdown's menu rows, and is + /// gone with it). + override func drawFace(in rect: NSRect) { + guard rect.width > 0, rect.height > 0 else { return } + let path = NSBezierPath(roundedRect: rect, xRadius: metrics.cornerRadius, yRadius: metrics.cornerRadius) + NSColor.textBackgroundColor.setFill() + path.fill() + if let swatchValue, let color = Palette.nsColor(for: swatchValue) { + color.setFill() + path.fill() + } + NSColor.separatorColor.setStroke() + path.lineWidth = 1 + path.stroke() + } +} + +// MARK: - The popover's content + +/// The popover's body: the palette grid, then **Other…** — `SymbolPickerPopoverContent`'s shape, one +/// grid narrower (no search, no second grid): this popover has exactly one dimension to offer. +private struct ColorSwatchPopoverContent: View { + + let current: String? + let palette: [PaletteColor] + let layout: ColorSwatchPopoverLayout + let onSelect: (String?) -> Void + let onPickOther: () -> Void + + var body: some View { + VStack(alignment: .leading, spacing: layout.wellSpacing) { + ColorSwatchWellGrid(current: current, palette: palette, layout: layout, onSelect: onSelect) + // Trailing-aligned link row onto the standalone panel — `SymbolPickerPopoverContent. + // deeperRow`'s own shape, restated: a well that is not a value is a well that has to be + // learned, and the row already spells this door as a titled affordance. + HStack(spacing: 0) { + Spacer(minLength: 0) + Button("Other…", action: onPickOther) + .buttonStyle(.link) + .font(.caption) + } + } + .padding(layout.contentPadding) + .frame(width: layout.popoverWidth) + } +} + +// MARK: - Wells + +/// One well in the grid: a palette name, or `nil` for the leading **None**. +private struct ColorSwatchWell: Identifiable { + let id: Int + let name: String? + let label: String + let isSelected: Bool +} + +/// The palette grid: the leading **None** well and `palette`'s own entries, seven wide — +/// `StyleWellGrid`'s pattern (StyleEditor.swift), mirrored rather than shared for the reason +/// `SymbolWellGrid`/`SymbolColorGrid` (SymbolPicker.swift) already mirror it instead of widening that +/// file's `private` access for one caller outside it. +private struct ColorSwatchWellGrid: View { + + let current: String? + let palette: [PaletteColor] + let layout: ColorSwatchPopoverLayout + let onSelect: (String?) -> Void + + @FocusState private var focused: Int? + @Environment(\.colorSchemeContrast) private var contrast + + var body: some View { + LazyVGrid( + columns: Array( + repeating: GridItem(.flexible(minimum: layout.wellSide), spacing: layout.wellSpacing), + count: ColorSwatchPopoverLayout.columns + ), + spacing: layout.wellSpacing + ) { + ForEach(wells) { well in + Button { + onSelect(well.name) + } label: { + swatch(well.name.flatMap(Palette.color(named:))) + .overlay(selectionRing(well.isSelected)) + .contentShape(Rectangle()) + } + .buttonStyle(.plain) + .focusable() + .focused($focused, equals: well.id) + .help(well.label) + .accessibilityLabel(well.label) + .accessibilityAddTraits(well.isSelected ? [.isSelected] : []) + } + } + .onKeyPress(keys: [.leftArrow, .rightArrow, .upArrow, .downArrow], phases: .down) { press in + move(press.key) + } + } + + /// **None** leads, selected whenever `current` is absent — literal-name matching only + /// (`ColorSwatchPicker`'s own header), so an off-palette hex leaves every well unselected exactly + /// as `StyleEditorView.backgroundWells` already does. + private var wells: [ColorSwatchWell] { + var wells = [ColorSwatchWell(id: 0, name: nil, label: "None", isSelected: current == nil)] + for (index, color) in palette.enumerated() { + wells.append(ColorSwatchWell( + id: index + 1, + name: color.name, + label: color.name, + isSelected: current == color.name + )) + } + return wells + } + + /// A colour swatch, always stroked (`chalk` is `#FFFFFF`; an unbordered white swatch is an + /// invisible control), with the corner-to-corner slash standing in for "no colour" on the None + /// well — `StyleWellFace`'s own vocabulary, restated. + private func swatch(_ color: Color?) -> some View { + RoundedRectangle(cornerRadius: cornerRadius) + .fill(color ?? Color(nsColor: .textBackgroundColor)) + .overlay { + if color == nil { + ColorSwatchNoneStrike(inset: max(1, (layout.wellSide * 0.15).rounded())) + .stroke(.secondary, lineWidth: Accommodations.borderWidth(1, contrast: contrast)) + } + } + .overlay( + RoundedRectangle(cornerRadius: cornerRadius) + .strokeBorder(.separator, lineWidth: Accommodations.borderWidth(1, contrast: contrast)) + ) + .frame(width: layout.wellSide, height: layout.wellSide) + } + + private var cornerRadius: CGFloat { max(1, (layout.wellSide * 0.2).rounded()) } + + private func selectionRing(_ isSelected: Bool) -> some View { + RoundedRectangle(cornerRadius: cornerRadius) + .strokeBorder( + isSelected ? AnyShapeStyle(Color.accentColor) : AnyShapeStyle(.clear), + lineWidth: Accommodations.borderWidth(2, contrast: contrast) + ) + .padding(-Accommodations.borderWidth(2, contrast: contrast) / 2) + } + + /// `SymbolWellGrid.move(_:)`'s rule, at this grid's own seven columns. + private func move(_ key: KeyEquivalent) -> KeyPress.Result { + let delta: Int + switch key { + case .leftArrow: delta = -1 + case .rightArrow: delta = 1 + case .upArrow: delta = -ColorSwatchPopoverLayout.columns + case .downArrow: delta = ColorSwatchPopoverLayout.columns + default: return .ignored + } + let current = focused ?? 0 + let next = min(max(0, current + delta), wells.count - 1) + focused = next + return .handled + } +} + +/// The None well's corner-to-corner slash — `StyleEditor.swift`'s `NoValueStrike`/`SymbolPicker. +/// swift`'s `ColorNoneStrike`, restated as a third sibling for the same reason those two are already +/// siblings rather than one shared exported shape. +private struct ColorSwatchNoneStrike: Shape { + let inset: CGFloat + + func path(in rect: CGRect) -> Path { + var path = Path() + path.move(to: CGPoint(x: rect.minX + inset, y: rect.maxY - inset)) + path.addLine(to: CGPoint(x: rect.maxX - inset, y: rect.minY + inset)) + return path + } +} diff --git a/Kanban/UI/ComboField.swift b/Kanban/UI/ComboField.swift deleted file mode 100644 index de58af5..0000000 --- a/Kanban/UI/ComboField.swift +++ /dev/null @@ -1,329 +0,0 @@ -import AppKit - -/// **The two-zone combo, as chrome both pickers share** — a bordered field with a large *face* and a -/// narrow accent-coloured *trigger* at the trailing edge, Xcode's inspector colour combo's own shape. -/// -/// It exists because "the two pickers should rhyme in terms of UX" (2026-08-09) is only true as long -/// as nobody edits one of them. `ColorComboControl` drew this shape first and drew it alone; a second -/// control drawing "the same" shape from its own constants would be one careless padding tweak away -/// from two controls that merely used to match. So the shape moved here and both subclass it: the -/// colour combo's face is a swatch, the symbol combo's is a glyph, and **every other pixel is this -/// file's** — width, height, radius, hairline, the trigger square, the disabled treatment, and the -/// hit split between the two zones. -/// -/// ### The grammar the two zones carry -/// -/// Identical on both controls, which is the whole point: -/// -/// - **The face** opens the *standalone* picker — `NSColorPanel.shared` for a colour -/// (`SystemColorPanel`), the symbol browser for a glyph (`SymbolBrowserPanel`). A window that -/// stays up while the user keeps choosing. -/// - **The trigger** pops the *quick* list — the colour dropdown's palette rows, the symbol -/// popover's curated grid. A transient surface that closes on a pick. -/// -/// Each quick list also carries a row onto the standalone picker (**Other…**, **More Symbols…**), -/// which is deliberate rather than redundant: the face zone is pointer-only — `keyDown` below reaches -/// the trigger, and there is no second key that would obviously mean "the other zone" — so the row -/// inside the popover is how the keyboard gets to the deep door at all. -/// -/// ### Everything scales with the body font -/// -/// 10-accessibility.md's full-relative-scaling rule, which the hard-coded constants this file -/// replaces did not follow. Every figure is a multiple of the body point size, chosen to reproduce -/// the numbers `ColorComboControl` shipped with at the standard 13pt body — with one deliberate -/// exception, `height`. -/// -/// ### The 2026-08-09 iterations: no padding, a taller narrower field, a centred glyph — then half -/// the height back off again -/// -/// The owner's first review of the shape above: "remove padding from the combo. make the field -/// taller and narrower. almost square. about 4:5 ratio. for symbol picker center the symbol." Three -/// changes, all in this file — -/// -/// - **No padding.** `facePaddingH`/`facePaddingV`/`glyphPadding` are gone; a face draws to fill its -/// whole zone, not a rect inset inside it. A swatch is now a solid patch flush with the field's own -/// edges rather than a bar floating in a ring of `controlColor`. -/// - **`width` joined `height` as a real figure**, replacing `NSView.noIntrinsicMetric`. The first -/// pass let every caller stretch the control to whatever width it had lying around — 93pt in the -/// card sidebar, 120pt's fallback in the board popover — which is the wide-bar shape this iteration -/// undoes. -/// - **The glyph centres in both axes.** It always claimed to (`SymbolComboControl.drawFace`'s own -/// doc comment said "centred") but the code only ever centred it vertically and offset it from the -/// leading edge by the swatch's own horizontal padding — dead code once that padding left, and the -/// wrong rect even before it did. -/// -/// That pass reached "almost square" by growing `height` to 4:5 against a `width` that was purely -/// `height`'s own restatement (`widthToHeightRatio`). The owner's evening review of the result: "reduce -/// vertical size of both pickers by 50%, on symbol picker by reducing padding" — the taller field read -/// as too tall once it was on screen, superseding the 4:5 ruling outright rather than adjusting it. -/// `height` is cut in half here (back to the very figure the first pass shipped, before "almost square" -/// ever entered the conversation). `width` had to stop being a function of `height` to survive that cut: -/// restating the old 0.8 ratio at the new height would ask for a 14pt-wide field, narrower than the -/// 16pt trigger strip alone, which drives `faceWidth` negative-then-clamped-to-zero and empties the -/// board popover anchor that sizes itself from this control's intrinsic width. So `width` is now its -/// own em figure, holding the *pixel width* the 4:5 pass shipped rather than any ratio to `height`. -/// -/// **The net shape is a short wide bar again, not the 4:5 pass's portrait** — 18×29 at the standard -/// body, width now the larger of the two figures. That is closer to the very first pass's spirit -/// (a field wider than tall) than to "almost square", which is the point: the evening review is a -/// reversal of the 4:5 ruling, not a smaller step along the same road. -struct ComboFieldMetrics: Equatable { - - /// The control's height. - /// - /// **1.4 em is 18pt at the standard body — exactly half of the 4:5 pass's 36**, and incidentally - /// the very figure the original pass before that one shipped. The owner's evening review asked for - /// "reduce vertical size of both pickers by 50%" outright, not a re-tuning against the trigger or - /// the glyph; halving the shipped 36 is that request applied literally. - var height: CGFloat - /// The whole control's width — its own em figure now, not a scaling of `height`. **2.2 em is 29pt - /// at the standard body, the exact pixel width the 4:5 pass shipped** (`36 × 0.8`, rounded) — this - /// iteration holds that width steady rather than re-deriving it from the now-shorter height, since - /// a re-derivation would starve the trigger strip and empty the face zone (this type's own header). - /// Every caller still gets this figure unless it hands SwiftUI an explicit finite proposal of its - /// own, which `sizeThatFits` on each representable still honours exactly as before. - var width: CGFloat - /// The trigger strip at the trailing edge, full height. Unchanged from the first pass: this - /// iteration narrows the *field*, not the door onto the quick list, and 1.25 em already gave the - /// trigger square room for a legible chevron. - var triggerWidth: CGFloat - /// The trigger square's inset from the strip's height — kept off the *ring* width rather than - /// a face padding that no longer exists, so the indicator stays a legible square rather than - /// growing to fill whichever of the strip's two dimensions is smaller (`ComboFieldControl. - /// drawTrigger`'s `min(width, height)`, which is what keeps this inset meaningful regardless of - /// which dimension ends up binding at a given body size). - var triggerInset: CGFloat - /// The face's and the trigger square's radius. - var cornerRadius: CGFloat - /// The field's own radius — a point more than the face's, so the two rounded rects run - /// concentric instead of pinching at the corners. - var fieldRadius: CGFloat - - static func metrics(bodyPointSize: CGFloat) -> ComboFieldMetrics { - func em(_ multiple: CGFloat) -> CGFloat { max(1, (bodyPointSize * multiple).rounded()) } - return ComboFieldMetrics( - height: em(1.4), - width: em(2.2), - triggerWidth: em(1.25), - triggerInset: em(0.15), - cornerRadius: em(0.23), - fieldRadius: em(0.31) - ) - } - - /// The live metrics — read from the same place every other font-derived geometry in the app - /// reads it (`CardWindowMetrics.bodyPointSize`), so a text-size change moves the combos with - /// everything else. - @MainActor - static var current: ComboFieldMetrics { metrics(bodyPointSize: CardWindowMetrics.bodyPointSize) } - - /// The face zone's own width — the whole field less the trigger strip. Computed here rather than - /// read off a live `NSView`'s `faceZone` so it is a plain function of `bodyPointSize`, available - /// to a pure test with no control on screen. Kept positive by construction now that `width` is its - /// own em figure rather than a ratio of `height` (this type's own header): the evening 2026-08-09 - /// cut halved `height` alone, and a `width` still deriving from it would have driven this to zero. - var faceWidth: CGFloat { max(0, width - triggerWidth) } -} - -// MARK: - Glyph sizing moved to the actual face rect - -// `ComboFieldMetrics` no longer carries a `glyphPointSize`. It used to (`max(1, min(faceWidth, -// height))`), a pure figure derived from the *intrinsic* portrait size — correct only when a control -// actually renders at its intrinsic width, which the card sidebar's row never lets it do (`sizeThatFits` -// honours the row's real width proposal). `SymbolComboControl.drawFace` now sizes its glyph off the -// `rect` it is actually handed, which is this type's whole reason to still track `faceWidth`: a pure -// test can assert that figure stays positive without a control on screen, even though nothing draws -// from it directly anymore. - -// MARK: - The control - -/// The shared field. Subclasses draw the face and answer the two zones; nothing else here is theirs -/// to change. -/// -/// Plain internal rather than `private`: both subclasses live in other files, and each is an -/// `NSViewRepresentable`'s `NSViewType`, an associated-type witness the compiler requires to be at -/// least as visible as the representable itself. -class ComboFieldControl: NSControl { - - /// The geometry every pass draws from, refreshed by the representable on each SwiftUI update so - /// a text-size change lands without recreating the view. - var metrics: ComboFieldMetrics = .metrics(bodyPointSize: 13) { - didSet { - guard metrics != oldValue else { return } - invalidateIntrinsicContentSize() - needsDisplay = true - } - } - - /// A click anywhere in the face zone — the standalone picker's door. - /// - /// Unannotated rather than `@MainActor`: this class is an `NSResponder` subclass and therefore - /// already main-actor isolated, so every call site below is on the main actor by construction — - /// and the annotation would only force every caller to hand over a `@Sendable` closure it has no - /// reason to be. - var onFaceClick: (() -> Void)? - /// A click in the trigger zone, or Space/Return — the quick list's door. - var onTriggerClick: (() -> Void)? - - /// What VoiceOver reads as this control's value: the checked dropdown row's title, the resolved - /// symbol's name. Set by whichever coordinator owns the control. - var accessibilityValueText: String? - - override var isEnabled: Bool { - get { super.isEnabled } - set { - super.isEnabled = newValue - needsDisplay = true - } - } - - /// Both dimensions are the metrics' own now. Width stopped being `NSView.noIntrinsicMetric` in - /// the 2026-08-09 iteration that gave the field a fixed shape instead of whatever a caller's frame - /// happened to propose; a representable's `sizeThatFits` still honours an explicit finite proposal - /// over this default, exactly as before. - override var intrinsicContentSize: NSSize { - NSSize(width: metrics.width, height: metrics.height) - } - - // MARK: Zones - - var triggerRect: NSRect { - NSRect( - x: bounds.maxX - metrics.triggerWidth, - y: bounds.minY, - width: metrics.triggerWidth, - height: bounds.height - ) - } - - var faceZone: NSRect { - NSRect( - x: bounds.minX, - y: bounds.minY, - width: max(0, bounds.width - metrics.triggerWidth), - height: bounds.height - ) - } - - // MARK: Drawing - - override func draw(_ dirtyRect: NSRect) { - guard let context = NSGraphicsContext.current?.cgContext else { return } - context.saveGState() - defer { context.restoreGState() } - // A transparency layer, not a flat `setAlpha` around each shape: the face's underlay, fill - // and stroke overlap, and drawing each at reduced alpha independently would let the stroke - // double up over the fill beneath it. Compositing the whole disabled face as one layer - // avoids that. - if !isEnabled { - context.setAlpha(0.35) - context.beginTransparencyLayer(auxiliaryInfo: nil) - } - drawField() - drawFace(in: faceZone) - drawTrigger() - if !isEnabled { - context.endTransparencyLayer() - } - } - - /// The subclass's half: whatever belongs in the face zone. The base draws nothing. - func drawFace(in rect: NSRect) {} - - /// The control's own field: a bordered, filled rounded rect over the whole bounds, under both - /// zones — what makes the face and the trigger read as one control rather than two shapes - /// floating beside each other. `controlColor` fill — the push-button neutral grey, not - /// `controlBackgroundColor`, whose near-black dark-mode reading drowned the padding ring — - /// `separatorColor` hairline, the half-point inset keeping the stroke on whole pixels. - private func drawField() { - let path = NSBezierPath( - roundedRect: bounds.insetBy(dx: 0.5, dy: 0.5), - xRadius: metrics.fieldRadius, - yRadius: metrics.fieldRadius - ) - NSColor.controlColor.setFill() - path.fill() - NSColor.separatorColor.setStroke() - path.lineWidth = 1 - path.stroke() - } - - /// The trigger: a small centred rounded square, `controlAccentColor`-filled, with a white - /// `chevron.up.chevron.down` centred inside — the standard `NSPopUpButton` indicator's own look, - /// redrawn here since this control has no bezel of its own to borrow one from. - /// - /// **The side is bounded by both of the strip's own dimensions**, not just its height: the first - /// pass only took `triggerRect.height` because the strip was never far from square, but the - /// 2026-08-09 iterations moved `height` and `width` independently of one another (this file's - /// header), and a side taken from height alone would ask for a square wider than the strip itself - /// on whichever body size that stopped holding. - private func drawTrigger() { - let side = min(triggerRect.width, triggerRect.height) - 2 * metrics.triggerInset - let square = NSRect( - x: triggerRect.midX - side / 2, - y: triggerRect.midY - side / 2, - width: side, - height: side - ) - let path = NSBezierPath(roundedRect: square, xRadius: metrics.cornerRadius, yRadius: metrics.cornerRadius) - NSColor.controlAccentColor.setFill() - path.fill() - - let config = NSImage.SymbolConfiguration(pointSize: (side * 0.6).rounded(), weight: .bold) - .applying(.init(paletteColors: [.white])) - guard let chevron = NSImage(systemSymbolName: "chevron.up.chevron.down", accessibilityDescription: nil)? - .withSymbolConfiguration(config) - else { return } - let size = chevron.size - chevron.draw(in: NSRect( - x: square.midX - size.width / 2, - y: square.midY - size.height / 2, - width: size.width, - height: size.height - )) - } - - // MARK: Events - - /// Point-in-trigger-zone pops the quick list; anywhere else fires the face. A disabled control - /// answers neither. - override func mouseDown(with event: NSEvent) { - guard isEnabled else { return } - let point = convert(event.locationInWindow, from: nil) - if triggerRect.contains(point) { - onTriggerClick?() - } else { - onFaceClick?() - } - } - - override var acceptsFirstResponder: Bool { isEnabled } - - /// Space and Return pop the quick list — the one keyboard path into this control. The face zone - /// has no key of its own on purpose; the standalone picker is reached from *inside* the quick - /// list instead (this file's header). - override func keyDown(with event: NSEvent) { - guard isEnabled else { - super.keyDown(with: event) - return - } - switch event.keyCode { - case 49, 36, 76: // Space, Return, keypad Enter - onTriggerClick?() - default: - super.keyDown(with: event) - } - } - - // MARK: Accessibility - - override func accessibilityRole() -> NSAccessibility.Role? { .popUpButton } - - override func accessibilityValue() -> Any? { accessibilityValueText } - - override func accessibilityPerformPress() -> Bool { - guard isEnabled else { return false } - onTriggerClick?() - return true - } -} diff --git a/Kanban/UI/Palette.swift b/Kanban/UI/Palette.swift index aaacfce..20a45d5 100644 --- a/Kanban/UI/Palette.swift +++ b/Kanban/UI/Palette.swift @@ -118,12 +118,10 @@ enum Palette { } // The pathfinder's panel round-trip helpers, ported below (`NSColor.paletteHexString`, -// `Palette.name(forHex:in:)`): the reusable colour-picker combo (`ColorComboView`, ColorCombo.swift) -// is the surface that finally needs them — a colour the *system picker* returns has to become a -// stored value the same way a palette pick already does: the palette NAME when the colour lands -// exactly on one of the twelve, the hex otherwise. Its swatch drawing, unchanged in spirit, is -// below: a menu can only render `Image`/`Text`, so both the quick-style row's dots and the combo's -// rows have to be pictures. +// `Palette.name(forHex:in:)`): the reusable colour-picker rectangle (`ColorSwatchPicker`, +// ColorSwatchPicker.swift) is the surface that finally needs them — a colour the *system picker* +// returns has to become a stored value the same way a palette pick already does: the palette NAME +// when the colour lands exactly on one of the twelve, the hex otherwise. // MARK: - Menu swatches @@ -131,6 +129,11 @@ enum Palette { /// items**. AppKit renders a menu row from its label's image and text, so the quick-style recents row /// (03-board-ui.md § Styling ▸ Controls) needs an `NSImage` per dot where the editor's own wells are /// ordinary views. +/// +/// **`rectImage`, the wide sibling this enum used to carry, is retired (2026-08-10).** It built the +/// `NSMenuItem` images for `ColorComboView`'s dropdown rows and its collapsed face — both gone with +/// that control (`ColorSwatchPicker.swift`'s own header). `circleImage` below is the one caller left: +/// the quick-style recents row is still a menu, and a menu can still only render `Image`/`Text`. enum PaletteSwatch { /// A filled dot for `value` (a palette name or `#RRGGBB[AA]` hex), hairline-bordered. @@ -159,28 +162,6 @@ enum PaletteSwatch { return true } } - - /// A wide rectangular swatch for `value` — `ColorComboView`'s own rows and its collapsed face, - /// which are wide and short rather than the quick-style row's small dots (hence a sibling - /// function rather than a parameter on `circleImage`: the two shapes are never interchangeable at - /// their call sites). `nil` draws the border alone, exactly `circleImage`'s "there is no colour, - /// so show none" — the collapsed face's **None** state and the dropdown's own **None** row both - /// call this with `nil` rather than a sentinel string. - static func rectImage(for value: String?, size: NSSize) -> NSImage { - let color = value.flatMap(Palette.nsColor(for:)) - return NSImage(size: size, flipped: false) { rect in - let inset = rect.insetBy(dx: 0.5, dy: 0.5) - let path = NSBezierPath(rect: inset) - NSColor.textBackgroundColor.setFill() - path.fill() - color?.setFill() - path.fill() - NSColor.separatorColor.setStroke() - path.lineWidth = 1 - path.stroke() - return true - } - } } extension Palette { @@ -215,10 +196,10 @@ extension Palette { } /// The name of `palette`'s entry whose hex matches `hex`, case-insensitively — the pathfinder's - /// round trip, ported for `ColorComboView`'s panel handoff: a colour the system picker returns - /// comes back as `NSColor.paletteHexString`'s canonical `#RRGGBB[AA]`, and this is what turns - /// that back into "the user picked Light Cayenne" instead of leaving it as an anonymous hex. - /// `nil` when nothing in `palette` matches, which the caller reads as "store the hex instead." + /// round trip, ported for `SystemColorPanel`'s own panel handoff: a colour the system picker + /// returns comes back as `NSColor.paletteHexString`'s canonical `#RRGGBB[AA]`, and this is what + /// turns that back into "the user picked Light Cayenne" instead of leaving it as an anonymous + /// hex. `nil` when nothing in `palette` matches, which the caller reads as "store the hex instead." static func name(forHex hex: String, in palette: [PaletteColor]) -> String? { palette.first { $0.hex.caseInsensitiveCompare(hex) == .orderedSame }?.name } diff --git a/Kanban/UI/PickerRect.swift b/Kanban/UI/PickerRect.swift new file mode 100644 index 0000000..a275ed8 --- /dev/null +++ b/Kanban/UI/PickerRect.swift @@ -0,0 +1,182 @@ +import AppKit + +/// **The shared single-zone rectangle** — the chrome behind both pickers (`SymbolGlyphControl` in +/// SymbolPicker.swift, `ColorSwatchControl` in ColorSwatchPicker.swift): a bordered, filled rounded +/// rect with exactly **one** hit zone, the whole of it. A click anywhere opens the control's popover. +/// +/// ### The owner's reversal (2026-08-10, Pipeline card 5004c540, verbatim) +/// +/// "lets revert to previous look of the symbol picker, no combo trigger. just a rectangle (slightly +/// oversized) clickable to show a popover with grid of symbols and colors (same popover that shows up +/// on click of combo trigger). lets do same for color..." +/// +/// This retires the two-zone combo chrome this file used to hold (`ComboFieldControl`/ +/// `ComboFieldMetrics`, the 2026-08-09 "the two pickers should rhyme" pass): the face/trigger split, +/// the `NSPopUpButton`-style chevron square at the trailing edge, and the face zone's own door onto a +/// *standalone* picker independent of the popover. What survives, because it was never about the +/// trigger: the bordered rounded-rect field (`drawField`), the disabled treatment (a single +/// transparency layer under both), and — because a popover is still the one way into a standalone +/// picker (`SymbolBrowserPanel`'s **More Symbols…** row, the new colour popover's **Other…** row) — +/// the `.popUpButton` accessibility role and the value text a coordinator hands over. +/// +/// ### Sizing (the same ruling): "50% taller... and about 4:6 ratio of height to width" +/// +/// `height` is **2.1 em** — 50% over the two-zone chrome's last-shipped 1.4 em (18pt → 27pt at the +/// standard 13pt body). `width` is `height` **times 1.5** — the 4:6 ratio stated as a multiplier on +/// `height` rather than as its own independent em figure, so the ratio holds exactly (to rounding) at +/// every body size instead of two multiples that could drift apart the way `ComboFieldMetrics`' +/// `height` and `width` once did (that file's own retired header told that story). 10-accessibility.md's +/// full-relative-scaling rule, unchanged: every figure is a multiple of the body point size. +struct PickerRectMetrics: Equatable { + + /// The control's height — 2.1 em, 27pt at the standard 13pt body. + var height: CGFloat + /// The control's width — `height × 1.5`, so the 4:6 ratio is exact rather than approximated by + /// two independently rounded em figures. + var width: CGFloat + /// The rectangle's own radius, and the field's — a point apart so the two rounded rects run + /// concentric. Unchanged from the two-zone chrome's own figures; nothing about the trigger's + /// removal touches these. + var cornerRadius: CGFloat + var fieldRadius: CGFloat + + static func metrics(bodyPointSize: CGFloat) -> PickerRectMetrics { + func em(_ multiple: CGFloat) -> CGFloat { max(1, (bodyPointSize * multiple).rounded()) } + let height = em(2.1) + let width = max(1, (height * 1.5).rounded()) + return PickerRectMetrics( + height: height, + width: width, + cornerRadius: em(0.23), + fieldRadius: em(0.31) + ) + } + + /// The live metrics — read from the same place every other font-derived geometry in the app + /// reads it (`CardWindowMetrics.bodyPointSize`), so a text-size change moves both rectangles with + /// everything else. + @MainActor + static var current: PickerRectMetrics { metrics(bodyPointSize: CardWindowMetrics.bodyPointSize) } +} + +// MARK: - The control + +/// The shared rectangle. Subclasses draw whatever fills it; nothing else here is theirs to change. +/// +/// Plain internal rather than `private`: both subclasses live in other files, and each is an +/// `NSViewRepresentable`'s `NSViewType`, an associated-type witness the compiler requires to be at +/// least as visible as the representable itself. +class PickerRectControl: NSControl { + + /// The geometry every pass draws from, refreshed by the representable on each SwiftUI update so + /// a text-size change lands without recreating the view. + var metrics: PickerRectMetrics = .metrics(bodyPointSize: 13) { + didSet { + guard metrics != oldValue else { return } + invalidateIntrinsicContentSize() + needsDisplay = true + } + } + + /// The one click this control answers — anywhere in its bounds, since there is only the one zone + /// now. Opens whichever popover the caller wired. + /// + /// Unannotated rather than `@MainActor`: this class is an `NSResponder` subclass and therefore + /// already main-actor isolated, so every call site is on the main actor by construction. + var onClick: (() -> Void)? + + /// What VoiceOver reads as this control's value: the resolved symbol's name, the swatch's stored + /// value. Set by whichever coordinator owns the control. + var accessibilityValueText: String? + + override var isEnabled: Bool { + get { super.isEnabled } + set { + super.isEnabled = newValue + needsDisplay = true + } + } + + override var intrinsicContentSize: NSSize { + NSSize(width: metrics.width, height: metrics.height) + } + + // MARK: Drawing + + override func draw(_ dirtyRect: NSRect) { + guard let context = NSGraphicsContext.current?.cgContext else { return } + context.saveGState() + defer { context.restoreGState() } + // A transparency layer, not a flat `setAlpha` around each shape: the face's underlay, fill + // and stroke overlap, and drawing each at reduced alpha independently would let the stroke + // double up over the fill beneath it. + if !isEnabled { + context.setAlpha(0.35) + context.beginTransparencyLayer(auxiliaryInfo: nil) + } + drawField() + drawFace(in: bounds) + if !isEnabled { + context.endTransparencyLayer() + } + } + + /// The subclass's half: whatever fills the whole rectangle. The base draws nothing. Handed the + /// control's own `bounds` — there is no zone to carve out of it any more. + func drawFace(in rect: NSRect) {} + + /// The control's own field: a bordered, filled rounded rect over the whole bounds, under the + /// face — `controlColor` fill, `separatorColor` hairline, the half-point inset keeping the + /// stroke on whole pixels. Unchanged from the two-zone chrome. + private func drawField() { + let path = NSBezierPath( + roundedRect: bounds.insetBy(dx: 0.5, dy: 0.5), + xRadius: metrics.fieldRadius, + yRadius: metrics.fieldRadius + ) + NSColor.controlColor.setFill() + path.fill() + NSColor.separatorColor.setStroke() + path.lineWidth = 1 + path.stroke() + } + + // MARK: Events + + /// Anywhere in the control, full stop — there is only the one zone now. + override func mouseDown(with event: NSEvent) { + guard isEnabled else { return } + onClick?() + } + + override var acceptsFirstResponder: Bool { isEnabled } + + /// Space and Return open the popover — the one keyboard path into this control, the same keys + /// the two-zone chrome's trigger answered. + override func keyDown(with event: NSEvent) { + guard isEnabled else { + super.keyDown(with: event) + return + } + switch event.keyCode { + case 49, 36, 76: // Space, Return, keypad Enter + onClick?() + default: + super.keyDown(with: event) + } + } + + // MARK: Accessibility + + /// `.popUpButton` — unchanged from the two-zone chrome: this control still opens a set of + /// choices, it just does so from one hit zone instead of two. + override func accessibilityRole() -> NSAccessibility.Role? { .popUpButton } + + override func accessibilityValue() -> Any? { accessibilityValueText } + + override func accessibilityPerformPress() -> Bool { + guard isEnabled else { return false } + onClick?() + return true + } +} diff --git a/Kanban/UI/StyleEditor.swift b/Kanban/UI/StyleEditor.swift index 3de8e97..40aa940 100644 --- a/Kanban/UI/StyleEditor.swift +++ b/Kanban/UI/StyleEditor.swift @@ -341,10 +341,10 @@ struct StyleEditorView: View { var showsSymbols: Bool = true /// Whether the background section appears at all. On everywhere but the card window sidebar, - /// where the labeled `ColorComboView` row is the background story (03 ▸ Styling ▸ Controls, - /// the 2026-08-06 anchor-ownership rule): the sidebar is the narrow context the combo was - /// built for, and grid-plus-combo over one value read as two settings — `showsSymbols`' - /// reasoning, pointed the other way. + /// where the captioned `ColorSwatchPicker` column is the background story (03 ▸ Styling ▸ + /// Controls, the 2026-08-06 anchor-ownership rule): the sidebar is the narrow context that + /// rectangle was built for, and grid-plus-rectangle over one value read as two settings — + /// `showsSymbols`' reasoning, pointed the other way. var showsBackground: Bool = true /// The live body metric, read here rather than passed in — `CardStyleSection`'s pattern, so @@ -404,10 +404,12 @@ struct StyleEditorView: View { /// BackgroundField.swift's original "there is no image picker and none is planned"). /// /// **"Palette-only in-app" is the sentence that changed**, and it changed before this card: - /// `ColorComboView` shipped an **Other…** row of its own, so the card window's sidebar could - /// already write an arbitrary hex while the Style… popover — the *primary* styling surface — - /// could not. This closes that gap rather than opening a new one. Seventeen wells at seven - /// columns is three rows where the old twelve made two, which is the other half of the same change. + /// the card sidebar's background control shipped an **Other…** row of its own, so the card + /// window's sidebar could already write an arbitrary hex while the Style… popover — the + /// *primary* styling surface — could not. This closes that gap rather than opening a new one. + /// Seventeen wells at seven columns is three rows where the old twelve made two, which is the + /// other half of the same change — and, since 2026-08-10, the same 7-wide fit `ColorSwatchPicker` + /// reuses for its own popover grid. /// /// **Choose Image… is absent, not disabled, off the board.** `background.image` is a board-root /// field alone — a lane or a card has a colour well and nothing else to point an image subkey at diff --git a/Kanban/UI/SymbolBrowser.swift b/Kanban/UI/SymbolBrowser.swift index 55a68e6..92c254a 100644 --- a/Kanban/UI/SymbolBrowser.swift +++ b/Kanban/UI/SymbolBrowser.swift @@ -120,7 +120,7 @@ final class SymbolBrowserModel { var query: String = "" /// The presenting surface's write. `@ObservationIgnored` because nothing observes it, and - /// unannotated for `ComboFieldControl.onFaceClick`'s reason — this class is already main-actor + /// unannotated for `PickerRectControl.onClick`'s reason — this class is already main-actor /// isolated, so the closure is called there by construction. @ObservationIgnored var onSelect: ((String?) -> Void)? diff --git a/Kanban/UI/SymbolPicker.swift b/Kanban/UI/SymbolPicker.swift index 684f754..c0aef00 100644 --- a/Kanban/UI/SymbolPicker.swift +++ b/Kanban/UI/SymbolPicker.swift @@ -1,10 +1,10 @@ import AppKit import SwiftUI -/// **A reusable SF Symbol picker** — a two-zone combo showing the resolved symbol, opening a curated -/// grid from its trigger and the standalone browser from its face (03-board-ui.md § Styling ▸ -/// Controls: "its leading well is the level's default symbol and removes the `icon` key … Any other -/// SF Symbol name works written by hand"). `StyleEditor.swift` +/// **A reusable SF Symbol picker** — a single clickable rectangle (`PickerRect.swift`'s shared +/// chrome) showing the resolved symbol, opening a curated grid popover on any click (03-board-ui.md § +/// Styling ▸ Controls: "its leading well is the level's default symbol and removes the `icon` key … +/// Any other SF Symbol name works written by hand"). `StyleEditor.swift` /// already builds that grid once, aimed at `background`/`icon` together and multiplexed across three /// anchors; this file builds the *symbol half alone*, aimed at any single field a caller names, so a /// control that only ever needs one glyph — a saved search, a smart filter, a future per-item @@ -29,23 +29,22 @@ import SwiftUI /// search with nothing to search would just move the dead end from "no matching well" to "no way to /// look further". /// -/// ### Shape and grammar (2026-08-09): the two pickers rhyme +/// ### Shape and grammar /// -/// This control used to be a single 20pt bordered square with one hit zone, sitting in a sidebar -/// beside a wide two-zone colour combo that had a *second* door onto `NSColorPanel.shared`. Two -/// controls setting adjacent frontmatter keys, looking and behaving nothing alike. Now it is a -/// `ComboFieldControl` like the colour combo — same field, same height, same trailing chevron, by -/// construction rather than by agreement — with the same two-zone grammar: +/// This control used to be a single 20pt bordered square with one hit zone, then (2026-08-09) grew a +/// second, trigger, zone to rhyme with the colour combo's own two-zone shape — the face opening +/// `SymbolBrowserPanel` directly, the trigger popping this curated grid. The owner's 2026-08-10 +/// reversal (Pipeline card 5004c540: "revert to previous look of the symbol picker, no combo trigger +/// … just a rectangle … clickable to show a popover with grid of symbols and colors") retired the +/// second zone outright: this is a `PickerRectControl` (`PickerRect.swift`) with exactly **one** hit +/// zone, and that one zone always opens the curated popover. `SymbolBrowserPanel`, the standalone +/// searchable/categorised browser, is reachable now **only** through the popover's own **More +/// Symbols…** row — which is also, as it always was, the only *keyboard* route to it, since a plain +/// rectangle has no second key that would mean "the other door." /// -/// - **the face** opens `SymbolBrowserPanel`, the standalone searchable/categorised browser, exactly -/// as the colour combo's swatch opens the Colors panel; -/// - **the trigger** pops the curated popover this file always had. -/// -/// And that popover gained the two rows the grammar implies: **More Symbols…** onto the browser (the -/// dropdown's **Other…**, one dimension over — and the only *keyboard* route to the face zone's -/// door), and an **Other…** under the tint grid onto the Colors panel. The tint grid itself grew from -/// 4×2 to 4×3, which is what the palette went from twelve entries to sixteen to make room for -/// (`Palette.tints`). +/// The popover still carries the two rows that grammar implies: **More Symbols…** onto the browser, +/// and an **Other…** under the tint grid onto the Colors panel. The tint grid itself is 4×3, eleven +/// tints plus a leading None (`Palette.tints`) — unrelated to this card and unchanged by it. // MARK: - The symbol catalogs @@ -156,8 +155,8 @@ enum SymbolPickerCatalog { /// is this popover's whole subject, where the style editor's is one section among several), still /// anchored to the shared base so the two components move together at every text size. /// -/// **The at-rest control's geometry is not here** — it is `ComboFieldMetrics`', shared with the -/// colour combo, which is what makes the two the same size (2026-08-09). This type now describes the +/// **The at-rest control's geometry is not here** — it is `PickerRectMetrics`', shared with the +/// colour rectangle, which is what makes the two the same size. This type now describes the /// popover alone. Only the shape wraps a picker's own frame around them — six /// columns fixed (not a /// caller-configurable count, since a picker has no anchor-width story the way `StyleEditorLayout`'s @@ -220,14 +219,15 @@ struct SymbolPickerLayout: Equatable { // MARK: - The control -/// A two-zone symbol combo — the reusable primitive a caller can aim at one symbol field without -/// wiring up a `BoardStore`, a `StyleTarget`, or the two-dimension batch machinery `StyleEditorView` -/// carries for the board's own background+icon editor. +/// A single-rectangle symbol picker — the reusable primitive a caller can aim at one symbol field +/// without wiring up a `BoardStore`, a `StyleTarget`, or the two-dimension batch machinery +/// `StyleEditorView` carries for the board's own background+icon editor. /// /// **No store, no undo stack, no target set** is known to this type; `onSelect`/`onSelectColor` are /// the whole of its contract with a caller, exactly as a `Picker`'s `selection` binding would be. -/// That is unchanged by the 2026-08-09 rework — the API is byte for byte what it was, so every -/// existing caller kept working while the control underneath became a `ComboFieldControl`. +/// That contract's API is byte for byte what it was through both the 2026-08-09 two-zone rework and +/// the 2026-08-10 reversal back to one — every existing caller kept working while the control +/// underneath changed shape twice. struct SymbolPicker: View { /// The committed symbol name, or `nil` for "no override" — read alongside `fallback` rather than @@ -270,13 +270,12 @@ struct SymbolPicker: View { @State private var isPopoverPresented = false var body: some View { - SymbolComboView( + SymbolGlyphView( current: current, fallback: fallback, isEnabled: isEnabled, currentColor: onSelectColor == nil ? nil : currentColor, - onFace: { openBrowser() }, - onTrigger: { isPopoverPresented = true } + onClick: { isPopoverPresented = true } ) .help("Symbol") .accessibilityLabel("Symbol") @@ -291,8 +290,9 @@ struct SymbolPicker: View { isPopoverPresented = false onSelect(name) }, - // **More Symbols…** — the popover's route to the face zone's door, and the only one - // the keyboard has (`ComboField.swift`'s note on why the face has no key of its own). + // **More Symbols…** — the popover's route to the standalone browser, and the only + // way there at all now that the rectangle itself has no second zone or second key + // (`PickerRect.swift`'s grammar). onBrowse: { isPopoverPresented = false openBrowser() @@ -304,7 +304,7 @@ struct SymbolPicker: View { select(name) } }, - // **Other…** under the tint grid — the colour combo's own row, one field over. + // **Other…** under the tint grid — the colour rectangle's own row, one field over. onPickCustomColor: onSelectColor.map { select in { isPopoverPresented = false @@ -315,8 +315,8 @@ struct SymbolPicker: View { } } - /// The face zone's door, and **More Symbols…**' — one call, because they are one gesture spelled - /// two ways (`ComboField.swift`'s grammar). + /// **More Symbols…**'s door onto the standalone browser — the popover's only route there now + /// that the rectangle itself has no second zone of its own (`PickerRect.swift`'s grammar). /// /// The closure handed over **outlives this view on purpose**. Opening the browser dismisses /// whichever popover this picker was mounted in, which would take a view-owned closure with it and @@ -341,17 +341,17 @@ struct SymbolPicker: View { // MARK: - The AppKit bridge -/// `SymbolPicker`'s face: a `SymbolComboControl` with its two zones wired to the caller's closures. +/// `SymbolPicker`'s face: a `SymbolGlyphControl` with its one zone wired to the caller's closure. /// /// Deliberately **dumb** — it draws and it reports clicks. Every decision about what a click *means* /// (which popover, which panel, what closure survives what) is `SymbolPicker`'s, in SwiftUI, where -/// the presentation modifiers and the state that drives them already live. `ColorComboView` carries a -/// coordinator because its dropdown is an `NSMenu` that has to be built in AppKit; this one has no -/// such need and gets no such thing. +/// the presentation modifiers and the state that drives them already live. Neither this view nor +/// `ColorSwatchPicker`'s own AppKit bridge carries a coordinator any more — the last thing that +/// needed one was the retired combo's `NSMenu` dropdown. /// /// `.disabled(_:)` reaches it through `@Environment(\.isEnabled)` on `SymbolPicker` rather than a /// parameter, since callers already spell the lock that way (`CardStyleSection`, `BoardInfoPopover`). -private struct SymbolComboView: NSViewRepresentable { +private struct SymbolGlyphView: NSViewRepresentable { let current: String? let fallback: String @@ -359,8 +359,7 @@ private struct SymbolComboView: NSViewRepresentable { /// The live tint, or `nil` for the standard label colour — already gated by the caller on whether /// a colour row is offered at all, so this view has no opinion about it. let currentColor: String? - let onFace: () -> Void - let onTrigger: () -> Void + let onClick: () -> Void /// What the face draws — `current` if this system can resolve it, `fallback` otherwise. The /// lenient rule `ItemSymbol.name(_:fallback:)` states for a `FieldValue`, restated because this @@ -370,27 +369,27 @@ private struct SymbolComboView: NSViewRepresentable { return fallback } - func makeNSView(context: Context) -> SymbolComboControl { - SymbolComboControl(frame: .zero) + func makeNSView(context: Context) -> SymbolGlyphControl { + SymbolGlyphControl(frame: .zero) } - func updateNSView(_ control: SymbolComboControl, context: Context) { + func updateNSView(_ control: SymbolGlyphControl, context: Context) { control.metrics = .current control.isEnabled = isEnabled control.glyphName = resolvedName control.glyphTint = currentColor.flatMap(Palette.nsColor(for:)) control.accessibilityValueText = resolvedName - // Rewired on every update rather than once in `makeNSView`: the closures close over this - // struct's current values, and a stale one would aim the browser at the field this picker + // Rewired on every update rather than once in `makeNSView`: the closure closes over this + // struct's current values, and a stale one would open the popover for the field this picker // used to be pointed at. - control.onFaceClick = onFace - control.onTriggerClick = onTrigger + control.onClick = onClick } /// Obeys an explicit finite proposal when SwiftUI hands one over, else falls back to the control's - /// own intrinsic width — `ColorComboView.sizeThatFits`'s rule, restated so the two combos answer a - /// proposal identically and a caller that frames them alike gets two controls the same size. - func sizeThatFits(_ proposal: ProposedViewSize, nsView: SymbolComboControl, context: Context) -> CGSize? { + /// own intrinsic width — `ColorSwatchView.sizeThatFits`'s rule, restated so the two rectangles + /// answer a proposal identically and a caller that frames them alike gets two controls the same + /// size. + func sizeThatFits(_ proposal: ProposedViewSize, nsView: SymbolGlyphControl, context: Context) -> CGSize? { let width: CGFloat if let proposed = proposal.width, proposed.isFinite { width = proposed @@ -403,9 +402,9 @@ private struct SymbolComboView: NSViewRepresentable { // MARK: - The face -/// `ComboFieldControl` with a **glyph** in its face zone — the symbol half of the shared chrome, and -/// the whole of what is specific to symbols about it. -final class SymbolComboControl: ComboFieldControl { +/// `PickerRectControl` with a **glyph** filling the whole face — the symbol half of the shared +/// chrome, and the whole of what is specific to symbols about it. +final class SymbolGlyphControl: PickerRectControl { /// The resolved name the face draws — the coordinator's `resolvedName`, handed over on every /// SwiftUI update. @@ -433,7 +432,7 @@ final class SymbolComboControl: ComboFieldControl { /// horizontal padding, which read as left-aligned once the glyph was more than a sliver narrower /// than the face. The owner's review named this directly ("for symbol picker center the symbol"). /// - /// **Sized off the actual `rect`, not `ComboFieldMetrics.glyphPointSize`, since the evening + /// **Sized off the actual `rect`, not a pure metrics figure, since the evening /// 2026-08-09 review.** That figure (now removed) was a pure derivation from the *intrinsic* /// portrait width, but `sizeThatFits` obeys any finite width SwiftUI proposes, and the card /// sidebar's row proposes real width — so the control renders wide while a glyph pinned at the diff --git a/Kanban/UI/SystemColorPanel.swift b/Kanban/UI/SystemColorPanel.swift index 5a7c435..da8abfb 100644 --- a/Kanban/UI/SystemColorPanel.swift +++ b/Kanban/UI/SystemColorPanel.swift @@ -3,29 +3,29 @@ import AppKit /// **The shared Colors panel, as one takeover every surface can borrow** — `NSColorPanel.shared` /// seeded, targeted, and translated back into the app's stored-value vocabulary. /// -/// This is `ColorComboView.Coordinator`'s panel handling, lifted out of it. That coordinator was the -/// only thing in the app that opened the system picker, so the logic could live inside it; now three -/// surfaces do — the colour combo's face and its **Other…** row, the Style… popover's Background -/// section, and the symbol popover's tint section (2026-08-09) — and the parts that must not be -/// re-derived are the awkward ones: `NSColorPanel` is a **singleton with a settable target and no -/// getter for it**, so "is the panel still mine to detach from" has nowhere to live but in a static, -/// and a second surface taking the panel over has to leave the first one's teardown harmless. +/// This used to be `ColorComboView.Coordinator`'s panel handling, lifted out of it when a second +/// surface needed the identical takeover: `NSColorPanel` is a **singleton with a settable target and +/// no getter for it**, so "is the panel still mine to detach from" has nowhere to live but in a +/// static, and a second surface taking the panel over has to leave the first one's teardown harmless. +/// `ColorComboView` is retired now (2026-08-10, `ColorSwatchPicker.swift`'s own header), and every +/// caller left — the Style… popover's Background section, the symbol popover's tint section, and the +/// card sidebar's own colour rectangle — reaches this class through the one shared instance +/// `SharedColorPanelSession` holds, below. /// /// ### What it hands back /// /// Not an `NSColor` — the **stored string**: the palette name when the picked colour lands exactly -/// on one of the caller's own palette entries, `#RRGGBB[AA]` otherwise. That rule (name wins over -/// hex) is the same one `ColorComboModel.match` applies to a value already on disk, which is what -/// makes a colour survive a round trip through the panel without drifting from `light-cayenne` into -/// an anonymous `#B6071E`. +/// on one of the caller's own palette entries, `#RRGGBB[AA]` otherwise (`changeColor(_:)` below). That +/// is what makes a colour survive a round trip through the panel without drifting from +/// `light-cayenne` into an anonymous `#B6071E`. /// /// ### It fires continuously /// /// `NSColorPanel`'s action runs on every tick of a drag, not on release, and that is deliberate: a /// live preview is the point of opening the panel at all. **Every caller therefore owes a debounce** -/// before the value reaches disk — `CardStyleSection.debounceBackground` is the pattern, ~400ms -/// trailing, and a drag that passes through a palette-exact colour mid-gesture must not be recorded -/// in `StyleRecents` the way a deliberate pick is. +/// before the value reaches disk — `SharedColorPanelSession.present`'s own ~400ms trailing debounce is +/// that owed debounce for every caller today, and a drag that passes through a palette-exact colour +/// mid-gesture must not be recorded in `StyleRecents` the way a deliberate pick is. @MainActor final class SystemColorPanel: NSObject { @@ -84,23 +84,27 @@ final class SystemColorPanel: NSObject { /// **The Colors panel for surfaces that do not outlive it, debounce included.** /// -/// The obvious shape is a `SystemColorPanel` in the opening view's own state, and that is exactly -/// what `CardStyleSection` does — correctly, because a card window's sidebar outlives any panel -/// opened from it. It does **not** work for the two surfaces that gained an **Other…** on -/// 2026-08-09: the Style… popover's Background section and the symbol popover's tint section are -/// both inside *transient popovers*. Opening the Colors panel takes key status away, the popover -/// dismisses, the view's state goes with it — and the panel the user is now dragging in has nothing -/// left to talk to. The gesture would break itself. +/// The obvious shape is a `SystemColorPanel` in the opening view's own state — and it does **not** +/// work for a surface whose door onto the panel sits *inside a transient popover*: the Style… +/// popover's Background section, the symbol popover's tint section, and, since 2026-08-10, the card +/// sidebar's own colour rectangle's popover (`ColorSwatchPicker`'s **Other…** row) are all like this. +/// Opening the Colors panel takes key status away, the popover dismisses, the view's state goes with +/// it — and the panel the user is now dragging in has nothing left to talk to. The gesture would +/// break itself. (`CardStyleSection` used to be the one exception — a card window's sidebar outlives +/// any panel opened from it, so it held its own `SystemColorPanel` and its own debounce `Task` +/// directly. It reaches this shared session too now that its own colour control opens the panel from +/// *inside* a popover rather than from a permanently-mounted face.) /// /// So the session lives above every view: one panel, one pending write, replaced by whichever /// surface opened it last. Callers capture their store **weakly**, so a board closed while its /// Colors panel is still up drops the write instead of resurrecting a dead store. /// -/// The debounce is `CardStyleSection.debounceBackground`'s, restated because the state it needs now -/// has to live somewhere a dismissed popover cannot take with it: the panel's action is continuous, -/// and only the value the user is still on ~400ms after the last tick reaches disk. A drag is -/// therefore one write, and never feeds `StyleRecents` — that row remembers deliberate palette -/// picks, not colours a drag swept through. +/// The debounce lives here because the state it needs has to live somewhere a dismissed popover +/// cannot take with it: the panel's action is continuous, and only the value the user is still on +/// ~400ms after the last tick reaches disk. A drag is therefore one write, and every caller keeps +/// that one settled value **out of** `StyleRecents` — that row remembers deliberate palette picks, +/// not colours a drag swept through, so every `write` closure above writes raw rather than through +/// `StyleCommand.apply`. @MainActor enum SharedColorPanelSession { diff --git a/KanbanTests/ColorComboTests.swift b/KanbanTests/ColorComboTests.swift deleted file mode 100644 index 6fe2694..0000000 --- a/KanbanTests/ColorComboTests.swift +++ /dev/null @@ -1,197 +0,0 @@ -import AppKit -import Testing -@testable import Kanban - -/// `ColorComboView`'s pure model (`ColorCombo.swift`): item-list composition, selection matching, -/// hex normalization and display-name casing — every rule stated where a test can hold it without -/// an `NSView`, matching `PaletteTests.swift`'s own split between the vocabulary and the view. -/// -/// One outer suite, nested by concern — swift-testing discovers nested types as sub-suites, which -/// is what lets `-only-testing:KanbanTests/ColorComboTests` run the whole file while each concern -/// still reads as its own group, `PaletteTests.swift`'s top-level-structs style scoped one level in. -struct ColorComboTests { - - // MARK: - Display names - - struct DisplayName { - - @Test func kebabCaseBecomesTitleCaseWithHyphensAsSpaces() { - #expect(ColorComboModel.displayName("light-cayenne") == "Light Cayenne") - #expect(ColorComboModel.displayName("smokey-rich-eggplant") == "Smokey Rich Eggplant") - #expect(ColorComboModel.displayName("obsidian") == "Obsidian") - #expect(ColorComboModel.displayName("deep-sky-blue") == "Deep Sky Blue") - } - } - - // MARK: - Hex normalization - - struct HexNormalization { - - @Test func sixDigitHexUppercasesUnchanged() { - #expect(ColorComboModel.normalizedHex("#b6071e") == "#B6071E") - #expect(ColorComboModel.normalizedHex("#B6071E") == "#B6071E") - } - - /// Alpha below full opacity is meaningful and stays — only a fully-opaque suffix collapses. - @Test func eightDigitHexWithPartialAlphaStaysEightDigits() { - #expect(ColorComboModel.normalizedHex("#b6071e80") == "#B6071E80") - } - - /// `#RRGGBBFF` — fully opaque, spelled with an explicit alpha byte — collapses to the - /// six-digit form, so it compares equal to a bare `#RRGGBB` written for the same colour. - @Test func fullyOpaqueEightDigitHexCollapsesToSixDigits() { - #expect(ColorComboModel.normalizedHex("#b6071eFF") == "#B6071E") - #expect(ColorComboModel.normalizedHex("#B6071EFF") == ColorComboModel.normalizedHex("#B6071E")) - } - - @Test func malformedOrUnprefixedValuesNormalizeToNil() { - for value in ["", "#", "#12", "#12345", "#1234567", "#GGGGGG", "B6071E", "light-cayenne"] { - #expect(ColorComboModel.normalizedHex(value) == nil, "'\(value)' should not normalize") - } - } - } - - // MARK: - Selection matching - - struct Matching { - - @Test func nilValueMatchesNone() { - #expect(ColorComboModel.match(role: .background, value: nil) == .none) - #expect(ColorComboModel.match(role: .foreground, value: nil) == .none) - } - - @Test func aNameInTheRolesOwnPaletteMatchesThatRowExactly() { - #expect(ColorComboModel.match(role: .background, value: "light-cayenne") == .palette("light-cayenne")) - #expect(ColorComboModel.match(role: .foreground, value: "fern") == .palette("fern")) - } - - /// Names are matched exactly, like `Palette.nsColor(for:)` — a near-miss is not "the same - /// row", it is an off-palette value with its own dynamic row. - @Test func aNearMissNameIsNotAPaletteMatch() { - let match = ColorComboModel.match(role: .background, value: "Light-Cayenne") - guard case let .current(_, title) = match else { - Issue.record("expected .current, got \(match)") - return - } - #expect(title == "Light-Cayenne") - } - - /// A hex that normalizes to one of the role's own palette hexes selects the **name**, not - /// the hex — case-insensitively, and with a fully-opaque `#RRGGBBFF` collapsing exactly like - /// a bare `#RRGGBB` would. - @Test func aHexEqualToAPaletteColorsHexMatchesItsNamedRow() { - #expect(ColorComboModel.match(role: .background, value: "#B6071E") == .palette("light-cayenne")) - #expect(ColorComboModel.match(role: .background, value: "#b6071e") == .palette("light-cayenne")) - #expect(ColorComboModel.match(role: .background, value: "#B6071EFF") == .palette("light-cayenne")) - #expect(ColorComboModel.match(role: .background, value: "#b6071eff") == .palette("light-cayenne")) - } - - /// Partial alpha keeps a value off the palette rows even when its RGB matches one exactly — - /// the stored colour is genuinely translucent, which no palette entry is. - @Test func aTranslucentHexNeverMatchesAnOpaquePaletteColor() { - let match = ColorComboModel.match(role: .background, value: "#B6071E80") - guard case let .current(swatchValue, _) = match else { - Issue.record("expected .current, got \(match)") - return - } - #expect(swatchValue == "#B6071E80") - } - - /// A name from the *other* picker's table — `carnation` is foreground-only — is not one of - /// `.background`'s own table, so it falls to the dynamic row, titled with its own display name - /// since the other table does know it. - @Test func aForeignPaletteNameFallsToTheDynamicRowNamedFromTheOtherTable() { - let match = ColorComboModel.match(role: .background, value: "carnation") - #expect(match == .current(swatchValue: "carnation", title: "Carnation")) - } - - /// A custom hex nowhere in either table: the dynamic row states it verbatim, uppercased. - @Test func aCustomHexFallsToTheDynamicRowUppercased() { - let match = ColorComboModel.match(role: .background, value: "#123456") - #expect(match == .current(swatchValue: "#123456", title: "#123456")) - let lowercase = ColorComboModel.match(role: .background, value: "#abcdef") - #expect(lowercase == .current(swatchValue: "#abcdef", title: "#ABCDEF")) - } - - /// Unresolvable garbage — neither a name either table knows nor a parseable hex — falls to - /// the dynamic row exactly as written, no casing applied. - @Test func garbageFallsToTheDynamicRowVerbatim() { - let match = ColorComboModel.match(role: .background, value: "chartreuse") - #expect(match == .current(swatchValue: "chartreuse", title: "chartreuse")) - } - - /// The three names shared by both tables (`obsidian`, `aluminum`, `chalk`) are in *both* - /// roles' own palettes, so they match directly and never reach the "foreign name" branch. - @Test func namesSharedByBothTablesMatchDirectlyInEitherRole() { - #expect(ColorComboModel.match(role: .background, value: "obsidian") == .palette("obsidian")) - #expect(ColorComboModel.match(role: .foreground, value: "obsidian") == .palette("obsidian")) - } - } - - // MARK: - Item list composition - - struct Menu { - - /// None first, a separator, then exactly the role's own entries, in the palette's own order. - @Test func baseOrderIsNoneSeparatorThenTheRolesTwelve() { - let menu = ColorComboModel.menu(role: .background, value: nil) - var expected: [ColorComboItem] = [.none, .separator] - expected.append(contentsOf: Palette.backgrounds.map { .palette($0.name) }) - expected.append(contentsOf: [.separator, .other]) - #expect(menu.items == expected) - } - - @Test func foregroundRoleListsTheForegroundTwelveNotTheBackgroundTwelve() { - let menu = ColorComboModel.menu(role: .foreground, value: nil) - let paletteNames = menu.items.compactMap { item -> String? in - if case let .palette(name) = item { return name } - return nil - } - #expect(paletteNames == Palette.foregrounds.map(\.name)) - } - - /// `Other…` is always last, and there is never more than one dynamic row. - @Test func otherIsAlwaysLast() { - for value in [nil, "obsidian", "carnation", "#123456", "chartreuse"] { - let menu = ColorComboModel.menu(role: .background, value: value) - #expect(menu.items.last == .other) - } - } - - /// No dynamic row, and no selected item beyond the palette rows, when the value is `nil` or - /// one of the role's own entries. - @Test func noDynamicRowWhenTheValueIsNoneOrAPaletteName() { - let none = ColorComboModel.menu(role: .background, value: nil) - #expect(!none.items.contains { if case .current = $0 { return true }; return false }) - #expect(none.selectedIndex == 0) - #expect(none.items[none.selectedIndex] == .none) - - let named = ColorComboModel.menu(role: .background, value: "dark-teal") - #expect(!named.items.contains { if case .current = $0 { return true }; return false }) - #expect(named.items[named.selectedIndex] == .palette("dark-teal")) - } - - /// A foreign or unresolvable value inserts exactly one dynamic row, immediately before the - /// trailing separator and `Other…`, and it is the checked row. - @Test func dynamicRowAppearsOnlyForAForeignValueAndIsSelected() { - let menu = ColorComboModel.menu(role: .background, value: "#123456") - let dynamicRows = menu.items.filter { if case .current = $0 { return true }; return false } - #expect(dynamicRows.count == 1) - #expect(menu.items[menu.selectedIndex] == .current(swatchValue: "#123456", title: "#123456")) - // Immediately before the trailing separator + Other… - #expect( - Array(menu.items.suffix(3)) == - [.current(swatchValue: "#123456", title: "#123456"), .separator, .other] - ) - } - - /// A hex landing exactly on a palette colour selects that named row and adds no dynamic row - /// at all — the same shape as picking the name directly. - @Test func anExactHexMatchProducesNoDynamicRow() { - let byName = ColorComboModel.menu(role: .background, value: "light-cayenne") - let byHex = ColorComboModel.menu(role: .background, value: "#B6071E") - #expect(byName.items == byHex.items) - #expect(byName.selectedIndex == byHex.selectedIndex) - } - } -} diff --git a/KanbanTests/ColorSwatchPickerTests.swift b/KanbanTests/ColorSwatchPickerTests.swift new file mode 100644 index 0000000..fd1459e --- /dev/null +++ b/KanbanTests/ColorSwatchPickerTests.swift @@ -0,0 +1,90 @@ +import AppKit +import Testing +@testable import Kanban + +/// **`ColorSwatchPicker`'s pure pieces** (`ColorSwatchPicker.swift`), the 2026-08-10 rework that +/// retired `ColorComboView`'s `NSMenu` dropdown and its pure model — `ColorComboItem`, `ColorComboMatch`, +/// `ColorComboMenu`, `ColorComboModel.match`/`.menu`/`.displayName`/`.normalizedHex` — wholesale. This +/// file used to hold that model's tests; nothing in it survives the dropdown's removal, so it now +/// tests what replaced it: which palette a role offers, and the popover's own grid geometry. + +// MARK: - Role + +@Suite("ColorSwatchRole ▸ which palette") +struct ColorSwatchRoleTests { + + @Test("Background lists the backgrounds table, foreground the foregrounds table") + func rolesListTheirOwnTable() { + #expect(ColorSwatchRole.background.palette.map(\.name) == Palette.backgrounds.map(\.name)) + #expect(ColorSwatchRole.foreground.palette.map(\.name) == Palette.foregrounds.map(\.name)) + } +} + +// MARK: - Popover geometry + +/// **`SymbolPickerLayout`'s pattern, restated for a plain colour grid.** The layout is pure and +/// font-derived (10-accessibility.md's full-relative-scaling rule), so every claim below is +/// assertable without a popover on screen. +@Suite("ColorSwatchPopoverLayout ▸ geometry") +struct ColorSwatchPopoverLayoutTests { + + /// How wide `columns` wells and the gaps between them actually draw, at a given text size — + /// `StyleEditorLayoutTests.gridWidth`'s own helper, restated for this layout's fixed seven. + private func gridWidth(bodyPointSize: CGFloat) -> CGFloat { + let columns = ColorSwatchPopoverLayout.columns + let layout = ColorSwatchPopoverLayout.metrics(bodyPointSize: bodyPointSize) + return CGFloat(columns) * layout.wellSide + CGFloat(columns - 1) * layout.wellSpacing + } + + @Test("Seven columns — None + sixteen wells fall as 7 + 7 + 3, StyleEditorLayout.popover's own settled fit") + func columnsAreSeven() { + #expect(ColorSwatchPopoverLayout.columns == 7) + // None + Palette.backgrounds (16) is 17 wells; seven columns is the count that keeps the + // last row from overflowing to four rows or wasting a nearly-empty one. + let wellCount = 1 + Palette.backgrounds.count + let rows = (wellCount + ColorSwatchPopoverLayout.columns - 1) / ColorSwatchPopoverLayout.columns + #expect(rows == 3) + } + + @Test("The grid's well side matches the symbol popover's own scale-up over the shared base") + func wellSideMatchesTheSymbolPopoversScale() { + for size in [11.0, 13.0, 17.0, 24.0] as [CGFloat] { + let color = ColorSwatchPopoverLayout.metrics(bodyPointSize: size) + let symbol = SymbolPickerLayout.metrics(bodyPointSize: size) + // Both scale `StyleEditorLayout.wellSide` by the identical `gridScale` — the two + // popovers introduced together (2026-08-10) read at one scale. + #expect(color.wellSide == symbol.wellSide, "the two popovers' wells drifted apart at \(size)pt") + #expect(ColorSwatchPopoverLayout.gridScale == SymbolPickerLayout.gridScale) + } + } + + @Test("The popover's frame grows with the text, and its wells keep fitting inside it") + func popoverScalesAndFits() { + var previousWidth: CGFloat = 0 + for size in [11.0, 13.0, 16.0, 18.0, 24.0, 36.0] as [CGFloat] { + let layout = ColorSwatchPopoverLayout.metrics(bodyPointSize: size) + #expect(layout.popoverWidth > previousWidth, "the popover must widen with the text at \(size)pt") + previousWidth = layout.popoverWidth + + let content = layout.popoverWidth - 2 * layout.contentPadding + #expect(gridWidth(bodyPointSize: size) <= content, "the grid overflows its popover at \(size)pt") + } + } + + @Test("The popover's settled geometry at the standard text size") + func settledGeometryAtTheStandardBody() { + let layout = ColorSwatchPopoverLayout.metrics(bodyPointSize: 13) + // `StyleEditorLayout`'s own statics at 13pt: wellSpacing 6, sectionSpacing 14. Well side is + // the style editor's own 20pt scaled by `gridScale`, computed the identical way the + // implementation does rather than restated as a literal — `20 * 1.3` is not exactly + // representable in binary floating point, and comparing two independently-rounded literals + // is exactly the kind of thing that can drift a bit apart from the real computation. + #expect(layout.wellSpacing == 6) + #expect(layout.contentPadding == 14) + let expectedSide = (StyleEditorLayout.wellSide(bodyPointSize: 13) * ColorSwatchPopoverLayout.gridScale).rounded() + #expect(layout.wellSide == expectedSide) + #expect(layout.wellSide == 26) + #expect(layout.gridWidth == layout.wellSide * 7 + layout.wellSpacing * 6) + #expect(layout.popoverWidth == layout.gridWidth + layout.contentPadding * 2) + } +} diff --git a/KanbanTests/CustomColorRoundTripTests.swift b/KanbanTests/CustomColorRoundTripTests.swift index 17fe39f..ec8ed88 100644 --- a/KanbanTests/CustomColorRoundTripTests.swift +++ b/KanbanTests/CustomColorRoundTripTests.swift @@ -40,8 +40,9 @@ struct CustomColorCodecTests { /// **Full opacity collapses to six digits.** A colour the user never touched the opacity slider /// on has to be written exactly as a curated palette entry would be, or it would never match one - /// (`ColorComboModel.match` compares normalized strings) and a panel pick that landed dead on - /// `fern` would store an anonymous hex instead of the name. + /// (`Palette.name(forHex:in:)`'s case-insensitive compare, `SystemColorPanel.changeColor(_:)`'s + /// own rule) and a panel pick that landed dead on `fern` would store an anonymous hex instead of + /// the name. @Test("Opacity is written only when there is some") func alphaCollapsesAtFullOpacity() throws { let opaque = NSColor(srgbRed: 0.5, green: 0.25, blue: 0.75, alpha: 1) diff --git a/KanbanTests/PaletteTests.swift b/KanbanTests/PaletteTests.swift index 59bcbd6..ce3b84d 100644 --- a/KanbanTests/PaletteTests.swift +++ b/KanbanTests/PaletteTests.swift @@ -47,9 +47,10 @@ struct PaletteTableTests { } /// Every hex is spelled the one way the app emits them — uppercase `#RRGGBB` — so a panel pick - /// that lands on a palette colour matches by string (`ColorComboModel.match`'s hex branch) and - /// comes back as the *name*. A lowercase entry would still resolve and would still round-trip; - /// it would just quietly stop being recognised as the palette colour it is. + /// that lands on a palette colour matches by string (`Palette.name(forHex:in:)`, `SystemColorPanel. + /// changeColor(_:)`'s own case-insensitive compare) and comes back as the *name*. A lowercase + /// entry would still resolve and would still round-trip; it would just quietly stop being + /// recognised as the palette colour it is. @Test func everyHexIsSixUppercaseDigits() { for entry in Palette.foregrounds + Palette.backgrounds { #expect(entry.hex == entry.hex.uppercased(), "'\(entry.name)' is not uppercase") diff --git a/KanbanTests/SymbolCatalogTests.swift b/KanbanTests/SymbolCatalogTests.swift index 75c1869..db88e8e 100644 --- a/KanbanTests/SymbolCatalogTests.swift +++ b/KanbanTests/SymbolCatalogTests.swift @@ -212,55 +212,50 @@ struct SymbolCatalogSearchTests { } } -// MARK: - The shared combo chrome +// MARK: - The shared rectangle chrome -/// **The rhyme, asserted.** The card's first ask was that the two pickers be "roughly same -/// shape/size", and the way that was made true is structural — one metrics value, one base control — -/// so the test is about the structure rather than about two numbers that happen to agree today. -@Suite("ComboField ▸ the shared chrome") -struct ComboFieldMetricsTests { +/// **The rhyme, asserted.** The owner's 2026-08-10 ruling asked for both pickers to become "a +/// rectangle (slightly oversized)... about 4:6 ratio of height to width" — and the way that is made +/// true is structural — one metrics value, one base control — so the test is about the structure +/// rather than about two numbers that happen to agree today. +@Suite("PickerRect ▸ the shared chrome") +struct PickerRectMetricsTests { @Test("Every figure scales with the body font, and reproduces the shipped numbers at 13pt") func metricsAtTheStandardBody() { - let metrics = ComboFieldMetrics.metrics(bodyPointSize: 13) - // Evening 2026-08-09 review: "reduce vertical size of both pickers by 50%" — height is half - // the 4:5 pass's 36, and width holds the 4:5 pass's own pixel figure instead of re-deriving - // from the now-shorter height (`ComboFieldMetrics`'s own doc comment). - #expect(metrics.height == 18) - #expect(metrics.width == 29) - #expect(metrics.triggerWidth == 16) + let metrics = PickerRectMetrics.metrics(bodyPointSize: 13) + // Owner's ruling, 2026-08-10: "50% taller" than the retired two-zone chrome's 18pt (→ 27pt), + // and "about 4:6 ratio" — width is height × 1.5 (`PickerRectMetrics`'s own doc comment). + #expect(metrics.height == 27) + #expect(metrics.width == 41) #expect(metrics.cornerRadius == 3) #expect(metrics.fieldRadius == 4) - #expect(metrics.triggerInset == 2) } @Test("A larger text size grows every figure, and none collapses to zero") func metricsScale() { - let small = ComboFieldMetrics.metrics(bodyPointSize: 11) - let large = ComboFieldMetrics.metrics(bodyPointSize: 24) + let small = PickerRectMetrics.metrics(bodyPointSize: 11) + let large = PickerRectMetrics.metrics(bodyPointSize: 24) #expect(large.height > small.height) #expect(large.width > small.width) - #expect(large.triggerWidth > small.triggerWidth) - for metrics in [ComboFieldMetrics.metrics(bodyPointSize: 8), small, large] { + for metrics in [PickerRectMetrics.metrics(bodyPointSize: 8), small, large] { #expect(metrics.height >= 1) #expect(metrics.width >= 1) - #expect(metrics.triggerWidth >= 1) #expect(metrics.cornerRadius >= 1) } } - /// **The owner's evening figures, held still.** "Reduce vertical size of both pickers by 50%" - /// (2026-08-09 evening) superseded the earlier "almost square, about 4:5 ratio" ruling outright — - /// `height` and `width` are now independent em figures rather than one derived from the other, so - /// this asserts both hold at every body size the superseded ratio test used, and that `faceWidth` - /// never collapses the way it would have if `width` had stayed a function of `height`. - @Test("Height is 1.4 em, width is 2.2 em, and the face zone stays positive at every body size") - func heightAndWidthAreIndependentEmFigures() { + /// **The owner's 2026-08-10 figures, pinned.** Height is 2.1 em; width is derived from height + /// (`height × 1.5`) rather than its own independent em multiple, which is what keeps the 4:6 + /// ratio exact — to rounding — at every body size instead of the two figures drifting apart. + @Test("Height is 2.1 em, width is 1.5 × height, at every body size") + func heightAndWidthHoldTheRatio() { for size in [8.0, 11.0, 13.0, 17.0, 24.0, 36.0] as [CGFloat] { - let metrics = ComboFieldMetrics.metrics(bodyPointSize: size) - #expect(metrics.height == max(1, (size * 1.4).rounded()), "height drifted from 1.4 em at \(size)pt") - #expect(metrics.width == max(1, (size * 2.2).rounded()), "width drifted from 2.2 em at \(size)pt") - #expect(metrics.faceWidth > 0, "the face zone collapsed at \(size)pt") + let metrics = PickerRectMetrics.metrics(bodyPointSize: size) + #expect(metrics.height == max(1, (size * 2.1).rounded()), "height drifted from 2.1 em at \(size)pt") + #expect(metrics.width == max(1, (metrics.height * 1.5).rounded()), "width drifted from 1.5 × height at \(size)pt") + // 4:6 as a ratio, within the slack one rounding step introduces at the smallest sizes. + #expect(abs(metrics.width / metrics.height - 1.5) < 0.06, "ratio drifted from 4:6 at \(size)pt") } } @@ -269,80 +264,57 @@ struct ComboFieldMetricsTests { @Test("The field's radius stays outside the face's") func radiiAreConcentric() { for size in [11.0, 13.0, 17.0, 24.0] as [CGFloat] { - let metrics = ComboFieldMetrics.metrics(bodyPointSize: size) + let metrics = PickerRectMetrics.metrics(bodyPointSize: size) #expect(metrics.fieldRadius >= metrics.cornerRadius) } } - /// **The new actual-rect rule.** `ComboFieldMetrics.glyphPointSize` is gone; `SymbolComboControl` - /// now sizes a glyph off whichever face rect it is actually handed at draw time (this is what lets - /// the sidebar's stretched-wide control still read as "no padding" rather than a small glyph in a - /// big field). Both the point-size rule and the overshoot-scaling rule are pure static functions - /// on `SymbolComboControl`, assertable with no control on screen and no draw. + /// **The actual-rect rule, unaffected by the trigger's removal.** `SymbolGlyphControl` sizes a + /// glyph off whichever face rect it is actually handed at draw time — now always the control's + /// own `bounds`, since there is no trigger strip left to subtract. Both the point-size rule and + /// the overshoot-scaling rule are pure static functions on `SymbolGlyphControl`, assertable with + /// no control on screen and no draw. @Test("A glyph's point size and drawn size follow the actual face rect, not the pure metrics") func glyphSizesOffTheActualFaceRect() { // A short, wide rect — the shape the card sidebar's row actually proposes now that the field // is wider than it is tall. The binding dimension is height, not width. - let wideRect = NSRect(x: 0, y: 0, width: 200, height: 18) - #expect(SymbolComboControl.glyphPointSize(forFace: wideRect) == 18) + let wideRect = NSRect(x: 0, y: 0, width: 200, height: 27) + #expect(SymbolGlyphControl.glyphPointSize(forFace: wideRect) == 27) // A tall, narrow rect, for symmetry — the binding dimension flips to width. let tallRect = NSRect(x: 0, y: 0, width: 12, height: 40) - #expect(SymbolComboControl.glyphPointSize(forFace: tallRect) == 12) + #expect(SymbolGlyphControl.glyphPointSize(forFace: tallRect) == 12) // A glyph configured within its rect never needs to grow. - let snugSize = NSSize(width: 18, height: 18) - #expect(SymbolComboControl.fittedSize(for: snugSize, in: wideRect) == snugSize) + let snugSize = NSSize(width: 27, height: 27) + #expect(SymbolGlyphControl.fittedSize(for: snugSize, in: wideRect) == snugSize) - // A horizontally elongated glyph (wider than the point size it was configured at) overshoots - // the rect on its long axis and must be scaled down proportionally, not clipped. - let elongated = NSSize(width: 36, height: 18) - let fitted = SymbolComboControl.fittedSize(for: elongated, in: wideRect) - #expect(fitted.width <= wideRect.width) - #expect(fitted.height <= wideRect.height) + // A glyph wider than the point size it was configured at genuinely overshoots a *narrow* + // rect on its long axis (the realistic case: `tallRect`'s 12pt binding dimension, a symbol a + // touch wider than tall at that size) and must be scaled down proportionally, not clipped. + let elongated = NSSize(width: 16, height: 12) + let fitted = SymbolGlyphControl.fittedSize(for: elongated, in: tallRect) + #expect(fitted.width < elongated.width, "the wide axis must actually shrink") + #expect(fitted.width <= tallRect.width) + #expect(fitted.height <= tallRect.height) #expect(abs(fitted.width / fitted.height - elongated.width / elongated.height) < 0.001, "the scale-down must preserve the glyph's own aspect ratio") } - /// **The two controls are the same control.** Both are `ComboFieldControl`s and both take their + /// **The two controls are the same control.** Both are `PickerRectControl`s and both take their /// geometry from the same value, so a change to one lands on the other — which is the whole of - /// the parity claim, and cheaper to assert than any pair of measurements. + /// the parity claim, and cheaper to assert than any pair of measurements. There are no zones left + /// to compare (`PickerRect.swift`'s own retirement of the trigger strip); the whole bounds is the + /// one hit zone on both, so intrinsic size is the whole of what "same chrome" means now. @MainActor - @Test("Both combos are the same chrome, at the same size") - func bothCombosShareTheChrome() { - let colour = ColorComboControl(frame: .zero) - let symbol = SymbolComboControl(frame: .zero) - for control in [colour as ComboFieldControl, symbol] { + @Test("Both rectangles are the same chrome, at the same size") + func bothRectanglesShareTheChrome() { + let colour = ColorSwatchControl(frame: .zero) + let symbol = SymbolGlyphControl(frame: .zero) + for control in [colour as PickerRectControl, symbol] { control.metrics = .metrics(bodyPointSize: 13) } #expect(colour.intrinsicContentSize.height == symbol.intrinsicContentSize.height) #expect(colour.intrinsicContentSize.width == symbol.intrinsicContentSize.width) - colour.setFrameSize(NSSize(width: 120, height: colour.intrinsicContentSize.height)) - symbol.setFrameSize(NSSize(width: 120, height: symbol.intrinsicContentSize.height)) - #expect(colour.triggerRect == symbol.triggerRect, "the trigger zones must line up") - #expect(colour.faceZone == symbol.faceZone, "the face zones must line up") - } - - /// The two zones tile the control exactly — no dead strip between them, no overlap that would - /// make one door swallow the other's clicks. - @MainActor - @Test("The face and the trigger tile the control with no gap and no overlap") - func zonesTileTheControl() { - let control = SymbolComboControl(frame: NSRect(x: 0, y: 0, width: 140, height: 18)) - control.metrics = .metrics(bodyPointSize: 13) - #expect(control.faceZone.maxX == control.triggerRect.minX) - #expect(control.faceZone.minX == control.bounds.minX) - #expect(control.triggerRect.maxX == control.bounds.maxX) - #expect(control.faceZone.width + control.triggerRect.width == control.bounds.width) - } - - /// A control too narrow for its own trigger must not hand the face a negative width — a sidebar - /// squeezed to nothing is a layout bug, not a crash. - @MainActor - @Test("A control narrower than its trigger degrades to an empty face") - func degenerateWidthIsSafe() { - let control = SymbolComboControl(frame: NSRect(x: 0, y: 0, width: 4, height: 18)) - control.metrics = .metrics(bodyPointSize: 13) - #expect(control.faceZone.width >= 0) } }