Skip to content

Commit 7e5b1d5

Browse files
Make a scrub reversible, cancellable with Escape, and keep the pill on screen
Reversibility: each scrub frame was recomposing from `values`, which now tracks the clamped colour, so every frame fed the previous frame's clamping back in. That ratcheted the components you weren't dragging a little further each frame, and dragging back where you came from no longer returned the colour you started with. Recompose from a pristine snapshot of the session's starting values instead — the displayed numbers still track the real colour, but they no longer feed the source. Escape: the tracking loop only watched for mouse events, so Escape did nothing mid-drag (the field isn't focused during a scrub, so the normal key path never saw it). Watch for it in the loop and route a cancelled drag to the same revert path focused editing already uses. Pill: centring it over a field near the swatch's left edge pushed it past that edge, where it was clipped. Anchor it leading so it grows rightward into the gutter the row already reserves.
1 parent bb6435b commit 7e5b1d5

3 files changed

Lines changed: 87 additions & 4 deletions

File tree

Pika/Views/EditableColorValue.swift

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,12 @@ struct EditableColorValue: View {
111111
/// trackpad momentum — can tell "this is still my session" apart from "a different field
112112
/// has since taken over, or this session already ended and a new one started elsewhere."
113113
@State private var sessionOwner: Int?
114+
/// The component values as they were when the session began, kept pristine for its whole
115+
/// duration. Every scrub frame recomposes from *these* rather than from `values`, which now
116+
/// tracks the clamped colour: feeding each frame's clamped result back in would make the drag
117+
/// path-dependent, ratcheting the untouched components a little further every frame so
118+
/// dragging back where you came from no longer returns the colour you started with.
119+
@State private var sessionStartValues: [String] = []
114120

115121
private var decomposed: DecomposedColor {
116122
format.decompose(eyedropper.color, style: style, in: colorSpace)
@@ -311,6 +317,7 @@ struct EditableColorValue: View {
311317
/// `finishEditing`/`abortEditingForExternalPick` have a consistent point to commit or revert to.
312318
private func startSession(layout: DecomposedColor) {
313319
isEditing = true
320+
sessionStartValues = layout.values
314321
// Sized to the *widest possible* value for this format, not the current one: keeping
315322
// `frozenSize` in step with the live value (as it started out) only froze the font size,
316323
// not the wrap decision — a component can still change digit count as it's scrubbed
@@ -386,8 +393,10 @@ struct EditableColorValue: View {
386393
@discardableResult
387394
private func previewLiveScrub(index: Int, layout: DecomposedColor, value: Double) -> Double {
388395
guard index < layout.components.count else { return value }
389-
let currentKey = FormatStyleKey(format: format, style: style, colorSpace: colorSpace)
390-
var liveValues = (valuesKey == currentKey && values.count == layout.components.count) ? values : layout.values
396+
// From the session's starting values, never the live (clamped) ones — see
397+
// `sessionStartValues`. This is what makes a scrub reversible: drag chroma up into the
398+
// clamped region and back down, and you land on exactly the colour you began with.
399+
var liveValues = sessionStartValues.count == layout.components.count ? sessionStartValues : layout.values
391400
guard index < liveValues.count else { return value }
392401
liveValues[index] = ColorComponentField.formattedDragValue(value, kind: layout.components[index].kind)
393402
guard let color = format.recompose(liveValues, style: style, in: colorSpace) else { return value }
@@ -591,6 +600,10 @@ struct ColorComponentField: View {
591600
onDragBegin: onDragBegin,
592601
onDragEnd: onDragEnd,
593602
onScrubPreview: { scrubPreviewText = $0 },
603+
onDragCancel: {
604+
scrubPreviewText = nil
605+
onCancel()
606+
},
594607
onLiveValue: onLiveValue,
595608
onStep: isDraggable ? stepValue : nil
596609
)
@@ -610,7 +623,10 @@ struct ColorComponentField: View {
610623
)
611624
// Purely decorative: an `.overlay` doesn't feed back into this view's own reported size,
612625
// so the pill can appear, change text, and disappear without ever perturbing `FlowLayout`.
613-
.overlay(alignment: .top) {
626+
// Leading, not centred: centring a wide pill over a field near the swatch's left edge
627+
// pushes it past that edge, where it's clipped (the value row starts hard against it).
628+
// Growing rightward instead keeps it inside — the row reserves a trailing gutter anyway.
629+
.overlay(alignment: .topLeading) {
614630
if let scrubPreviewText {
615631
Text(scrubPreviewText)
616632
.font(.system(size: 12, weight: .semibold, design: .rounded))

Pika/Views/ScrubTextField.swift

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@ struct ScrubbableColorField: NSViewRepresentable {
109109
/// instead of the field's own text, which stays frozen for the whole drag (see
110110
/// `EditableColorValue.scrubPreviewText`).
111111
let onScrubPreview: (String?) -> Void
112+
/// Escape pressed mid-drag — abandon the scrub and put the colour back how it was.
113+
let onDragCancel: () -> Void
112114
/// Fired with the raw live value on every drag step, so the parent can preview the eyedropper
113115
/// colour without touching `text` (which stays frozen — see `onScrubPreview` above).
114116
let onLiveValue: (Double) -> Double
@@ -159,6 +161,10 @@ struct ScrubbableColorField: NSViewRepresentable {
159161
nsView.range = range
160162
nsView.kind = kind
161163
nsView.onDragBegin = onDragBegin
164+
nsView.onDragCancel = { [weak nsView] in
165+
nsView?.lastAchievedValue = nil
166+
onDragCancel()
167+
}
162168
// The field's own `stringValue` is deliberately never touched here — it stays frozen at
163169
// whatever it showed when the drag began, for `FlowLayout`'s benefit (see
164170
// `EditableColorValue.scrubPreviewText`). Only the floating pill sees the live value.
@@ -279,6 +285,8 @@ final class ScrubTextField: NSTextField {
279285
var onDragChanged: ((Double) -> Void)?
280286
/// Fires with the drag's final value once it ends.
281287
var onDragEnd: ((Double) -> Void)?
288+
/// Fires instead of `onDragEnd` when the drag is abandoned with Escape.
289+
var onDragCancel: (() -> Void)?
282290
/// Reports true/false as this field becomes/resigns first responder. Driven from these
283291
/// overrides rather than `NSTextFieldDelegate`'s controlTextDidBeginEditing/EndEditing —
284292
/// see the note at the `onFocusChange` assignment in `ScrubbableColorField.updateNSView`.
@@ -394,7 +402,7 @@ final class ScrubTextField: NSTextField {
394402
// of starting fresh.
395403
while true {
396404
guard let next = NSApp.nextEvent(
397-
matching: [.leftMouseDragged, .leftMouseUp],
405+
matching: [.leftMouseDragged, .leftMouseUp, .keyDown],
398406
until: .distantFuture,
399407
inMode: .eventTracking,
400408
dequeue: true
@@ -404,6 +412,12 @@ final class ScrubTextField: NSTextField {
404412
}
405413

406414
switch next.type {
415+
case .keyDown:
416+
// Escape abandons the scrub. Only meaningful once a drag is actually under way;
417+
// otherwise let the key fall through to its normal handling.
418+
guard didBeginDrag, next.keyCode == 53 else { continue }
419+
cancelDrag()
420+
return
407421
case .leftMouseDragged:
408422
let translationX = next.locationInWindow.x - startPoint.x
409423
if !didBeginDrag {
@@ -509,6 +523,13 @@ final class ScrubTextField: NSTextField {
509523
onDragChanged?(newValue)
510524
}
511525

526+
private func cancelDrag() {
527+
dragAnchorValue = nil
528+
lastDragValue = nil
529+
NSCursor.arrow.set()
530+
onDragCancel?()
531+
}
532+
512533
private func finishDrag() {
513534
dragAnchorValue = nil
514535
NSCursor.arrow.set()

PikaTests/EditableColorValueTests.swift

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,50 @@ final class EditableColorValueTests: XCTestCase {
4747
func test_stableDecimalPlaces_moreThanFourDecimals_clampsDownToFour() {
4848
XCTAssertEqual(ColorComponentField.stableDecimalPlaces(for: "5.123456"), 4)
4949
}
50+
51+
// MARK: - Scrub reversibility / gamut clamping
52+
53+
/// Recomposing from a session's *starting* values is what makes a scrub reversible. Pushing
54+
/// OKLCH chroma past the sRGB gamut clamps the colour, and decomposing that clamped colour
55+
/// reports a different lightness and hue — so feeding each frame's result back in would drag
56+
/// the untouched components along with it and you could never return to where you began.
57+
func test_scrubFromPristineValues_isReversible() {
58+
let start = ["40.68", "0.2173", "264.58"]
59+
let format = ColorFormat.oklch
60+
let space = NSColorSpace.sRGB
61+
62+
guard let original = format.recompose(start, style: .css, in: space) else {
63+
return XCTFail("expected the starting values to recompose")
64+
}
65+
66+
// Drag chroma far out of gamut, then back to exactly where it started.
67+
var pushed = start
68+
pushed[1] = "1.0"
69+
guard let clamped = format.recompose(pushed, style: .css, in: space) else {
70+
return XCTFail("expected the out-of-gamut values to recompose")
71+
}
72+
let clampedBack = format.decompose(clamped, style: .css, in: space)
73+
XCTAssertNotEqual(clampedBack.values[0], start[0], "clamping should have moved lightness")
74+
XCTAssertNotEqual(clampedBack.values[2], start[2], "clamping should have moved hue")
75+
76+
// Returning to the original chroma from the pristine starting values restores the colour.
77+
guard let restored = format.recompose(start, style: .css, in: space) else {
78+
return XCTFail("expected the restored values to recompose")
79+
}
80+
XCTAssertEqual(restored.toHex(in: space), original.toHex(in: space))
81+
82+
// Whereas carrying the clamped values forward does not.
83+
var ratcheted = clampedBack.values
84+
ratcheted[1] = start[1]
85+
guard let notRestored = format.recompose(ratcheted, style: .css, in: space) else {
86+
return XCTFail("expected the ratcheted values to recompose")
87+
}
88+
XCTAssertNotEqual(notRestored.toHex(in: space), original.toHex(in: space))
89+
}
90+
91+
func test_naturalDecimalPlaces_matchesTheStepItCanResolve() {
92+
XCTAssertEqual(ColorComponentField.naturalDecimalPlaces(forRange: 0 ... 360), 0)
93+
XCTAssertEqual(ColorComponentField.naturalDecimalPlaces(forRange: 0 ... 100), 1)
94+
XCTAssertEqual(ColorComponentField.naturalDecimalPlaces(forRange: 0 ... 1), 3)
95+
}
5096
}

0 commit comments

Comments
 (0)