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
311 lines
15 KiB
Swift
311 lines
15 KiB
Swift
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 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.
|
|
///
|
|
/// **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 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
|
|
/// 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 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()) }
|
|
let height = em(2.75)
|
|
return ComboFieldMetrics(
|
|
height: height,
|
|
width: max(1, (height * widthToHeightRatio).rounded()),
|
|
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 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
|
|
|
|
/// 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, 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: 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 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 = 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
|
|
}
|
|
}
|