Make annotations less ugly

gnachman · gnachman · commit 0a4b3eafe102 · 2026-02-27T15:50:11.000-08:00
diff --git a/sources/PTYNoteView.m b/sources/PTYNoteView.m
@@ -7,18 +7,18 @@
 //
 
 #import "PTYNoteView.h"
+#import "NSAppearance+iTerm.h"
 #import "SFSymbolEnum/SFSymbolEnum.h"
 #import "iTermMouseCursor.h"
 #import "NSImage+iTerm.h"
 
 static const CGFloat kMinWidth = 50;
-static const CGFloat kMinHeight = 33;
+static const CGFloat kMinHeight = 39;
 
-static const CGFloat kLeftMargin = 5;
-static const CGFloat kRightMargin = 14;
-static const CGFloat kTopMargin = 4;
-static const CGFloat kBottomMargin = 0;
-static const CGFloat kKillButtonTopMargin = 2;
+static const CGFloat kLeftMargin = 8;
+static const CGFloat kRightMargin = 20;
+static const CGFloat kTopMargin = 6;
+static const CGFloat kBottomMargin = 2;
 
 static const CGFloat kInset = 5;
 static const CGFloat kRadius = 5;
@@ -41,46 +41,63 @@ - (instancetype)initWithFrame:(NSRect)frame {
     self = [super initWithFrame:frame];
     if (self) {
         backgroundColor_ = [[self defaultBackgroundColor] retain];
-        NSImage *closeImage;
-        if (@available(macOS 11.0, *)) {
-            closeImage = [NSImage imageWithSystemSymbolName:SFSymbolGetString(SFSymbolTrash)
-                                   accessibilityDescription:@"Delete annotation"];
-            closeImage = [closeImage it_imageWithTintColor:[NSColor blackColor]];
-        } else {
-            closeImage = [NSImage it_imageNamed:@"closebutton" forClass:self.class];
-        }
         killButton_ = [[NSButton alloc] initWithFrame:NSMakeRect(0, 0, kButtonSize, kButtonSize)];
         [killButton_ setButtonType:NSButtonTypeMomentaryPushIn];
-        [killButton_ setImage:closeImage];
         [killButton_ setTarget:self];
         [killButton_ setAction:@selector(kill:)];
         [killButton_ setBordered:NO];
         [[killButton_ cell] setHighlightsBy:NSContentsCellMask];
         [killButton_ setTitle:@""];
         [self addSubview:killButton_];
+        [self updateKillButtonImage];
         [killButton_ release];
     }
     return self;
 }
 
