rzen/lanework
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

The combo drops its padding and turns portrait — a 4:5 field puts the symbol dead center

Browse Source 
Owner's first review of the combo rework (2026-08-09): remove the face padding, make the
field taller and narrower at about a 4:5 width:height ratio, and center the symbol glyph in
its face. All three land in ComboFieldMetrics, so ColorComboControl and SymbolComboControl
stay the identical shape they were built to share.

- ComboFieldMetrics grows a width figure (height * widthToHeightRatio, 0.8), replacing
  NSView.noIntrinsicMetric — every combo now carries its own taller, narrower intrinsic size
  instead of stretching to whatever a caller's frame proposed.
- facePaddingH/facePaddingV/glyphPadding are gone; a face fills its zone edge to edge.
  glyphPointSize is now whichever of the face's own width/height is smaller, with nothing
  subtracted for padding that no longer exists.
- SymbolComboControl.drawFace centers the glyph on both axes — it only ever centered
  vertically before, despite its own doc comment claiming otherwise.
- ComboFieldControl.drawTrigger bounds its chevron square by the smaller of the trigger
  strip's own width/height, not height alone, since the strip is no longer close to square
  once the field is much taller than it is wide.
- CardSidebarSections drops the sidebar's old '* 0.55' fixed-width frame on both combo rows;
  each control now sizes itself, and both anchors (card sidebar, board popover) compose the
  narrower field with no other changes needed.
- ComboFieldMetricsTests updated for the new figures, plus a ratio-holds-at-every-size test
  and a rewritten glyph-fit test matching the no-padding rule.

Verification: xcodebuild build succeeded; xcodebuild test -only-testing:KanbanTests — 3220
tests in 559 suites, 3 failures, all PointerLatencyTests (documented locked-screen
environmental mode, confirmed unrelated by isolated rerun). Pixel verification unexercised —
same locked-screen constraint the first pass hit.

