The combo halves its height — the symbol picker gives up its padding, not its glyph

The owner's evening review superseded the same day's 4:5-ratio pass: 'reduce
vertical size of both pickers by 50%, on symbol picker by reducing padding.'

ComboFieldMetrics.height goes back to 1.4 em (18pt at the standard body,
exactly half the 4:5 pass's 36) — both combos get this for free, since it's
shared. width stops being height's own restatement (widthToHeightRatio is
gone); it holds the 4:5 pass's pixel figure as its own em number instead,
because re-deriving it at the new height would starve the trigger strip and
empty the face zone entirely.

SymbolComboControl.drawFace now sizes its glyph off the actual face rect it
is handed at draw time rather than the pure ComboFieldMetrics.glyphPointSize
(removed, along with its doc — it only ever described the control's intrinsic
portrait width, not the stretched-wide shape the card sidebar's row actually
proposes). No inset — SF Symbols carry their own margins — with the sizing
and overshoot-scaling rules split into two pure static functions so the fit
is assertable without a live draw.

ComboFieldMetricsTests updated for the new figures and the new actual-rect
glyph rule; the superseded 4:5-ratio-at-six-sizes test is replaced by one
pinning height/width independently and asserting faceWidth stays positive at
every size the old test used.

Claude-Session: https://claude.ai/code/session_014PtZdPwqZuqEDLc6wZMtEy
This commit is contained in:
2026-08-09 17:23:14 -04:00
parent 5da86dcf05
commit cb0af4b95a
3 changed files with 126 additions and 64 deletions
+52 -33
View File
@@ -33,7 +33,8 @@ import AppKit
/// the numbers `ColorComboControl` shipped with at the standard 13pt body with one deliberate /// the numbers `ColorComboControl` shipped with at the standard 13pt body with one deliberate
/// exception, `height`. /// 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 /// 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 /// 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 /// - **`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 /// 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 /// 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 /// undoes.
/// the two cannot drift into some other proportion at a text size nobody checked.
/// - **The glyph centres in both axes.** It always claimed to (`SymbolComboControl.drawFace`'s own /// - **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 /// 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 /// leading edge by the swatch's own horizontal padding dead code once that padding left, and the
/// wrong rect even before it did. /// 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 { 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. /// The control's height.
/// ///
/// **2.75 em is 36pt at the standard body, where the first pass shipped 1.4 em / 18pt.** 18 was /// **1.4 em is 18pt at the standard body exactly half of the 4:5 pass's 36**, and incidentally
/// tuned for a *wide* bar; reaching the 4:5 ratio without starving the trigger of the room a /// the very figure the original pass before that one shipped. The owner's evening review asked for
/// legible chevron needs (`triggerWidth` below, unchanged since the first pass) takes a taller /// "reduce vertical size of both pickers by 50%" outright, not a re-tuning against the trigger or
/// field than that, which is the same request the owner made in words. /// the glyph; halving the shipped 36 is that request applied literally.
var height: CGFloat var height: CGFloat
/// The whole control's width `height` scaled by `widthToHeightRatio` and rounded. Every caller /// The whole control's width its own em figure now, not a scaling of `height`. **2.2 em is 29pt
/// now gets this same narrow field unless it hands SwiftUI an explicit finite proposal of its /// 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. /// own, which `sizeThatFits` on each representable still honours exactly as before.
var width: CGFloat var width: CGFloat
/// The trigger strip at the trailing edge, full height. Unchanged from the first pass: this /// 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 var triggerWidth: CGFloat
/// The trigger square's inset from the strip's height kept off the *ring* width rather than /// 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 /// 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. /// 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 once the strip /// drawTrigger`'s `min(width, height)`, which is what keeps this inset meaningful regardless of
/// stopped being roughly as wide as the control is tall). /// which dimension ends up binding at a given body size).
var triggerInset: CGFloat var triggerInset: CGFloat
/// The face's and the trigger square's radius. /// The face's and the trigger square's radius.
var cornerRadius: CGFloat var cornerRadius: CGFloat
@@ -87,10 +101,9 @@ struct ComboFieldMetrics: Equatable {
static func metrics(bodyPointSize: CGFloat) -> ComboFieldMetrics { static func metrics(bodyPointSize: CGFloat) -> ComboFieldMetrics {
func em(_ multiple: CGFloat) -> CGFloat { max(1, (bodyPointSize * multiple).rounded()) } func em(_ multiple: CGFloat) -> CGFloat { max(1, (bodyPointSize * multiple).rounded()) }
let height = em(2.75)
return ComboFieldMetrics( return ComboFieldMetrics(
height: height, height: em(1.4),
width: max(1, (height * widthToHeightRatio).rounded()), width: em(2.2),
triggerWidth: em(1.25), triggerWidth: em(1.25),
triggerInset: em(0.15), triggerInset: em(0.15),
cornerRadius: em(0.23), 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 /// 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 /// 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) } 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 // MARK: - The control
/// The shared field. Subclasses draw the face and answer the two zones; nothing else here is theirs /// 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 /// 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 /// the 2026-08-09 iteration that gave the field a fixed shape instead of whatever a caller's frame
/// whatever a caller's frame happened to propose; a representable's `sizeThatFits` still honours /// happened to propose; a representable's `sizeThatFits` still honours an explicit finite proposal
/// an explicit finite proposal over this default, exactly as before. /// over this default, exactly as before.
override var intrinsicContentSize: NSSize { override var intrinsicContentSize: NSSize {
NSSize(width: metrics.width, height: metrics.height) 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 /// **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 /// 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 /// 2026-08-09 iterations moved `height` and `width` independently of one another (this file's
/// from height alone would then ask for a square wider than the strip itself. /// 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() { private func drawTrigger() {
let side = min(triggerRect.width, triggerRect.height) - 2 * metrics.triggerInset let side = min(triggerRect.width, triggerRect.height) - 2 * metrics.triggerInset
let square = NSRect( let square = NSRect(
+32 -2
View File
@@ -432,15 +432,25 @@ final class SymbolComboControl: ComboFieldControl {
/// draw only ever centred vertically and offset from the leading edge by the swatch's own /// 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 /// 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"). /// 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) { override func drawFace(in rect: NSRect) {
guard rect.width > 0, rect.height > 0 else { return } guard rect.width > 0, rect.height > 0 else { return }
let name = ItemSymbol.exists(glyphName) ? glyphName : "questionmark.square.dashed" 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])) .applying(.init(paletteColors: [glyphTint ?? .labelColor]))
guard let image = NSImage(systemSymbolName: name, accessibilityDescription: nil)? guard let image = NSImage(systemSymbolName: name, accessibilityDescription: nil)?
.withSymbolConfiguration(config) .withSymbolConfiguration(config)
else { return } else { return }
let size = image.size let size = Self.fittedSize(for: image.size, in: rect)
image.draw(in: NSRect( image.draw(in: NSRect(
x: rect.midX - size.width / 2, x: rect.midX - size.width / 2,
y: rect.midY - size.height / 2, y: rect.midY - size.height / 2,
@@ -448,6 +458,26 @@ final class SymbolComboControl: ComboFieldControl {
height: size.height 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 // MARK: - The popover's content
+42 -29
View File
@@ -223,9 +223,10 @@ struct ComboFieldMetricsTests {
@Test("Every figure scales with the body font, and reproduces the shipped numbers at 13pt") @Test("Every figure scales with the body font, and reproduces the shipped numbers at 13pt")
func metricsAtTheStandardBody() { func metricsAtTheStandardBody() {
let metrics = ComboFieldMetrics.metrics(bodyPointSize: 13) let metrics = ComboFieldMetrics.metrics(bodyPointSize: 13)
// 2026-08-09 iteration: taller and narrower than the first pass's 18×(whatever the caller // Evening 2026-08-09 review: "reduce vertical size of both pickers by 50%" height is half
// proposed) bar `width` is now `ComboFieldMetrics`' own figure, not the caller's. // the 4:5 pass's 36, and width holds the 4:5 pass's own pixel figure instead of re-deriving
#expect(metrics.height == 36) // from the now-shorter height (`ComboFieldMetrics`'s own doc comment).
#expect(metrics.height == 18)
#expect(metrics.width == 29) #expect(metrics.width == 29)
#expect(metrics.triggerWidth == 16) #expect(metrics.triggerWidth == 16)
#expect(metrics.cornerRadius == 3) #expect(metrics.cornerRadius == 3)
@@ -240,27 +241,26 @@ struct ComboFieldMetricsTests {
#expect(large.height > small.height) #expect(large.height > small.height)
#expect(large.width > small.width) #expect(large.width > small.width)
#expect(large.triggerWidth > small.triggerWidth) #expect(large.triggerWidth > small.triggerWidth)
#expect(large.glyphPointSize > small.glyphPointSize)
for metrics in [ComboFieldMetrics.metrics(bodyPointSize: 8), small, large] { for metrics in [ComboFieldMetrics.metrics(bodyPointSize: 8), small, large] {
#expect(metrics.height >= 1) #expect(metrics.height >= 1)
#expect(metrics.width >= 1) #expect(metrics.width >= 1)
#expect(metrics.triggerWidth >= 1) #expect(metrics.triggerWidth >= 1)
#expect(metrics.cornerRadius >= 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 /// **The owner's evening figures, held still.** "Reduce vertical size of both pickers by 50%"
/// a one-off measurement `width` is derived from `height` by `widthToHeightRatio`, so this /// (2026-08-09 evening) superseded the earlier "almost square, about 4:5 ratio" ruling outright
/// holds at every body size rather than only the one somebody happened to check. /// `height` and `width` are now independent em figures rather than one derived from the other, so
@Test("The field is taller than wide, at about a 4:5 ratio, from height alone") /// this asserts both hold at every body size the superseded ratio test used, and that `faceWidth`
func fieldIsAlmostSquare() { /// 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] { for size in [8.0, 11.0, 13.0, 17.0, 24.0, 36.0] as [CGFloat] {
let metrics = ComboFieldMetrics.metrics(bodyPointSize: size) let metrics = ComboFieldMetrics.metrics(bodyPointSize: size)
#expect(metrics.width < metrics.height, "the field should read taller than wide at \(size)pt") #expect(metrics.height == max(1, (size * 1.4).rounded()), "height drifted from 1.4 em at \(size)pt")
let ratio = metrics.width / metrics.height #expect(metrics.width == max(1, (size * 2.2).rounded()), "width drifted from 2.2 em at \(size)pt")
#expect(abs(ratio - ComboFieldMetrics.widthToHeightRatio) < 0.05, #expect(metrics.faceWidth > 0, "the face zone collapsed at \(size)pt")
"ratio drifted to \(ratio) at \(size)pt, want ~4:5 (0.8)")
} }
} }
@@ -274,21 +274,34 @@ struct ComboFieldMetricsTests {
} }
} }
/// A glyph must actually fit, and since the 2026-08-09 iteration removed the face's padding /// **The new actual-rect rule.** `ComboFieldMetrics.glyphPointSize` is gone; `SymbolComboControl`
/// fit *exactly*: `glyphPointSize` is whichever of the face's own two dimensions is smaller, with /// now sizes a glyph off whichever face rect it is actually handed at draw time (this is what lets
/// nothing subtracted for a padding that no longer exists. A negative or vanishing figure is the /// the sidebar's stretched-wide control still read as "no padding" rather than a small glyph in a
/// bug the first pass's 14pt height had. /// big field). Both the point-size rule and the overshoot-scaling rule are pure static functions
@Test("A glyph fills its face zone's limiting dimension, with no padding taken out of it") /// on `SymbolComboControl`, assertable with no control on screen and no draw.
func glyphFillsTheField() { @Test("A glyph's point size and drawn size follow the actual face rect, not the pure metrics")
for size in [11.0, 13.0, 17.0, 24.0] as [CGFloat] { func glyphSizesOffTheActualFaceRect() {
let metrics = ComboFieldMetrics.metrics(bodyPointSize: size) // A short, wide rect the shape the card sidebar's row actually proposes now that the field
#expect(metrics.glyphPointSize == min(metrics.faceWidth, metrics.height)) // is wider than it is tall. The binding dimension is height, not width.
// The field is taller than wide by construction, so the face reads the same way and let wideRect = NSRect(x: 0, y: 0, width: 200, height: 18)
// `faceWidth` is the dimension actually doing the limiting. #expect(SymbolComboControl.glyphPointSize(forFace: wideRect) == 18)
#expect(metrics.faceWidth < metrics.height, "expected the face to be the binding dimension at \(size)pt")
#expect(metrics.glyphPointSize == metrics.faceWidth) // A tall, narrow rect, for symmetry the binding dimension flips to width.
#expect(metrics.glyphPointSize >= 1) 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 /// **The two controls are the same control.** Both are `ComboFieldControl`s and both take their