+- (void)updateKillButtonImage {
+    NSImage *closeImage;
+    if (@available(macOS 11.0, *)) {
+        closeImage = [NSImage imageWithSystemSymbolName:SFSymbolGetString(SFSymbolTrash)
+                               accessibilityDescription:@"Delete annotation"];
+        NSColor *tintColor = self.effectiveAppearance.it_isDark ? [NSColor whiteColor] : [NSColor blackColor];
+        closeImage = [closeImage it_imageWithTintColor:tintColor];
+    } else {
+        closeImage = [NSImage it_imageNamed:@"closebutton" forClass:self.class];
+    }
+    [killButton_ setImage:closeImage];
+}
+
 - (void)dealloc {
     [contentView_ release];
     [backgroundColor_ release];
     [super dealloc];
 }
 
 - (NSColor *)defaultBackgroundColor {
-    return [NSColor colorWithCalibratedRed:252.0/255.0
+    if (self.effectiveAppearance.it_isDark) {
+        return [NSColor colorWithCalibratedRed:50.0/255.0
+                                         green:50.0/255.0
+                                          blue:52.0/255.0
+                                         alpha:1];
+    }
+    return [NSColor colorWithCalibratedRed:250.0/255.0
                                      green:250.0/255.0
-                                      blue:198.0/255.0
+                                      blue:250.0/255.0
                                      alpha:1];
 }
 
 - (NSColor *)borderColor {
-    return [NSColor colorWithCalibratedRed:255.0/255.0
-                                     green:229.0/255.0
-                                      blue:114.0/255.0
-                                     alpha:0.95];
+    if (self.effectiveAppearance.it_isDark) {
+        return [NSColor colorWithCalibratedRed:1
+                                         green:1
+                                          blue:1
+                                         alpha:0.4];
+    }
+    return [NSColor colorWithCalibratedRed:0
+                                     green:0
+                                      blue:0
+                                     alpha:0.1];
 }
 
 static NSPoint FlipPoint(NSPoint p, CGFloat height) {
@@ -312,11 +329,21 @@ - (NSBezierPath *)roundedRectangleWithPointerInRect:(NSRect)frame
 - (void)viewDidMoveToSuperview {
     [super viewDidMoveToSuperview];
 
+    [self updateKillButtonImage];
+
     NSClickGestureRecognizer *clickRecognizer = [[NSClickGestureRecognizer alloc] initWithTarget:self action:@selector(handleClick:)];
     clickRecognizer.delegate = self;
     [self addGestureRecognizer:clickRecognizer];
 }
 
+- (void)viewDidChangeEffectiveAppearance {
+    [super viewDidChangeEffectiveAppearance];
+    [backgroundColor_ release];
+    backgroundColor_ = [[self defaultBackgroundColor] retain];
+    [self updateKillButtonImage];
+    [self setNeedsDisplay:YES];
+}
+
 - (void)handleClick:(NSClickGestureRecognizer *)recognizer {
     if (recognizer.state == NSGestureRecognizerStateRecognized) {
         [self reorderAboveSiblings];
@@ -487,28 +514,39 @@ - (void)layoutSubviews {
                                                  bubbleFrame.size.width - kLeftMargin - kRightMargin,
                                                  bubbleFrame.size.height - kTopMargin - kBottomMargin),
                                       self.frame.size.height);
+    // Calculate what the centered position would be at minimum height to avoid jumps
+    CGFloat minBubbleHeight = kMinHeight - kPointerLength - kInset;
+    CGFloat centeredMarginAtMinHeight = (minBubbleHeight - kButtonSize) / 2.0;
+    CGFloat killButtonY = NSMinY(bubbleFrame) + centeredMarginAtMinHeight;
     killButton_.frame = FlipRect(NSMakeRect(NSMaxX(bubbleFrame) - kButtonSize,
-                                            NSMinY(bubbleFrame) + kKillButtonTopMargin,
+                                            killButtonY,
                                             kButtonSize,
                                             kButtonSize),
                                  self.frame.size.height);
     [self.window invalidateCursorRectsForView:self];
 }
 
 - (NSSize)sizeThatFitsContentView {
+    CGFloat width;
+    CGFloat height;
     switch (tipEdge_) {
         case kPTYNoteViewTipEdgeBottom:
         case kPTYNoteViewTipEdgeTop:
-            return NSMakeSize(contentView_.frame.size.width + kLeftMargin + kRightMargin + kInset,
-                              contentView_.frame.size.height + kBottomMargin + kTopMargin + kPointerLength + kInset);
+            width = contentView_.frame.size.width + kLeftMargin + kRightMargin + kInset;
+            height = contentView_.frame.size.height + kBottomMargin + kTopMargin + kPointerLength + kInset;
+            break;
 
         case kPTYNoteViewTipEdgeLeft:
         case kPTYNoteViewTipEdgeRight:
-            return NSMakeSize(contentView_.frame.size.width + kLeftMargin + kRightMargin + kPointerLength + kInset,
-                              contentView_.frame.size.height + kBottomMargin + kTopMargin + kInset);
+            width = contentView_.frame.size.width + kLeftMargin + kRightMargin + kPointerLength + kInset;
+            height = contentView_.frame.size.height + kBottomMargin + kTopMargin + kInset;
+            break;
+
+        default:
+            assert(false);
+            return NSMakeSize(0, 0);
     }
-    assert(false);
-    return NSMakeSize(0, 0);
+    return NSMakeSize(MAX(width, kMinWidth), MAX(height, kMinHeight));
 }
 
 - (void)kill:(id)sender {
diff --git a/sources/PTYNoteViewController.m b/sources/PTYNoteViewController.m
@@ -86,9 +86,9 @@ - (void)loadView {
     self.noteView.autoresizesSubviews = YES;
     self.noteView.delegate = self;
     NSShadow *shadow = [[NSShadow alloc] init];
-    shadow.shadowColor = [[NSColor blackColor] colorWithAlphaComponent:0.5];
-    shadow.shadowOffset = NSMakeSize(1, -1);
-    shadow.shadowBlurRadius = 1.0;
+    shadow.shadowColor = [[NSColor blackColor] colorWithAlphaComponent:0.15];
+    shadow.shadowOffset = NSMakeSize(0, -2);
+    shadow.shadowBlurRadius = 8.0;
     self.noteView.wantsLayer = YES;
     self.noteView.shadow = shadow;
 
@@ -117,7 +117,6 @@ - (void)loadView {
     textView_.textContainer.containerSize = NSMakeSize(scrollView_.frame.size.width, FLT_MAX);
     textView_.textContainer.widthTracksTextView = YES;
     textView_.delegate = self;
-    textView_.appearance = [NSAppearance appearanceNamed:NSAppearanceNameAqua];
     scrollView_.documentView = textView_;
 
     // Put the scrollview in a wrapper so we can have a few pixels of padding
@@ -142,12 +141,31 @@ - (void)setAnchor:(NSPoint)anchor {
     anchor_ = anchor;
 
     NSRect superViewFrame = noteView_.superview.frame;
-    CGFloat xOffset = 0;
-    if (anchor_.x + noteView_.frame.size.width > superViewFrame.size.width) {
-        xOffset = anchor_.x + noteView_.frame.size.width - superViewFrame.size.width;
+    NSSize size = [noteView_ sizeThatFitsContentView];
+
+    // Position the bubble so the pointer is roughly centered, making it more vertical.
+    // xOffset is how far the pointer tip is from the left edge of the bubble.
+    const CGFloat pointerInset = 40;
+    CGFloat xOffset;
+
+    // Ideal bubble position: pointer at pointerInset from left edge.
+    CGFloat idealBubbleX = anchor_.x - pointerInset;
+
+    if (idealBubbleX < 0) {
+        // Bubble would go off left edge - align to left edge.
+        xOffset = anchor_.x;
+    } else if (idealBubbleX + size.width > superViewFrame.size.width) {
+        // Bubble would go off right edge - align to right edge.
+        xOffset = anchor_.x - (superViewFrame.size.width - size.width);
+    } else {
+        // Bubble fits with ideal positioning.
+        xOffset = pointerInset;
     }
+
+    // Clamp xOffset to valid range (pointer must be within bubble).
+    xOffset = MAX(0, MIN(xOffset, size.width));
+
     noteView_.tipEdge = kPTYNoteViewTipEdgeTop;
-    NSSize size = [noteView_ sizeThatFitsContentView];
     noteView_.point = NSMakePoint(xOffset, 0);
     noteView_.frame = NSMakeRect(anchor_.x - xOffset,
                                  anchor_.y,
@@ -333,7 +351,7 @@ - (void)updateBackgroundColor {
     alpha = alpha * alpha;
 
     NSColor *defaultBg = [self.noteView defaultBackgroundColor];
-    CGFloat highlightComponents[] = { 0.9, 0.8, 0 };
+    CGFloat highlightComponents[] = { 0.85, 0.92, 1.0 };
     CGFloat components[3] = {
         [defaultBg redComponent] * alpha + (1 - alpha) * highlightComponents[0],
         [defaultBg greenComponent] * alpha + (1 - alpha) * highlightComponents[1],
diff --git a/sources/iTermTextDrawingHelper.m b/sources/iTermTextDrawingHelper.m
@@ -1395,9 +1395,7 @@ - (void)drawNoteRangesOnLine:(int)line
 
             CGFloat maxX = MIN(_frame.size.width - [iTermPreferences sideMargins], range.length * _cellSize.width + x);
             CGFloat w = maxX - x;
-            iTermRectFill(NSMakeRect(x, y + _cellSize.height - 1.5, w, 1), virtualOffset);
-            [[NSColor orangeColor] set];
-            iTermRectFill(NSMakeRect(x, y + _cellSize.height - 1, w, 1), virtualOffset);
+            iTermRectFill(NSMakeRect(x, y + _cellSize.height - 2, w, 1), virtualOffset);
         }
 
     }

Original file line number	Diff line number	Diff line change
`@@ -1395,9 +1395,7 @@ - (void)drawNoteRangesOnLine:(int)line`
`1395`	`1395`
`1396`	`1396`	`CGFloat maxX = MIN(_frame.size.width - [iTermPreferences sideMargins], range.length * _cellSize.width + x);`
`1397`	`1397`	`CGFloat w = maxX - x;`
`1398`		`- iTermRectFill(NSMakeRect(x, y + _cellSize.height - 1.5, w, 1), virtualOffset);`
`1399`		`- [[NSColor orangeColor] set];`
`1400`		`- iTermRectFill(NSMakeRect(x, y + _cellSize.height - 1, w, 1), virtualOffset);`
	`1398`	`+ iTermRectFill(NSMakeRect(x, y + _cellSize.height - 2, w, 1), virtualOffset);`
`1401`	`1399`	`}`
`1402`	`1400`
`1403`	`1401`	`}`