Claude-Session: https://claude.ai/code/session_014PtZdPwqZuqEDLc6wZMtEy
This commit is contained in:
rzen
2026-08-09 12:06:01 -04:00
parent fda19881de
commit 50e26efdb9
6 changed files with 136 additions and 88 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Kanban/UI/Card/CardSidebarSections.swift
+10 -8
View File
@@ -108,8 +108,12 @@ struct CardStyleSection: View {
/// 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.
/// The combo takes just over half the row rather than filling it — sized off the same metric
/// the sidebar's own width comes from, so the pair holds its proportions at every text size.
///
/// **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")
@@ -123,7 +127,6 @@ struct CardStyleSection: View {
onChange: { commitBackground($0) },
onPanelChange: { debounceBackground($0) }
)
.frame(width: CardWindowMetrics.sidebarContentWidth(bodyPointSize: pointSize) * 0.55)
}
.frame(maxWidth: .infinity, alignment: .leading)
}
@@ -175,14 +178,14 @@ struct CardStyleSection: View {
// 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 width**.
/// inspector-row shape, restated: caption leading, the combo trailing, **at the same size**.
///
/// That width used to be the difference between the two rows. The picker was a 20pt bordered
/// 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 row hands it the identical `* 0.55` frame and the pair reads as
/// one inspector.
/// (`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) {
Text("Symbol")
@@ -197,7 +200,6 @@ struct CardStyleSection: View {
currentColor: currentIconColor,
onSelectColor: { applyIconColor($0) }
)
.frame(width: CardWindowMetrics.sidebarContentWidth(bodyPointSize: pointSize) * 0.55)
// 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
Kanban/UI/Card/CardWindowMetrics.swift
+6 -6
View File
@@ -68,12 +68,12 @@ enum CardWindowMetrics {
/// What a sidebar *section* actually gets to lay out in: the column minus its two gutters.
///
/// Named because a section can need a number rather than a proposal — originally 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 compact
/// `SymbolPicker` (2026-08-08) replaced that embed, but the Background color combo it sits beside
/// still sizes itself off this figure (`CardStyleSection.backgroundComboRow`). Everything else in
/// the sidebar simply fills what it is proposed and never asks.
/// 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
/// they are proposed, like everything else in the sidebar.
static func sidebarContentWidth(bodyPointSize: CGFloat) -> CGFloat {
sidebarWidth(bodyPointSize: bodyPointSize) - 2 * gutter(bodyPointSize: bodyPointSize)
}
Kanban/UI/ColorCombo.swift
+11 -16
View File
@@ -210,12 +210,6 @@ struct ColorComboView: NSViewRepresentable {
/// debounce and never feeds style recents.
var onPanelChange: @MainActor (String?) -> Void
/// The width `sizeThatFits` hands back when SwiftUI has no concrete proposal to fill — an
/// unconstrained measuring pass, not the normal case. The normal case is a finite proposal (this
/// view sits under `.frame(maxWidth: .infinity)` in the sidebar row, `CardActionsSection`'s
/// Delete button's own trick), which this default never has to stand in for.
private static let defaultFaceWidth: CGFloat = 120
func makeNSView(context: Context) -> ColorComboControl {
let control = ColorComboControl(frame: .zero)
// The two zones' jobs, wired once (`ComboFieldControl`'s grammar): the face opens the same
@@ -240,18 +234,17 @@ struct ColorComboView: NSViewRepresentable {
context.coordinator.rebuild(control, value: value)
}
/// Obeys whatever width SwiftUI proposes, like any other control — never the widest menu item,
/// which is what the old caller-supplied `width` input existed to work around. Height is the
/// control's own fitting height (`ColorComboControl.intrinsicContentSize`, about half the old
/// regular `NSPopUpButton`'s — the whole point of this rework); width is the proposal's when it
/// is an actual number, else `defaultFaceWidth`, since a `nil`/infinite proposal happens on an
/// unconstrained measuring pass, not a sidebar row.
/// 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 = Self.defaultFaceWidth
width = nsView.intrinsicContentSize.width
}
return CGSize(width: width, height: nsView.intrinsicContentSize.height)
}
@@ -430,10 +423,12 @@ final class ColorComboControl: ComboFieldControl {
/// 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) {
let inset = rect.insetBy(dx: metrics.facePaddingH, dy: metrics.facePaddingV)
guard inset.width > 0, inset.height > 0 else { return }
let path = NSBezierPath(roundedRect: inset, xRadius: metrics.cornerRadius, yRadius: metrics.cornerRadius)
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) {
Kanban/UI/ComboField.swift
+67 -36
View File
@@ -32,36 +32,53 @@ import AppKit
/// 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`.
// MARK: - Metrics
/// The combo's geometry, as a pure value — no `NSView`, so the parity claim ("the two controls are
/// the same size") is a thing a test can assert rather than a thing a screenshot suggests.
///
/// ### The 2026-08-09 iteration: no padding, a taller narrower field, a centred glyph
///
/// 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. `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.
/// - **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.
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.
///
/// **1.4 em is 18pt at the standard body, where this shape shipped at a flat 14** — the one
/// figure here that is not a restatement of an old constant. 14 was tuned for a colour bar and
/// is a fine height for one; a *glyph* in a 14pt field, once the face's own padding is taken
/// out, is six points tall and unreadable. 18 still reads as the compact control the original
/// rework was after (a regular `NSPopUpButton` is about 25) and leaves the glyph room to be a
/// glyph.
/// **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.
var height: CGFloat
/// The trigger strip at the trailing edge, full height.
/// 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
/// 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
/// the face's larger padding, so the indicator stays a legible square instead of shrinking with
/// every padding tweak the face takes.
/// 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).
var triggerInset: CGFloat
/// The face's inset inside its zone, asymmetric and user-tuned: a wider berth at the sides than
/// above and below, so a swatch reads as a bar sitting in the field rather than filling it wall
/// to wall. The space comes out of the face — the control's overall size is untouched.
var facePaddingH: CGFloat
var facePaddingV: CGFloat
/// A glyph face's own inset, much tighter than a swatch's: a symbol *is* the face, where a
/// swatch is a sample sitting in one.
var glyphPadding: 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
@@ -70,13 +87,12 @@ 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: em(1.4),
height: height,
width: max(1, (height * widthToHeightRatio).rounded()),
triggerWidth: em(1.25),
triggerInset: em(0.15),
facePaddingH: em(0.54),
facePaddingV: em(0.38),
glyphPadding: em(0.15),
cornerRadius: em(0.23),
fieldRadius: em(0.31)
)
@@ -88,9 +104,17 @@ struct ComboFieldMetrics: Equatable {
@MainActor
static var current: ComboFieldMetrics { metrics(bodyPointSize: CardWindowMetrics.bodyPointSize) }
/// The glyph's point size inside a face — the face zone's height less its padding, which is
/// what makes a symbol fill the control instead of floating in it.
var glyphPointSize: CGFloat { max(1, height - 2 * glyphPadding) }
/// 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.
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: - The control
@@ -135,10 +159,12 @@ class ComboFieldControl: NSControl {
}
}
/// Width is `noIntrinsicMetric`: the control obeys whatever SwiftUI proposes, so a sidebar row
/// sizes it and an unconstrained pass falls back to the representable's own default.
/// 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.
override var intrinsicContentSize: NSSize {
NSSize(width: NSView.noIntrinsicMetric, height: metrics.height)
NSSize(width: metrics.width, height: metrics.height)
}
// MARK: Zones
@@ -204,11 +230,16 @@ class ComboFieldControl: NSControl {
path.stroke()
}
/// The trigger: a small vertically-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 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 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.
private func drawTrigger() {
let side = triggerRect.height - 2 * metrics.triggerInset
let side = min(triggerRect.width, triggerRect.height) - 2 * metrics.triggerInset
let square = NSRect(
x: triggerRect.midX - side / 2,
y: triggerRect.midY - side / 2,
Kanban/UI/SymbolPicker.swift
+11 -14
View File
@@ -370,15 +370,6 @@ private struct SymbolComboView: NSViewRepresentable {
return fallback
}
/// The width an unconstrained measuring pass falls back to — a face wide enough for the glyph to
/// sit in with room to read as a *field* rather than a button, plus the trigger. The normal case
/// is a finite proposal (a sidebar row's `.frame(width:)`), which this never has to stand in for.
@MainActor
private var defaultFaceWidth: CGFloat {
let metrics = ComboFieldMetrics.current
return (metrics.height * 2 + metrics.triggerWidth).rounded()
}
func makeNSView(context: Context) -> SymbolComboControl {
SymbolComboControl(frame: .zero)
}
@@ -396,15 +387,15 @@ private struct SymbolComboView: NSViewRepresentable {
control.onTriggerClick = onTrigger
}
/// Obeys whatever width SwiftUI proposes, like any other control — `ColorComboView.sizeThatFits`'
/// rule, restated so the two combos answer a proposal identically and a caller that frames them
/// alike gets two controls the same size.
/// 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? {
let width: CGFloat
if let proposed = proposal.width, proposed.isFinite {
width = proposed
} else {
width = defaultFaceWidth
width = nsView.intrinsicContentSize.width
}
return CGSize(width: width, height: nsView.intrinsicContentSize.height)
}
@@ -436,7 +427,13 @@ final class SymbolComboControl: ComboFieldControl {
/// The glyph, centred and tinted. A name this system cannot draw falls back to the dashed
/// question mark the wells use — the same "show that there is nothing here" the grids draw.
///
/// **Centred on both axes since 2026-08-09** — the doc comment above always said "centred" but the
/// 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").
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)
.applying(.init(paletteColors: [glyphTint ?? .labelColor]))
@@ -445,7 +442,7 @@ final class SymbolComboControl: ComboFieldControl {
else { return }
let size = image.size
image.draw(in: NSRect(
x: rect.minX + metrics.facePaddingH,
x: rect.midX - size.width / 2,
y: rect.midY - size.height / 2,
width: size.width,
height: size.height