Skip to content

Commit 41263b3

Browse files
angusbezzinaclaude
andcommitted
fix(overlay): the pill is pinned to the panel's corner, not centred in it
Closing the menu shortened the pill leftwards and then jumped it back to the right. The animation was never the problem: the pill was in the wrong place for a third of a second on every open and close. OverlayPlacement.toolbarFrame computes the panel's frame BACKWARDS from where the control has to land, subtracting the trailing/bottom chrome. That is only true of the drawn pill if the content is pinned to that corner, and a fixedSize root in an oversized NSHostingView is CENTRED instead. Since VRT-jiuo the panel grows the instant the mode flips and shrinks 0.3s later (early shrinking clips the pill mid-animation), so the two disagree across the whole transition. Measured: idle content (72pt) in the still-open panel (258pt) sits 93pt from each edge instead of 14pt from the trailing one. The pill collapsed toward the middle of the still-wide panel, then snapped 79pt right when the panel finally narrowed under it. Pinning bottom-trailing leaves the width animation as the only thing that moves: the pill opens leftwards and closes back in from the left, and the late shrink is invisible because it only takes away space the pill never occupied. It also corrects the same offset at mount, where the panel is still at its generous unmeasured seed. The flexible frame does not disturb the measurement the same view is used for: an unspecified proposal, which is what fittingSize asks with, resolves it to the content's ideal size. Pinned by a test, along with the anchoring itself, which is asserted by rasterising the real view and finding the pill's opaque body. PanelFrameEnforcementTests passed throughout because it derives the pill's corner FROM the panel frame, which is the assumption that was false. 233 unit tests, warning-free build, iOS simulator cross-compile clean, probe 11/11 phases PASS against a live AX tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 181b42b commit 41263b3

2 files changed

Lines changed: 158 additions & 7 deletions

File tree

