diff --git a/Kanban/UI/ComboField.swift b/Kanban/UI/ComboField.swift index 7b91bf3..de58af5 100644 --- a/Kanban/UI/ComboField.swift +++ b/Kanban/UI/ComboField.swift @@ -33,7 +33,8 @@ import AppKit /// the numbers `ColorComboControl` shipped with at the standard 13pt body — with one deliberate /// exception, `height`. /// -/// ### The 2026-08-09 iteration: no padding, a taller narrower field, a centred glyph +/// ### 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 @@ -45,28 +46,41 @@ import AppKit /// - **`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. `width` is `height` scaled by `widthToHeightRatio`, never its own independent figure, so -/// the two cannot drift into some other proportion at a text size nobody checked. +/// 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 { - /// **4:5, width:height** — "almost square," the owner's own phrase. Applied to `height` rather - /// than carried as its own figure, so `width` is a restatement of `height` and not a second - /// number that could quietly stop agreeing with it. - static let widthToHeightRatio: CGFloat = 0.8 - /// The control's height. /// - /// **2.75 em is 36pt at the standard body, where the first pass shipped 1.4 em / 18pt.** 18 was - /// tuned for a *wide* bar; reaching the 4:5 ratio without starving the trigger of the room a - /// legible chevron needs (`triggerWidth` below, unchanged since the first pass) takes a taller - /// field than that, which is the same request the owner made in words. + /// **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 — `height` scaled by `widthToHeightRatio` and rounded. Every caller - /// now gets this same narrow field unless it hands SwiftUI an explicit finite proposal of its + /// 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 @@ -75,9 +89,9 @@ struct ComboFieldMetrics: Equatable { 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 a strip that is now much taller than it is wide (`ComboFieldControl. - /// drawTrigger`'s `min(width, height)`, which is what keeps this inset meaningful once the strip - /// stopped being roughly as wide as the control is tall). + /// 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 @@ -87,10 +101,9 @@ struct ComboFieldMetrics: Equatable { static func metrics(bodyPointSize: CGFloat) -> ComboFieldMetrics { func em(_ multiple: CGFloat) -> CGFloat { max(1, (bodyPointSize * multiple).rounded()) } - let height = em(2.75) return ComboFieldMetrics( - height: height, - width: max(1, (height * widthToHeightRatio).rounded()), + height: em(1.4), + width: em(2.2), triggerWidth: em(1.25), triggerInset: em(0.15), cornerRadius: em(0.23), @@ -106,17 +119,22 @@ struct ComboFieldMetrics: Equatable { /// 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 and to `glyphPointSize` below with no control on screen. + /// 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) } - - /// The glyph's point size inside a face — no padding subtracted now, so this is simply whichever - /// of the face's two dimensions is smaller. That is `faceWidth` at every body size this control - /// ships at (the field reads taller than wide by construction); `height` only becomes the binding - /// one if a future caller widens `triggerWidth` past `width`'s own share of it, which the `min` - /// guards against turning into an oversized, clipped glyph. - var glyphPointSize: CGFloat { max(1, min(faceWidth, height)) } } +// 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 @@ -160,9 +178,9 @@ class ComboFieldControl: NSControl { } /// Both dimensions are the metrics' own now. Width stopped being `NSView.noIntrinsicMetric` in - /// the 2026-08-09 iteration that gave the field a fixed, taller-than-wide 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. + /// 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) } @@ -236,8 +254,9 @@ class ComboFieldControl: NSControl { /// /// **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 taller-narrower field can make a strip much taller than it is wide, and a side taken - /// from height alone would then ask for a square wider than the strip itself. + /// 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( diff --git a/Kanban/UI/SymbolPicker.swift b/Kanban/UI/SymbolPicker.swift index dcfa943..684f754 100644 --- a/Kanban/UI/SymbolPicker.swift +++ b/Kanban/UI/SymbolPicker.swift @@ -432,15 +432,25 @@ final class SymbolComboControl: ComboFieldControl { /// draw only ever centred vertically and offset from the leading edge by the swatch's own /// 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 + /// 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 + /// intrinsic figure stayed small, a glyph floating in visible dead space. That space *was* the + /// "padding" the owner's "reduce vertical size … on symbol picker by reducing padding" was naming, + /// so the fix reads the point size off the rect the face actually receives, via `glyphPointSize( + /// forFace:)` and `fittedSize(for:in:)` below — pure functions, not inlined here, so the sizing + /// rule is assertable without an actual draw. override func drawFace(in rect: NSRect) { guard rect.width > 0, rect.height > 0 else { return } let name = ItemSymbol.exists(glyphName) ? glyphName : "questionmark.square.dashed" - let config = NSImage.SymbolConfiguration(pointSize: metrics.glyphPointSize, weight: .regular) + let config = NSImage.SymbolConfiguration(pointSize: Self.glyphPointSize(forFace: rect), weight: .regular) .applying(.init(paletteColors: [glyphTint ?? .labelColor])) guard let image = NSImage(systemSymbolName: name, accessibilityDescription: nil)? .withSymbolConfiguration(config) else { return } - let size = image.size + let size = Self.fittedSize(for: image.size, in: rect) image.draw(in: NSRect( x: rect.midX - size.width / 2, y: rect.midY - size.height / 2, @@ -448,6 +458,26 @@ final class SymbolComboControl: ComboFieldControl { height: size.height )) } + + /// The glyph's target point size for a face rect — no inset subtracted, so this is simply whichever + /// of the rect's own two dimensions is smaller. SF Symbols already carry their own internal + /// margins, and the owner's evening review asked for the *dead space around* the glyph gone, not a + /// second margin layered on top of the system's own. + static func glyphPointSize(forFace rect: NSRect) -> CGFloat { + min(rect.width, rect.height) + } + + /// `imageSize` unchanged, unless it overshoots `rect` on either axis — a symbol configured at + /// `glyphPointSize(forFace:)` can still render wider (or, rarely, taller) than that on its long + /// axis, since SF Symbols are not all square glyphs. Scaled down proportionally so the drawn glyph + /// stays fully inside its face rather than clipping at the edges, and never scaled up: a glyph + /// smaller than its face on both axes is left exactly as configured. + static func fittedSize(for imageSize: NSSize, in rect: NSRect) -> NSSize { + guard rect.width > 0, rect.height > 0 else { return imageSize } + let overshoot = max(imageSize.width / rect.width, imageSize.height / rect.height) + guard overshoot > 1 else { return imageSize } + return NSSize(width: imageSize.width / overshoot, height: imageSize.height / overshoot) + } } // MARK: - The popover's content diff --git a/KanbanTests/SymbolCatalogTests.swift b/KanbanTests/SymbolCatalogTests.swift index 18d70fb..75c1869 100644 --- a/KanbanTests/SymbolCatalogTests.swift +++ b/KanbanTests/SymbolCatalogTests.swift @@ -223,9 +223,10 @@ struct ComboFieldMetricsTests { @Test("Every figure scales with the body font, and reproduces the shipped numbers at 13pt") func metricsAtTheStandardBody() { let metrics = ComboFieldMetrics.metrics(bodyPointSize: 13) - // 2026-08-09 iteration: taller and narrower than the first pass's 18×(whatever the caller - // proposed) bar — `width` is now `ComboFieldMetrics`' own figure, not the caller's. - #expect(metrics.height == 36) + // 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) #expect(metrics.cornerRadius == 3) @@ -240,27 +241,26 @@ struct ComboFieldMetricsTests { #expect(large.height > small.height) #expect(large.width > small.width) #expect(large.triggerWidth > small.triggerWidth) - #expect(large.glyphPointSize > small.glyphPointSize) for metrics in [ComboFieldMetrics.metrics(bodyPointSize: 8), small, large] { #expect(metrics.height >= 1) #expect(metrics.width >= 1) #expect(metrics.triggerWidth >= 1) #expect(metrics.cornerRadius >= 1) - #expect(metrics.glyphPointSize >= 1) } } - /// **The owner's own figure, held still.** "Almost square, about 4:5 ratio" (2026-08-09) is not - /// a one-off measurement — `width` is derived from `height` by `widthToHeightRatio`, so this - /// holds at every body size rather than only the one somebody happened to check. - @Test("The field is taller than wide, at about a 4:5 ratio, from height alone") - func fieldIsAlmostSquare() { + /// **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() { 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.width < metrics.height, "the field should read taller than wide at \(size)pt") - let ratio = metrics.width / metrics.height - #expect(abs(ratio - ComboFieldMetrics.widthToHeightRatio) < 0.05, - "ratio drifted to \(ratio) at \(size)pt, want ~4:5 (0.8)") + #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") } } @@ -274,21 +274,34 @@ struct ComboFieldMetricsTests { } } - /// A glyph must actually fit, and — since the 2026-08-09 iteration removed the face's padding — - /// fit *exactly*: `glyphPointSize` is whichever of the face's own two dimensions is smaller, with - /// nothing subtracted for a padding that no longer exists. A negative or vanishing figure is the - /// bug the first pass's 14pt height had. - @Test("A glyph fills its face zone's limiting dimension, with no padding taken out of it") - func glyphFillsTheField() { - for size in [11.0, 13.0, 17.0, 24.0] as [CGFloat] { - let metrics = ComboFieldMetrics.metrics(bodyPointSize: size) - #expect(metrics.glyphPointSize == min(metrics.faceWidth, metrics.height)) - // The field is taller than wide by construction, so the face reads the same way and - // `faceWidth` is the dimension actually doing the limiting. - #expect(metrics.faceWidth < metrics.height, "expected the face to be the binding dimension at \(size)pt") - #expect(metrics.glyphPointSize == metrics.faceWidth) - #expect(metrics.glyphPointSize >= 1) - } + /// **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. + @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) + + // 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) + + // 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) + + // 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) + #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