Sources/AnnotKit/Overlay/OverlayView.swift

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -646,13 +646,14 @@ struct ToolbarOverlayView: View {
646646
let onExport: () -> Void
647647

648648
var body: some View {
649-
// JUST the pill, inset by the space its shadow and count badge need. There
650-
// are no Spacers pushing it into a corner any more, because the panel is no
651-
// longer a fixed 240x104 rect the pill sits in the corner of — it is sized
652-
// to THIS view (`fittingSize`) and placed so the pill lands exactly where it
653-
// always has. The panel covering only the control is the point: every pixel
654-
// it covers is a pixel the host app cannot be clicked through, in both
655-
// modes, permanently.
649+
// JUST the pill, inset by the space its shadow and count badge need. The
650+
// panel is no longer a fixed 240x104 rect with the pill parked in one
651+
// corner of it — it is sized to THIS view (`fittingSize`) and placed so the
652+
// pill lands exactly where it always has. The panel covering only the
653+
// control is the point: every pixel it covers is a pixel the host app
654+
// cannot be clicked through, in both modes, permanently. The corner
655+
// alignment below is what keeps that true while the panel is briefly LARGER
656+
// than the pill, which is every open and close.
656657
ToolbarView(
657658
session: session,
658659
onToggle: onToggle,
@@ -661,6 +662,29 @@ struct ToolbarOverlayView: View {
661662
)
662663
.padding(PillStyle.panelChrome)
663664
.fixedSize()
665+
// PINNED to the panel's bottom-trailing corner, which is where
666+
// ``OverlayPlacement/toolbarFrame(hostFrame:visibleFrame:panelSize:)``
667+
// computes backwards FROM — it subtracts the trailing/bottom chrome to put
668+
// the pill at its inset from the host's corner. Without this the pill is
669+
// CENTERED in whatever bounds the panel has, and the two agree only while
670+
// the panel is exactly the measured size.
671+
//
672+
// They disagree for a third of a second on EVERY open and close, because
673+
// the panel grows immediately and shrinks late (see
674+
// ``OverlayController/scheduleToolbarShrink()``), and that is the whole of
675+
// the "it shortens to the left, then jumps right" close animation: a
676+
// centered pill collapsed toward the middle of the still-wide panel, then
677+
// snapped back to the corner when the panel finally narrowed under it. With
678+
// the corner pinned, the width animation is the ONLY thing that moves — the
679+
// pill opens leftwards and closes back in from the left — and the late
680+
// shrink is invisible because it only takes away space the pill was never
681+
// occupying. It also fixes the same offset at mount, where the panel is
682+
// still at its generous unmeasured seed.
683+
//
684+
// `maxWidth`/`maxHeight` do not disturb the measurement this same view is
685+
// used for: an unspecified proposal (what `fittingSize` asks with) resolves
686+
// a flexible frame to its content's ideal size. Verified.
687+
.frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .bottomTrailing)
664688
.ignoresSafeArea()
665689
.accessibilityHidden(true)
666690
}
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
#if os(macOS)
2+
import AppKit
3+
import SwiftUI
4+
import XCTest
5+
@testable import AnnotKit
6+
7+
/// "Closing the menu shortens it to the left, then it jumps to the right."
8+
///
9+
/// The toolbar panel does not change size in step with the pill it carries. It
10+
/// GROWS the instant the mode flips and SHRINKS a third of a second later, once the
11+
/// pill's 0.15s width animation has settled (``OverlayController``'s asymmetry: a
12+
/// panel that shrinks early clips the control mid-animation). So on every open and
13+
/// close there is a window in which the panel is wider than its content — and
14+
/// ``OverlayPlacement/toolbarFrame(hostFrame:visibleFrame:panelSize:)`` places the
15+
/// panel by computing backwards from the pill's corner, which is only where the
16+
/// pill actually IS if the content is pinned to that corner.
17+
///
18+
/// It was not: a `fixedSize` root in an oversized `NSHostingView` is CENTERED.
19+
/// Measured before the fix, idle content (72pt) in the still-open panel (258pt):
20+
/// the pill sat 93pt from each edge instead of 14pt from the trailing one — so it
21+
/// collapsed toward the middle as it narrowed, then snapped 79pt right when the
22+
/// panel finally shrank under it.
23+
///
24+
/// These tests rasterise the real view and look for the pill's opaque body, because
25+
/// the bug is entirely about where the control is DRAWN — every value the placement
26+
/// math produced was already correct. That is also why
27+
/// `PanelFrameEnforcementTests.testTheToolbarDoesNotMoveWhenTheMenuOpensOrCloses`
28+
/// passed throughout: it derives the pill's corner FROM the panel frame, which is
29+
/// the very assumption that only held once the panel had settled.
30+
@MainActor
31+
final class ToolbarPillAnchorTests: XCTestCase {
32+
/// The panel size the controller holds during a close: measured for ANNOTATE
33+
/// mode, while the pill has already animated back down to the lone pencil.
34+
private let oversized = CGSize(width: 258, height: 104)
35+
36+
private func makeSession() -> AnnotationSession {
37+
AnnotationSession(source: MacElementSource(), sink: NotesFileSink(path: "/dev/null"))
38+
}
39+
40+
/// The mid-close state: idle content, panel still at its annotate-mode width.
41+
func testThePillKeepsItsCornerWhileThePanelIsWiderThanTheContent() throws {
42+
let pill = try drawnPillFrame(panelSize: oversized, session: makeSession())
43+
XCTAssertEqual(oversized.width - pill.maxX, PillStyle.panelChrome.trailing, accuracy: 1,
44+
"the pill's trailing edge must stay one chrome margin from the panel's, "
45+
+ "or the panel's late shrink drags the control sideways")
46+
XCTAssertEqual(pill.minY, PillStyle.panelChrome.bottom, accuracy: 1,
47+
"same for the bottom edge — the seed panel is taller than the pill too")
48+
}
49+
50+
/// The settled state, and the one the placement math is written against. The
51+
/// two must agree pixel for pixel: that they did NOT is the jump.
52+
func testTheSettledPanelDrawsThePillInTheSamePlaceAsTheOversizedOne() throws {
53+
let session = makeSession()
54+
let snugSize = measuredSize(session: session)
55+
let snug = try drawnPillFrame(panelSize: snugSize, session: session)
56+
let wide = try drawnPillFrame(panelSize: oversized, session: session)
57+
58+
XCTAssertEqual(snugSize.width - snug.maxX, oversized.width - wide.maxX, accuracy: 1)
59+
XCTAssertEqual(snug.minY, wide.minY, accuracy: 1)
60+
XCTAssertEqual(snug.size.width, wide.size.width, accuracy: 1)
61+
}
62+
63+
/// The corner pin is a `maxWidth`/`maxHeight` frame, and the SAME view is what
64+
/// ``OverlayController/measuredToolbarSize()`` sizes the panel from. A flexible
65+
/// frame that reported its max instead of its content's ideal would inflate the
66+
/// panel to fill the screen — every pixel of which is a pixel of the host app
67+
/// that can no longer be clicked.
68+
func testPinningTheCornerDoesNotInflateTheMeasuredSize() {
69+
let size = measuredSize(session: makeSession())
70+
XCTAssertLessThan(size.width, 200, "idle is one 44pt pill plus 14pt of chrome a side")
71+
XCTAssertLessThan(size.height, 120)
72+
XCTAssertGreaterThan(size.width, 40)
73+
XCTAssertGreaterThan(size.height, 40)
74+
}
75+
76+
// MARK: - Rasterising
77+
78+
private func toolbarView(session: AnnotationSession) -> ToolbarOverlayView {
79+
ToolbarOverlayView(session: session, onToggle: {}, onCopy: {}, onExport: {})
80+
}
81+
82+
/// What the controller sizes the panel from: a fresh, uninstalled hosting view's
83+
/// `fittingSize`, with the default sizing options that make it report one.
84+
private func measuredSize(session: AnnotationSession) -> CGSize {
85+
NSHostingView(rootView: toolbarView(session: session)).fittingSize
86+
}
87+
88+
/// The pill's drawn bounds inside a panel of `panelSize`, in AppKit's y-UP
89+
/// panel-local coordinates.
90+
///
91+
/// Found by alpha, not by colour: the pill's capsule is opaque, and the only
92+
/// other thing the view draws is its shadow — black at 0.4, radius 12 — so a
93+
/// high alpha threshold picks out the body and ignores the halo around it.
94+
private func drawnPillFrame(panelSize: CGSize, session: AnnotationSession) throws -> CGRect {
95+
let view = NSHostingView(rootView: toolbarView(session: session))
96+
// Match the panel's own hosting view: the controller owns the frame, so the
97+
// view must NOT resize itself to its content — being stretched past its
98+
// content is the whole state under test.
99+
view.sizingOptions = []
100+
view.frame = CGRect(origin: .zero, size: panelSize)
101+
view.layoutSubtreeIfNeeded()
102+
103+
let rep = try XCTUnwrap(view.bitmapImageRepForCachingDisplay(in: view.bounds),
104+
"no bitmap backing for the hosting view")
105+
view.cacheDisplay(in: view.bounds, to: rep)
106+
107+
let scale = CGFloat(rep.pixelsWide) / panelSize.width
108+
var minX = Int.max, maxX = Int.min, minY = Int.max, maxY = Int.min
109+
for y in 0..<rep.pixelsHigh {
110+
for x in 0..<rep.pixelsWide {
111+
guard let color = rep.colorAt(x: x, y: y), color.alphaComponent > 0.9 else { continue }
112+
minX = min(minX, x); maxX = max(maxX, x)
113+
minY = min(minY, y); maxY = max(maxY, y)
114+
}
115+
}
116+
try XCTSkipIf(minX == Int.max, "the offscreen rasterisation drew nothing on this machine")
117+
118+
// The bitmap is y-DOWN; the panel geometry this is compared against is y-UP.
119+
let top = CGFloat(minY) / scale
120+
let bottom = CGFloat(maxY + 1) / scale
121+
return CGRect(x: CGFloat(minX) / scale,
122+
y: panelSize.height - bottom,
123+
width: CGFloat(maxX + 1 - minX) / scale,
124+
height: bottom - top)
125+
}
126+
}
127+
#endif

0 commit comments

Comments
 (0)