From 4c3da8e536e1a89bc39bd3063b651e57b01b9e6f Mon Sep 17 00:00:00 2001 From: Lukas Frias Santos Date: Sun, 19 Jul 2026 11:29:37 +0200 Subject: [PATCH 01/11] Floor the wrapped-text measurement width so lines don't clip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MessageTextView.sizeThatFits measured wrapped text at the exact proposed width, but MessageTextContent.setFrameSize syncs the text container to the device-pixel-rounded frame width (<= proposed). A line sitting right at the wrap boundary then wrapped one extra line on screen that the measured row height never accounted for, clipping the last line (or a trailing link card). Measure the wrapped case at pw.rounded(.down) so measurement and render use the same integral width — guarded with max(1, ...) so a sub-point proposed width can't floor to a zero-width text container and produce a garbage measured height. Co-Authored-By: Claude Opus 4.8 --- Relay/Views/Message/MessageTextView.swift | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/Relay/Views/Message/MessageTextView.swift b/Relay/Views/Message/MessageTextView.swift index 8bbfa3fd..d032d2c3 100644 --- a/Relay/Views/Message/MessageTextView.swift +++ b/Relay/Views/Message/MessageTextView.swift @@ -245,11 +245,24 @@ struct MessageTextView: NSViewRepresentable { // swiftlint:disable:next identifier_name if let pw = proposedWidth, pw > 0 { if tightWidth > pw { - // Text must wrap to fit the proposed width. - container.containerSize = NSSize(width: pw, height: CGFloat.greatestFiniteMagnitude) + // Text must wrap. Measure at the *integral* width the text + // container will actually be assigned at render time: SwiftUI + // hands the NSView a device-pixel-rounded frame, and + // `MessageTextContent.setFrameSize` syncs the container to that + // frame width — which is a fraction narrower than a fractional + // `pw`. Measuring at `pw` while the live container ends up at + // `floor(pw)` makes a line sitting right at the wrap boundary + // wrap one extra line on screen, overflowing the measured row + // height and clipping the last line (or a trailing link-preview + // card pushed down by it). Flooring keeps measurement and render + // on the same width so wrapping — and therefore height — agree. + // Guard against a sub-point proposal flooring to 0, which would + // give a zero-width container and a garbage height. + let wrapWidth = max(1, pw.rounded(.down)) + container.containerSize = NSSize(width: wrapWidth, height: CGFloat.greatestFiniteMagnitude) lm.ensureLayout(for: container) let constrainedHeight = lm.usedRect(for: container).height - result = CGSize(width: pw, height: ceil(constrainedHeight)) + result = CGSize(width: wrapWidth, height: ceil(constrainedHeight)) } else { // Text fits on fewer lines — hug the text width but never // exceed the proposed width. This ensures SwiftUI sets the From 26e5e644565ae04d9b0cdbf94a08a45882e8d02e Mon Sep 17 00:00:00 2001 From: Lukas Frias Santos Date: Sun, 19 Jul 2026 11:29:56 +0200 Subject: [PATCH 02/11] Size mention pills to the message font MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PillTextAttachment.paddedBounds sized the pill to ~18pt — taller than the ~15.3pt font line box — and centered it on the font midline. That grew the pill's line fragment ~2pt, stretched the 16pt pill image ~12%, and pinned the pill's top flush against the bubble's inner top edge, reading as a clipped, vertically tight pill on a message's first line. Cap paddedHeight to ascender - descender so a pill line matches a normal text line. MentionPillView also drew its label at a fixed .callout style while the attachment bounds were sized from the surrounding font, so at a larger font the small glyph bitmap was upscaled into the larger bounds and the capsule read as stretched and blurry. Pass the font size into the pill view and size the attachment to the rendered image, so the bitmap draws 1:1 and scales cleanly with the message font (and the text-zoom level). Co-Authored-By: Claude Opus 4.8 --- Relay/Utilities/MentionPillView.swift | 9 ++- Relay/Utilities/PillTextAttachment.swift | 93 ++++++++++++------------ 2 files changed, 54 insertions(+), 48 deletions(-) diff --git a/Relay/Utilities/MentionPillView.swift b/Relay/Utilities/MentionPillView.swift index b4472908..33f8d957 100644 --- a/Relay/Utilities/MentionPillView.swift +++ b/Relay/Utilities/MentionPillView.swift @@ -62,6 +62,12 @@ struct MentionPillView: View { /// Set to `false` for keyword highlight pills. var showAtPrefix: Bool = true + /// Point size of the surrounding message text. The pill's own text renders + /// one point smaller than this, so the pill scales with the timeline's + /// text-zoom level and the rendered bitmap always matches the attachment + /// bounds rather than being upscaled (which reads as a stretched capsule). + var fontSize: CGFloat = NSFont.systemFontSize + private var pillText: String { if !showAtPrefix { return displayName } return displayName.hasPrefix("@") ? displayName : "@\(displayName)" @@ -91,8 +97,7 @@ struct MentionPillView: View { var body: some View { Text(pillText) - .font(.callout) - .bold() + .font(.system(size: fontSize - 1, weight: .bold)) .foregroundStyle(textColor) .padding(.horizontal, 4) .background(backgroundColor, in: .capsule) diff --git a/Relay/Utilities/PillTextAttachment.swift b/Relay/Utilities/PillTextAttachment.swift index dbd007f0..dcc5212b 100644 --- a/Relay/Utilities/PillTextAttachment.swift +++ b/Relay/Utilities/PillTextAttachment.swift @@ -57,17 +57,12 @@ nonisolated final class PillTextAttachment: NSTextAttachment, @unchecked Sendabl super.init(data: nil, ofType: nil) self.attachmentCell = nil - let fontSize = font.pointSize - let pillSize = MainActor.assumeIsolated { - MentionPillView.measureSize( - displayName: displayName, - font: NSFont.systemFont(ofSize: fontSize) - ) - } - self.image = Self.renderPillImage( - userId: userId, displayName: displayName, size: pillSize, style: .compose + let rendered = Self.renderPill( + userId: userId, displayName: displayName, + fontSize: font.pointSize, style: .compose ) - self.bounds = CGRect(origin: .zero, size: pillSize) + self.image = rendered.image + self.bounds = CGRect(origin: .zero, size: rendered.size) } /// Creates a pill attachment for message rendering with a specific style. @@ -78,17 +73,12 @@ nonisolated final class PillTextAttachment: NSTextAttachment, @unchecked Sendabl super.init(data: nil, ofType: nil) self.attachmentCell = nil - let fontSize = font.pointSize - let pillSize = MainActor.assumeIsolated { - MentionPillView.measureSize( - displayName: displayName, - font: NSFont.systemFont(ofSize: fontSize) - ) - } - self.image = Self.renderPillImage( - userId: userId, displayName: displayName, size: pillSize, style: style + let rendered = Self.renderPill( + userId: userId, displayName: displayName, + fontSize: font.pointSize, style: style ) - self.bounds = CGRect(origin: .zero, size: pillSize) + self.image = rendered.image + self.bounds = CGRect(origin: .zero, size: rendered.size) } /// Creates a pill attachment for a keyword highlight (no `@` prefix, no link). @@ -99,19 +89,12 @@ nonisolated final class PillTextAttachment: NSTextAttachment, @unchecked Sendabl super.init(data: nil, ofType: nil) self.attachmentCell = nil - let fontSize = font.pointSize - let pillSize = MainActor.assumeIsolated { - MentionPillView.measureSize( - displayName: keyword, - font: NSFont.systemFont(ofSize: fontSize), - showAtPrefix: false - ) - } - self.image = Self.renderPillImage( - userId: "", displayName: keyword, size: pillSize, style: style, - showAtPrefix: false + let rendered = Self.renderPill( + userId: "", displayName: keyword, + fontSize: font.pointSize, style: style, showAtPrefix: false ) - self.bounds = CGRect(origin: .zero, size: pillSize) + self.image = rendered.image + self.bounds = CGRect(origin: .zero, size: rendered.size) } @available(*, unavailable) @@ -121,16 +104,19 @@ nonisolated final class PillTextAttachment: NSTextAttachment, @unchecked Sendabl // MARK: - Image Rendering - /// Renders the ``MentionPillView`` to a static `NSImage` at 2x resolution. + /// Renders the ``MentionPillView`` to an `NSImage` at 2x resolution and + /// returns it together with its natural point size. /// - /// Uses SwiftUI's `ImageRenderer` with an explicit scale of 2 so that pills - /// look sharp on Retina displays without relying on window backing scale. - /// The resulting `NSImage` has its logical size set to the 1x point size so - /// TextKit positions it correctly. - private static func renderPillImage( - userId: String, displayName: String, size: CGSize, style: MentionPillStyle, - showAtPrefix: Bool = true - ) -> NSImage { + /// The pill text is rendered at `fontSize`, and the caller uses the returned + /// natural size as the attachment bounds, so the bitmap is drawn 1:1 and is + /// never scaled. The capsule therefore stays sharp and undistorted at any + /// surrounding font size — including when the timeline text-zoom enlarges the + /// message font. The explicit `scale` of 2 keeps pills crisp on Retina + /// displays without relying on the window backing scale. + private static func renderPill( + userId: String, displayName: String, fontSize: CGFloat, + style: MentionPillStyle, showAtPrefix: Bool = true + ) -> (image: NSImage, size: CGSize) { MainActor.assumeIsolated { let tintColor = Color(stableColorFor: userId) let colorScheme: ColorScheme = @@ -138,16 +124,23 @@ nonisolated final class PillTextAttachment: NSTextAttachment, @unchecked Sendabl ? .dark : .light let pillView = MentionPillView( displayName: displayName, tintColor: tintColor, style: style, - showAtPrefix: showAtPrefix + showAtPrefix: showAtPrefix, fontSize: fontSize ) .environment(\.colorScheme, colorScheme) let renderer = ImageRenderer(content: pillView) renderer.scale = 2 - guard let cgImage = renderer.cgImage else { - return NSImage(size: size) + if let image = renderer.nsImage { + return (image, image.size) } - return NSImage(cgImage: cgImage, size: size) + // Rendering should never fail; fall back to the measured size so the + // mention still reserves inline space. + let size = MentionPillView.measureSize( + displayName: displayName, + font: NSFont.systemFont(ofSize: fontSize), + showAtPrefix: showAtPrefix + ) + return (NSImage(size: size), size) } } @@ -186,7 +179,15 @@ nonisolated final class PillTextAttachment: NSTextAttachment, @unchecked Sendabl /// visual midline (midpoint between ascender and descender). private static func paddedBounds(pillSize: CGSize, fontSize: CGFloat) -> CGRect { let font = NSFont.systemFont(ofSize: fontSize) - let paddedHeight = pillSize.height + verticalPadding + // Cap the attachment height to the font's line box (ascender − descender) + // so the pill can never overhang the line it sits on. Without the cap the + // padded pill (~18pt) is taller than the ~15.8pt line box, so a pill on + // the first line overhangs the ascender and its top is clipped by the + // bubble; every pill also reads as vertically tight. Centering the capped + // height on the font midline places the pill exactly within + // [descender, ascender]. + let lineHeight = font.ascender - font.descender + let paddedHeight = min(pillSize.height + verticalPadding, lineHeight) let midline = (font.ascender + font.descender) / 2 let y = midline - paddedHeight / 2 return CGRect( From f35842176f0e2cce990628f242c0d9c8fef7baab Mon Sep 17 00:00:00 2001 From: Lukas Frias Santos Date: Sun, 19 Jul 2026 11:30:40 +0200 Subject: [PATCH 03/11] Size link-preview cards to their image aspect ratio, with a bounded card cache and favicons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Link cards were a fixed 260x260 square with a scaledToFill image, cropping wide Open-Graph banners (including text baked into the image). Show the image edge-to-edge at its native aspect ratio instead. Because the card height is now variable and the detached row-height measurement host never runs the async image load, the resolved aspect ratio is published to a synchronously readable cache so the measurement host and live cell compute the same card height; the card triggers a one-time remeasureRow when its image resolves. The title reserves two lines so height stays deterministic regardless of load state. Replace the unbounded [URL: CGFloat] aspect cache and its nil/0/>0 sentinel with a bounded LRU (ParseCache) of an explicit LinkPreviewCard enum (unavailable / banner(aspect) / compact). Links without an Open-Graph banner show the site favicon (scaled to fit, capped at its native size so small icons aren't upscaled) instead of a globe; the globe remains only when no icon is available at all. Re-measure the row on any height-changing card transition — first resolution, a compact→banner upgrade, or a changed banner aspect — while a same-height re-resolve (globe→favicon) skips it. ParseCache.get promoted entries to most-recently-used on every read — an O(n) firstIndex plus array mutation under a lock — called from SwiftUI body evaluation for every visible card and every detached measurement. Replace it with an O(1) non-mutating peek; recency is still maintained by set() on resolution, which suffices since the card cache writes once per URL. Add ParseCache.removeAll() to drop every entry when a global input changes, and rename set(_:_:) to set(_:forKey:) to match value(forKey:). Add headless tests reconstructing the exact MessageTextContent TextKit layout to assert, without a homeserver, that message content fits the measured row height: pill lines don't grow beyond plain-text lines, pill attachment bounds stay within the font line box, text height is measured at the floored render width across a sweep of fractional widths, link-card height is deterministic from the aspect cache regardless of image-load state, a pill's rendered bitmap scales with the font size, and a wrapping row grows taller as the width shrinks. Co-Authored-By: Claude Opus 4.8 --- Relay/Utilities/ParseCache.swift | 37 ++ Relay/Views/Media/LinkPreviewView.swift | 222 +++++++++--- Relay/Views/Timeline/TimelineActions.swift | 8 + Relay/Views/Timeline/TimelineRowView.swift | 4 + .../TimelineTableViewRepresentable.swift | 7 + .../TimelineHeightMeasurementTests.swift | 317 ++++++++++++++++++ 6 files changed, 545 insertions(+), 50 deletions(-) create mode 100644 RelayTests/TimelineHeightMeasurementTests.swift diff --git a/Relay/Utilities/ParseCache.swift b/Relay/Utilities/ParseCache.swift index b77bf5d4..219ff659 100644 --- a/Relay/Utilities/ParseCache.swift +++ b/Relay/Utilities/ParseCache.swift @@ -54,4 +54,41 @@ final class ParseCache: @unchecked Sendable { return result } + + /// Returns the cached value for `key` without computing or promoting it — + /// an O(1) read safe to call from hot paths such as SwiftUI `body`. Recency + /// is updated only by ``set(_:_:)``, which suffices for caches that write on + /// resolution. Returns `nil` on a miss. + func peek(_ key: Key) -> Value? { + lock.lock() + defer { lock.unlock() } + return storage[key] + } + + /// Removes every cached entry. Used when a global input the cached values + /// depend on (e.g. the message text-zoom level) changes and every previously + /// computed value is stale. + func removeAll() { + lock.lock() + defer { lock.unlock() } + storage.removeAll() + order.removeAll() + } + + /// Stores `value` for `key`, evicting the least-recently-used entry when the + /// cache exceeds its capacity. + func set(_ value: Value, forKey key: Key) { + lock.lock() + defer { lock.unlock() } + if storage[key] == nil { + order.append(key) + } else if let idx = order.firstIndex(of: key) { + order.append(order.remove(at: idx)) + } + storage[key] = value + if order.count > capacity { + let evicted = order.removeFirst() + storage.removeValue(forKey: evicted) + } + } } diff --git a/Relay/Views/Media/LinkPreviewView.swift b/Relay/Views/Media/LinkPreviewView.swift index 9eb08948..043dfd68 100644 --- a/Relay/Views/Media/LinkPreviewView.swift +++ b/Relay/Views/Media/LinkPreviewView.swift @@ -61,76 +61,168 @@ actor LinkMetadataCache { } } -// MARK: - LinkPreviewView - -/// The fixed side length of the link preview card in points. -private let previewSize: CGFloat = 260 +// MARK: - Card Cache -/// Displays a fixed-size link preview card for a URL. +/// The resolved presentation of a link preview, cached per URL. /// -/// The card has a constant size (`260×260` pt) so that loading metadata never -/// changes the row height. This eliminates the height-cache invalidation and -/// re-measurement that previously caused visible jumps during scrolling. +/// Modelled as a type rather than a sentinel number so the loading, hidden, and +/// resolved states are explicit. A cache *miss* (no entry) means "not resolved +/// yet" — the card shows a placeholder. +enum LinkPreviewCard: Sendable, Equatable { + /// The link has no usable preview; the card is hidden entirely. + case unavailable + /// A full-bleed Open-Graph image with the given aspect ratio (width ÷ height). + case banner(aspect: CGFloat) + /// A compact card (favicon or globe fallback) with a fixed image height. + case compact +} + +// MARK: - LinkPreviewView + +/// Displays a link preview card sized to its Open-Graph image's aspect ratio. /// -/// Metadata is fetched asynchronously via `LPMetadataProvider` and cached -/// globally so that scrolling through the timeline doesn't re-fetch. +/// The card width is fixed; the image height follows the loaded image's aspect +/// ratio (clamped to a sane range), so wide banners are shown edge-to-edge +/// without cropping. Links without a banner image fall back to a compact card +/// showing the site's favicon. Because the height is variable, the card triggers +/// a one-time row re-measure (via ``TimelineActions/remeasureRow``) when its +/// presentation resolves. Both the live cell and the timeline's detached +/// height-measurement host read the resolved card from ``cardCache`` so +/// measured and rendered heights match. struct LinkPreviewView: View { let url: URL let isOutgoing: Bool - /// The timeline message ID that contains this preview. + /// The timeline message ID that contains this preview. Used to request a + /// row re-measure once the card's presentation is known. let messageID: String + @Environment(\.timelineActions) private var actions + @State private var title: String? @State private var image: NSImage? - @State private var didLoad = false - @State private var didFail = false + + /// This instance's resolved card, mirroring ``cardCache``. + /// `nil` before resolution (placeholder / loading). + @State private var card: LinkPreviewCard? + + /// A bounded cache of resolved link-preview presentations, keyed by URL. + /// + /// This is the linchpin that lets link-preview cards be **variable height** + /// (sized to their image, iMessage-style) without the timeline clipping them. + /// Row heights in ``TimelineTableViewController`` are measured by a *detached* + /// `NSHostingController` whose SwiftUI `.task` never runs, so it cannot observe + /// a per-view `@State` image loaded asynchronously. By publishing the resolved + /// card here, both the live cell **and** the detached measurement host compute + /// the identical card height synchronously at body-evaluation time — the + /// measurement host renders a placeholder glyph at the *same* frame size the + /// live cell renders the real image at, so measured and rendered heights agree. + /// + /// Backed by an LRU (``ParseCache``) so a long session browsing many links + /// does not grow the cache without bound. + static let cardCache = ParseCache(capacity: 256) + + /// Fixed card width in points. Only the image height varies. + private static let cardWidth: CGFloat = 260 + /// Image height shown before the card resolves. + private static let placeholderImageHeight: CGFloat = 150 + /// Clamp so extreme aspect ratios don't produce absurdly short/tall cards. + private static let minImageHeight: CGFloat = 90 + private static let maxImageHeight: CGFloat = 340 + /// Compact image height for links with no Open-Graph banner (favicon/globe). + private static let iconImageHeight: CGFloat = 72 + /// Maximum favicon edge within the compact card. Small favicons are drawn at + /// their native size rather than upscaled to this. + private static let faviconMaxSize: CGFloat = 40 + + /// The card resolved for this URL: this instance's state, falling back to + /// the shared cache so a freshly-built view (including the detached + /// measurement host) sizes correctly without waiting for its `.task`. + private var resolvedCard: LinkPreviewCard? { + card ?? Self.cardCache.peek(url) + } + + /// Whether the link has no usable preview and the card should be hidden. + private var isHidden: Bool { resolvedCard == .unavailable } + + /// Whether this is a compact (favicon/globe) card rather than a banner. + private var isCompact: Bool { resolvedCard == .compact } + + /// The image area height, derived from the resolved card. + private var imageHeight: CGFloat { + switch resolvedCard { + case .banner(let aspect) where aspect > 0: + return min(max(Self.cardWidth / aspect, Self.minImageHeight), Self.maxImageHeight) + case .compact: + return Self.iconImageHeight + default: + return Self.placeholderImageHeight + } + } var body: some View { - cardContent - .frame(width: previewSize, height: previewSize) - .clipShape(.rect(cornerRadius: 12)) - .overlay( - RoundedRectangle(cornerRadius: 12) - .strokeBorder(.quaternary, lineWidth: 0.5) - ) - .contentShape(.rect(cornerRadius: 12)) - .onTapGesture { - NSWorkspace.shared.open(url) - } - .task(id: url) { - await loadMetadata() + Group { + if isHidden { + EmptyView() + } else { + cardBody } + } + .task(id: url) { + // Seed from the cache so recycled/measurement instances size + // correctly immediately, then (re)load for display. + card = Self.cardCache.peek(url) + await loadMetadata() + } } - @ViewBuilder - private var cardContent: some View { - if didFail { - EmptyView() - } else { - VStack(spacing: 0) { - // Image area — fills the top portion. - imageArea - .frame(maxWidth: .infinity, maxHeight: .infinity) - .clipped() + private var cardBody: some View { + VStack(spacing: 0) { + imageArea + .frame(width: Self.cardWidth, height: imageHeight) + .clipped() - // Text area — fixed at the bottom. - textArea - } - .background(.fill.tertiary) + textArea + } + .frame(width: Self.cardWidth) + .background(.fill.tertiary) + .clipShape(.rect(cornerRadius: 12)) + .overlay( + RoundedRectangle(cornerRadius: 12) + .strokeBorder(.quaternary, lineWidth: 0.5) + ) + .contentShape(.rect(cornerRadius: 12)) + .onTapGesture { + NSWorkspace.shared.open(url) } } @ViewBuilder private var imageArea: some View { if let image { - Image(nsImage: image) - .resizable() - .scaledToFill() - } else if !didLoad { + if isCompact { + // Favicon: fit within the compact area rather than filling, so a + // small square icon isn't cropped or stretched. + Image(nsImage: image) + .resizable() + .scaledToFit() + // Never upscale a small favicon (e.g. 16×16): cap the fit + // frame at the icon's native size and at faviconMaxSize. + .frame( + maxWidth: min(image.size.width, Self.faviconMaxSize), + maxHeight: min(image.size.height, Self.faviconMaxSize) + ) + .frame(maxWidth: .infinity, maxHeight: .infinity) + } else { + Image(nsImage: image) + .resizable() + .scaledToFill() + } + } else if resolvedCard == nil { ProgressView() .controlSize(.small) } else { + // Compact card with no favicon available. Image(systemName: "globe") .font(.largeTitle) .foregroundStyle(.secondary) @@ -139,10 +231,14 @@ struct LinkPreviewView: View { private var textArea: some View { VStack(alignment: .leading, spacing: 2) { + // Reserve two lines regardless of the actual title length so the + // text area height is deterministic — the detached measurement + // host (which has no title yet) reserves the same space the live + // cell uses for a wrapped two-line title. Text(title ?? url.host() ?? url.absoluteString) .font(.callout) .bold() - .lineLimit(2) + .lineLimit(2, reservesSpace: true) .truncationMode(.tail) Text(url.host() ?? url.absoluteString) @@ -158,18 +254,44 @@ struct LinkPreviewView: View { private func loadMetadata() async { guard let metadata = await LinkMetadataCache.shared.metadata(for: url) else { - didFail = true + resolve(.unavailable) return } title = metadata.title - // Extract the preview image from the metadata provider. - if let imageProvider = metadata.imageProvider ?? metadata.iconProvider { - image = await loadImage(from: imageProvider) + // A real Open-Graph image drives a full-bleed banner card. Otherwise fall + // back to a compact card showing the favicon (or a globe if none). + if let imageProvider = metadata.imageProvider, + let loaded = await loadImage(from: imageProvider), + loaded.size.width > 0, loaded.size.height > 0 { + image = loaded + resolve(.banner(aspect: loaded.size.width / loaded.size.height)) + } else if let iconProvider = metadata.iconProvider, + let icon = await loadImage(from: iconProvider), + icon.size.width > 0, icon.size.height > 0 { + image = icon + resolve(.compact) + } else { + image = nil + resolve(.compact) } + } - didLoad = true + /// Publishes the resolved card to the shared cache and this instance, and + /// re-measures the row when the resolved *height* can change. + private func resolve(_ resolved: LinkPreviewCard) { + let previous = Self.cardCache.peek(url) + Self.cardCache.set(resolved, forKey: url) + card = resolved + // Re-measure on any height-changing transition: the first resolution, a + // compact fallback upgrading to a banner (e.g. after a transient + // image-load failure), or a changed banner aspect. Re-resolving to the + // same card — including a globe→favicon swap, which keeps the compact + // height — needs no re-measure. + if previous != resolved { + actions.remeasureRow?(messageID) + } } private func loadImage(from provider: NSItemProvider) async -> NSImage? { diff --git a/Relay/Views/Timeline/TimelineActions.swift b/Relay/Views/Timeline/TimelineActions.swift index cd8a028e..1e1c3993 100644 --- a/Relay/Views/Timeline/TimelineActions.swift +++ b/Relay/Views/Timeline/TimelineActions.swift @@ -112,6 +112,14 @@ final class TimelineActions: Equatable { /// has expanded. Keyed by the first message's ID in each collapsed group. let expandedGroups = ExpandedGroupsState() + /// Requests that the table-backed renderer re-measure a specific row by + /// message ID. Used when a row's content height changes asynchronously + /// without any change to the underlying message data — e.g. a link-preview + /// card resizing to its image's aspect ratio once the Open-Graph image + /// loads. Without this, the height cache keeps the pre-load placeholder + /// height and clips (or leaves a gap under) the resized card. + var remeasureRow: ((String) -> Void)? + /// Creates a ``TimelineActions`` with default (no-op) callbacks. init(currentUserID: String? = nil) { self.currentUserID = currentUserID diff --git a/Relay/Views/Timeline/TimelineRowView.swift b/Relay/Views/Timeline/TimelineRowView.swift index f33d2f92..24641a96 100644 --- a/Relay/Views/Timeline/TimelineRowView.swift +++ b/Relay/Views/Timeline/TimelineRowView.swift @@ -183,6 +183,10 @@ struct TimelineRowView: View, Equatable { if showURLPreviews, message.kind == .text, let url = URLPreviewExtractor.firstPreviewURL(in: message.body) { LinkPreviewView(url: url, isOutgoing: message.isOutgoing, messageID: message.id) + // Bind identity to the URL so a recycled cell reused for a + // different-URL message gets fresh state instead of bleeding + // the previous URL's cached aspect ratio. + .id(url) .padding(.leading, message.isOutgoing ? 0 : 34) .frame(maxWidth: .infinity, alignment: message.isOutgoing ? .trailing : .leading) } diff --git a/Relay/Views/Timeline/TimelineTableViewRepresentable.swift b/Relay/Views/Timeline/TimelineTableViewRepresentable.swift index 81ae7e4b..d3f15dca 100644 --- a/Relay/Views/Timeline/TimelineTableViewRepresentable.swift +++ b/Relay/Views/Timeline/TimelineTableViewRepresentable.swift @@ -78,6 +78,13 @@ struct TimelineTableViewRepresentable: NSViewControllerRepresentable { vc?.remeasureRow(forMessageID: groupID) } + // A link-preview card resizes to its image's aspect ratio once the + // Open-Graph image loads; the row must re-measure so the height cache + // picks up the new card height instead of the pre-load placeholder. + actions.remeasureRow = { [weak vc] messageID in + vc?.remeasureRow(forMessageID: messageID) + } + vc.callbacks = .init( onNearBottomChanged: onNearBottomChanged, onPaginateBackward: onPaginateBackward, diff --git a/RelayTests/TimelineHeightMeasurementTests.swift b/RelayTests/TimelineHeightMeasurementTests.swift new file mode 100644 index 00000000..7e059cd6 --- /dev/null +++ b/RelayTests/TimelineHeightMeasurementTests.swift @@ -0,0 +1,317 @@ +// Copyright 2026 Link Dupont +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import AppKit +import SwiftUI +import Testing + +@testable import Relay + +// MARK: - Timeline Height / Clipping Measurement Tests +// +// These are headless regression guards for timeline message clipping — the +// class of bug where a message row is allotted slightly less height than its +// content actually draws, so the bottom/top gets cut off. They need no +// homeserver: they reconstruct the exact TextKit layout `MessageTextContent` +// uses and assert the content fits within the height the timeline measures. +// +// They cover three independent clipping causes: +// 1. Fractional container-width wrapping (MessageTextView.sizeThatFits). +// 2. Mention-pill vertical overhang (PillTextAttachment.paddedBounds). +// 3. Link-preview card height determinism (LinkPreviewView + aspect cache). + +@MainActor +struct TimelineHeightMeasurementTests { + + // MARK: - TextKit Layout Harness + + /// A faithful replica of the `NSTextView`/`NSLayoutManager`/`NSTextContainer` + /// configuration `MessageTextView.makeNSView` builds, so height/geometry + /// measured here matches what the app renders. + private struct Layout { + let layoutManager: NSLayoutManager + let container: NSTextContainer + let storage: NSTextStorage + /// The ceil'd used-rect height — what `MessageTextView.sizeThatFits` + /// reports and the timeline uses as the row's text height. + let usedHeight: CGFloat + } + + /// Lays out `attributed` at the given container width using the app's exact + /// TextKit settings (`usesFontLeading = false`, `lineFragmentPadding = 0`). + /// The width is floored to match the app's render-time container width + /// (`MessageTextView.sizeThatFits` measures the wrapped case at + /// `pw.rounded(.down)`). + private func layout(_ attributed: NSAttributedString, width: CGFloat) -> Layout { + let storage = NSTextStorage(attributedString: attributed) + let layoutManager = NSLayoutManager() + layoutManager.usesFontLeading = false + storage.addLayoutManager(layoutManager) + let container = NSTextContainer( + size: NSSize(width: width.rounded(.down), height: .greatestFiniteMagnitude) + ) + container.widthTracksTextView = false + container.lineFragmentPadding = 0 + layoutManager.addTextContainer(container) + layoutManager.ensureLayout(for: container) + let usedHeight = ceil(layoutManager.usedRect(for: container).height) + return Layout( + layoutManager: layoutManager, container: container, + storage: storage, usedHeight: usedHeight + ) + } + + /// The base font used for message text and pill sizing. + private var baseFont: NSFont { NSFont.systemFont(ofSize: NSFont.systemFontSize) } + + /// Builds a resolved (pill-substituted) attributed string exactly as + /// `MessageBubbleContent` → `MessageTextView` would, for an incoming bubble. + private func resolvedIncoming(_ source: NSAttributedString) -> NSAttributedString { + MessageTextView.applyColorOverrides( + source, foreground: .labelColor, linkColor: .linkColor, + pillStyle: .messageDefault + ) + } + + /// A source attributed string with a `matrix.to` user mention over `mentionText` + /// at the very start, so the resulting pill lands on the first (and, when + /// short, the last) line. + private func mentionSource( + mentionText: String = "Sample User", + trailing: String = " sent a short message to the room" + ) -> NSAttributedString { + let full = mentionText + trailing + let src = NSMutableAttributedString( + string: full, + attributes: [.font: baseFont] + ) + src.addAttribute( + .link, + value: URL(string: "https://matrix.to/#/@sample:matrix.org")!, + range: NSRange(location: 0, length: (mentionText as NSString).length) + ) + return src + } + + // MARK: - 1. Mention-pill line height (must not grow / poke above text) + + /// A line containing a mention pill must be no taller than the same line of + /// plain text. `PillTextAttachment.paddedBounds` previously sized the pill to + /// ~18pt — taller than the ~15.3pt font line box — which grew the pill's line + /// fragment ~2pt, stretched the pill image, and pushed the pill's top flush + /// against the bubble's inner top edge (reading as a clipped, tight top). + /// Capping the attachment to the line box keeps a pill line the same height + /// as a normal text line. (TextKit "grow-and-shift" keeps the pill inside the + /// used rect either way, so line growth — not a draw-overhang — is the defect.) + @Test + func mentionPillDoesNotGrowLineHeight() { + // The same short message on one line (600pt-wide container) as plain + // text and with a leading mention pill. + let plain = layout( + NSAttributedString(string: "Sample User wrote something", attributes: [.font: baseFont]), + width: 600 + ) + + let resolved = resolvedIncoming(mentionSource(trailing: " wrote something")) + let withPill = layout(resolved, width: 600) + + var foundPill = false + resolved.enumerateAttribute(.attachment, in: NSRange(location: 0, length: resolved.length)) { value, _, _ in + if value is PillTextAttachment { foundPill = true } + } + #expect(foundPill, "Expected the mention to be substituted with a PillTextAttachment") + + #expect( + withPill.usedHeight <= plain.usedHeight + 0.5, + "Pill line height \(withPill.usedHeight)pt exceeds plain-text line height \(plain.usedHeight)pt — the pill grows the line and pokes above surrounding text." + ) + } + + /// Direct unit invariant: a pill's attachment bounds must fit inside the + /// font line box (top ≤ ascender, bottom ≥ descender) so it can never + /// overhang whatever line it lands on. + @Test + func pillAttachmentBoundsFitWithinFontLineBox() { + let pill = PillTextAttachment( + userId: "@sample:matrix.org", displayName: "Sample User", + font: baseFont, style: .messageDefault + ) + let dummyContainer = NSTextContainer( + size: NSSize(width: 320, height: CGFloat.greatestFiniteMagnitude) + ) + let bounds = pill.attachmentBounds( + for: dummyContainer, proposedLineFragment: .zero, + glyphPosition: .zero, characterIndex: 0 + ) + // Baseline-relative, +y up: top = bounds.maxY, bottom = bounds.minY. + #expect( + bounds.maxY <= baseFont.ascender + 0.5, + "Pill top \(bounds.maxY) exceeds font ascender \(baseFont.ascender) — overhangs the line box." + ) + #expect( + bounds.minY >= baseFont.descender - 0.5, + "Pill bottom \(bounds.minY) is below font descender \(baseFont.descender) — overhangs the line box." + ) + } + + /// A pill's rendered glyphs must scale with the surrounding font size. If the + /// pill view is rendered at a fixed text style while its attachment bounds are + /// sized for a larger font, TextKit upscales the small bitmap into the large + /// bounds and the capsule reads as stretched/blurry — the defect that surfaces + /// once the timeline text-zoom enlarges the message font. + @Test + func mentionPillContentScalesWithFontSize() { + func renderedPixelHeight(fontSize: CGFloat) -> Int { + let view = MentionPillView( + displayName: "Sample User", style: .messageDefault, fontSize: fontSize + ) + let renderer = ImageRenderer(content: view) + renderer.scale = 2 + return renderer.cgImage?.height ?? 0 + } + let small = renderedPixelHeight(fontSize: NSFont.systemFontSize) + let large = renderedPixelHeight(fontSize: NSFont.systemFontSize * 2) + #expect(small > 0 && large > 0, "Pill failed to render.") + #expect( + Double(large) > Double(small) * 1.6, + "Pill render height barely grew (\(small)px → \(large)px): the glyphs are not drawn at the target font size, so the bitmap is upscaled into the font-sized bounds and reads as stretched." + ) + } + + // MARK: - 2. Fractional-width wrapping determinism + + /// Text height must be measured at the same integral width the container is + /// rendered at. Measuring at a fractional `pw` while the live container ends + /// up at `floor(pw)` makes a boundary line wrap one extra line on screen that + /// the measured height never accounted for — clipping the last line. + /// + /// Here we assert the measured height (at `floor(w)`) fully contains the text + /// laid out at that same render width across a sweep of fractional widths. + @Test + func textHeightMeasuredAtRenderWidthAcrossFractionalWidths() { + let body = "the quick brown fox jumps over the lazy dog again " + + "and again to make this message wrap onto several lines" + let attributed = NSAttributedString(matrixMarkdown: body) + + // Sweep sub-point widths around a plausible bubble content width. + for tenths in 0..<60 { + let width = 300.0 + CGFloat(tenths) / 10.0 + let laid = layout(attributed, width: width) + // Re-layout at exactly the floored render width and confirm the + // reported (floored) used height contains it — i.e. no extra wrap + // beyond what was measured. + let renderWidth = width.rounded(.down) + laid.container.size = NSSize(width: renderWidth, height: .greatestFiniteMagnitude) + laid.layoutManager.ensureLayout(for: laid.container) + let renderHeight = ceil(laid.layoutManager.usedRect(for: laid.container).height) + #expect( + laid.usedHeight >= renderHeight, + "At width \(width): measured \(laid.usedHeight)pt < rendered \(renderHeight)pt — last line clips." + ) + } + } + + // MARK: - 3. Re-measurement on resize (narrower width must grow height) + + /// After a window resize the timeline re-measures visible rows at the new + /// width. A message that wraps must report a *taller* height at a narrower + /// width — the regression guard for rows keeping their old (too-short) + /// height after a resize and clipping the re-wrapped text. + @Test + func wrappingMessageHeightGrowsAsWidthShrinks() { + let body = "the quick brown fox jumps over the lazy dog again and again " + + "so that this message must wrap onto several lines when it is narrow" + let attributed = NSAttributedString(matrixMarkdown: body) + let wide = layout(attributed, width: 520).usedHeight + let narrow = layout(attributed, width: 240).usedHeight + #expect( + narrow > wide, + "Height at 240pt (\(narrow)pt) is not greater than at 520pt (\(wide)pt); a resize to a narrower width would keep the old, too-short height and clip the re-wrapped text." + ) + } + + // MARK: - 4. Link-preview card height determinism + + /// A variable-height link card must derive its height synchronously from the + /// shared card cache, so the detached measurement host (whose async image + /// load never runs) computes the same card height the live cell renders. + @Test + func linkCardHeightIsDeterministicFromCache() { + let url = URL(string: "https://example.com/deterministic-\(UUID().uuidString)")! + + // Unresolved: placeholder height. + let placeholderHeight = measuredHeight( + LinkPreviewView(url: url, isOutgoing: false, messageID: "m1"), width: 400 + ) + + // Resolved wide banner: stable, independent of message/instance. + LinkPreviewView.cardCache.set(.banner(aspect: 2.0), forKey: url) + let resolvedA = measuredHeight( + LinkPreviewView(url: url, isOutgoing: false, messageID: "m2"), width: 400 + ) + let resolvedB = measuredHeight( + LinkPreviewView(url: url, isOutgoing: true, messageID: "m3"), width: 400 + ) + #expect(resolvedA == resolvedB, "Card height must not depend on the message/instance.") + #expect( + resolvedA != placeholderHeight, + "Card height must reflect the resolved aspect ratio, not the pre-load placeholder." + ) + + // Portrait aspect yields a taller card — height is genuinely aspect-driven. + let tallURL = URL(string: "https://example.com/tall-\(UUID().uuidString)")! + LinkPreviewView.cardCache.set(.banner(aspect: 0.5), forKey: tallURL) + let tall = measuredHeight( + LinkPreviewView(url: tallURL, isOutgoing: false, messageID: "m4"), width: 400 + ) + #expect(tall > resolvedA, "A portrait image should yield a taller card than a wide one.") + } + + /// An unavailable link resolves to a hidden (zero-height) card. + @Test + func unavailableLinkCardIsHidden() { + let url = URL(string: "https://example.com/gone-\(UUID().uuidString)")! + LinkPreviewView.cardCache.set(.unavailable, forKey: url) + let height = measuredHeight( + LinkPreviewView(url: url, isOutgoing: false, messageID: "m1"), width: 400 + ) + #expect(height <= 1, "An unavailable link preview must collapse to zero height, got \(height)pt.") + } + + /// A compact (favicon/globe) card has a fixed, deterministic height distinct + /// from a hidden card. + @Test + func compactLinkCardHeightIsFixed() { + let url = URL(string: "https://example.com/compact-\(UUID().uuidString)")! + LinkPreviewView.cardCache.set(.compact, forKey: url) + let a = measuredHeight( + LinkPreviewView(url: url, isOutgoing: false, messageID: "m1"), width: 400 + ) + let b = measuredHeight( + LinkPreviewView(url: url, isOutgoing: true, messageID: "m2"), width: 400 + ) + #expect(a == b, "Compact card height must be deterministic.") + #expect(a > 1, "Compact card must have a non-zero height.") + } + + // MARK: - Hosting Measurement Helper + + /// The height a detached `NSHostingController` reports for `view` at `width` + /// — the same measurement path `TimelineTableViewController` uses for row + /// heights (its `measurementHost`). + private func measuredHeight(_ view: some View, width: CGFloat) -> CGFloat { + let host = NSHostingController(rootView: view) + return host.sizeThatFits(in: CGSize(width: width, height: .greatestFiniteMagnitude)).height + } +} From f99743986c46c21f46068e1982f7bb6d7dba6d0d Mon Sep 17 00:00:00 2001 From: Lukas Frias Santos Date: Sun, 19 Jul 2026 11:31:00 +0200 Subject: [PATCH 04/11] Debounce timeline row re-measurement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Coalesce bursts of remeasureRow(forMessageID:) calls — several link-preview cards resolving their images at once, or a collapsed-group toggle — into a single noteHeightOfRows pass on a 16ms trailing window, preserving scroll position, instead of one height pass per row change. Anchor a 120ms max-wait to the first queued request so a continuous stream of remeasureRow calls can't keep resetting the trailing debounce and starve the flush. flushPendingRemeasures cancels both timers up front. Co-Authored-By: Claude Opus 4.8 --- Relay/Views/Timeline/TimelineTableView.swift | 84 +++++++++++++++----- 1 file changed, 64 insertions(+), 20 deletions(-) diff --git a/Relay/Views/Timeline/TimelineTableView.swift b/Relay/Views/Timeline/TimelineTableView.swift index 386aed81..c5db14f7 100644 --- a/Relay/Views/Timeline/TimelineTableView.swift +++ b/Relay/Views/Timeline/TimelineTableView.swift @@ -315,6 +315,8 @@ final class TimelineTableViewController: NSViewController { NotificationCenter.default.removeObserver(self) MainActor.assumeIsolated { paginateTask?.cancel() + remeasureDebounceTask?.cancel() + remeasureMaxWaitTask?.cancel() } } @@ -801,30 +803,72 @@ final class TimelineTableViewController: NSViewController { /// cached height for this row and note its new height; `heightOfRow`'s /// measurement host rebuilds the row reading the now-updated expansion /// state, so it returns the full expanded (or collapsed) height. - func remeasureRow(forMessageID id: String) { - guard let messageIndex = rows.firstIndex(where: { $0.id == id }) else { return } - let rowIndex = messageIndex + /// Message IDs awaiting a debounced height re-measure. + private var pendingRemeasureIDs: Set = [] + /// Coalesces a burst of ``remeasureRow(forMessageID:)`` calls into one pass. + private var remeasureDebounceTask: Task? + /// Guarantees a flush even under a continuous stream of requests, so the + /// trailing debounce can't be reset indefinitely. + private var remeasureMaxWaitTask: Task? - // Defer so the live hosting cell has settled its SwiftUI re-render - // before we note the new height (matches the resize handler). - Task { @MainActor [weak self] in - guard let self else { return } - self.invalidateHeight(for: id) - let scrollBefore = self.scrollView.contentView.bounds.origin - NSAnimationContext.runAnimationGroup { context in - context.duration = 0 - context.allowsImplicitAnimation = false - self.tableView.noteHeightOfRows(withIndexesChanged: IndexSet(integer: rowIndex)) + func remeasureRow(forMessageID id: String) { + let wasEmpty = pendingRemeasureIDs.isEmpty + pendingRemeasureIDs.insert(id) + // Trailing debounce: defer so the live hosting cell settles its SwiftUI + // re-render, and so several rows changing in the same window (e.g. + // multiple link-preview cards resolving at once) collapse into a single + // height pass instead of one per row. + remeasureDebounceTask?.cancel() + remeasureDebounceTask = Task { @MainActor [weak self] in + try? await Task.sleep(for: .milliseconds(16)) + guard let self, !Task.isCancelled else { return } + self.flushPendingRemeasures() + } + // Max-wait, anchored to the first queued request: a continuous stream of + // calls can't keep resetting the trailing timer past this bound. + if wasEmpty { + remeasureMaxWaitTask = Task { @MainActor [weak self] in + try? await Task.sleep(for: .milliseconds(120)) + guard let self, !Task.isCancelled else { return } + self.flushPendingRemeasures() } - // Preserve the scroll position; growing a row above the viewport - // would otherwise shift the visible content. - if self.isNearBottom { - self.scrollToBottom(animated: false) - } else if abs(scrollBefore.y - self.scrollView.contentView.bounds.origin.y) > 0.5 { - self.scrollView.contentView.scroll(to: scrollBefore) - self.scrollView.reflectScrolledClipView(self.scrollView.contentView) + } + } + + /// Re-measures every row queued since the last flush in a single + /// `noteHeightOfRows` pass, preserving scroll position. + private func flushPendingRemeasures() { + remeasureDebounceTask?.cancel() + remeasureDebounceTask = nil + remeasureMaxWaitTask?.cancel() + remeasureMaxWaitTask = nil + + let ids = pendingRemeasureIDs + pendingRemeasureIDs.removeAll() + + var indices = IndexSet() + for id in ids { + invalidateHeight(for: id) + if let idx = rows.firstIndex(where: { $0.id == id }) { + indices.insert(idx) } } + guard !indices.isEmpty else { return } + + let scrollBefore = scrollView.contentView.bounds.origin + NSAnimationContext.runAnimationGroup { context in + context.duration = 0 + context.allowsImplicitAnimation = false + tableView.noteHeightOfRows(withIndexesChanged: indices) + } + // Preserve the scroll position; growing a row above the viewport would + // otherwise shift the visible content. + if isNearBottom { + scrollToBottom(animated: false) + } else if abs(scrollBefore.y - scrollView.contentView.bounds.origin.y) > 0.5 { + scrollView.contentView.scroll(to: scrollBefore) + scrollView.reflectScrolledClipView(scrollView.contentView) + } } // MARK: - Resize Handling From 60dfe3a78fb21fc0aaac7b144b646dbcd8ceb66e Mon Sep 17 00:00:00 2001 From: Lukas Frias Santos Date: Sun, 19 Jul 2026 11:31:38 +0200 Subject: [PATCH 05/11] Add timeline and compose text zoom to the View menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add Increase Text Size (⌘+), Decrease Text Size (⌘−), and Reset Text Size (⌥⌘0). Message bodies, mention pills, emote name prefixes, and the compose field derive their size from MessageTextScale, a persisted zoom factor, instead of NSFont.systemFontSize directly. Changing the scale clears the parse caches and re-renders and re-measures every row; the compose field re-applies its font in step. Also restore the remeasureRow(forMessageID:) doc comment that a prior edit stranded above pendingRemeasureIDs, and refresh it to cover both triggers and the debounce. Co-Authored-By: Claude Opus 4.8 Co-Authored-By: Claude Sonnet 5 --- Relay/RelayApp.swift | 30 +++++++ Relay/Utilities/MatrixHTMLParser.swift | 16 ++-- Relay/Utilities/MessageTextScale.swift | 84 +++++++++++++++++++ Relay/Views/Compose/ComposeTextView.swift | 38 ++++++++- .../Message/MessageAttributeResolver.swift | 2 +- .../Views/Message/MessageBubbleContent.swift | 2 +- Relay/Views/Timeline/TimelineTableView.swift | 49 +++++++++-- 7 files changed, 198 insertions(+), 23 deletions(-) create mode 100644 Relay/Utilities/MessageTextScale.swift diff --git a/Relay/RelayApp.swift b/Relay/RelayApp.swift index 04bf4ff4..8a82d603 100644 --- a/Relay/RelayApp.swift +++ b/Relay/RelayApp.swift @@ -58,6 +58,7 @@ struct RelayApp: App { QuickSwitchCommand(appActions: appActions) SidebarCommands() InspectorCommands() + TextSizeCommands() CommandGroup(before: .appTermination) { Button("Clear Cache…") { showClearCacheConfirmation = true @@ -449,6 +450,35 @@ struct QuickSwitchCommand: Commands { } } +// MARK: - Text Size Commands + +/// Adds message text-zoom items to the View menu: Increase Text Size (⌘+), +/// Reset Text Size (⌥⌘0), and Decrease Text Size (⌘−). +/// +/// Each adjusts ``MessageTextScale``, which rescales the conversation text, +/// mention pills, and the compose field together. +struct TextSizeCommands: Commands { + var body: some Commands { + CommandGroup(after: .toolbar) { + Divider() + Button("Increase Text Size") { + MessageTextScale.increase() + } + .keyboardShortcut("+", modifiers: .command) + + Button("Reset Text Size") { + MessageTextScale.reset() + } + .keyboardShortcut("0", modifiers: [.option, .command]) + + Button("Decrease Text Size") { + MessageTextScale.decrease() + } + .keyboardShortcut("-", modifiers: .command) + } + } +} + // MARK: - Notification Delegate /// Handles notification presentation and user interactions for local notifications. diff --git a/Relay/Utilities/MatrixHTMLParser.swift b/Relay/Utilities/MatrixHTMLParser.swift index 5746975f..a4b0c2cc 100644 --- a/Relay/Utilities/MatrixHTMLParser.swift +++ b/Relay/Utilities/MatrixHTMLParser.swift @@ -110,7 +110,7 @@ extension NSAttributedString { // 4. Bridge to NSAttributedString and resolve InlinePresentationIntent // into concrete AppKit fonts and decorations. - let baseFont = NSFont.systemFont(ofSize: NSFont.systemFontSize) + let baseFont = MessageTextScale.baseFont let result = NSMutableAttributedString(attributedString: NSAttributedString(source)) let fullRange = NSRange(location: 0, length: result.length) @@ -360,7 +360,7 @@ private struct MatrixHTMLParser { // swiftlint:disable:this type_body_length case "blockquote": blockquoteDepth += 1 - let baseFont = NSFont.systemFont(ofSize: NSFont.systemFontSize) + let baseFont = MessageTextScale.baseFont let barString = "\u{2502} " let barWidth = (barString as NSString) .size(withAttributes: [.font: baseFont]).width @@ -392,7 +392,7 @@ private struct MatrixHTMLParser { // swiftlint:disable:this type_body_length let separator = NSAttributedString( string: "\u{2500}\u{2500}\u{2500}\u{2500}\u{2500}\u{2500}\u{2500}\u{2500}", attributes: [ - .font: NSFont.systemFont(ofSize: NSFont.systemFontSize), + .font: MessageTextScale.baseFont, .foregroundColor: NSColor.separatorColor ] ) @@ -417,7 +417,7 @@ private struct MatrixHTMLParser { // swiftlint:disable:this type_body_length marker = "\(bullets[min(depth - 1, bullets.count - 1)]) " } ensureNewline(in: result) - let baseFont = NSFont.systemFont(ofSize: NSFont.systemFontSize) + let baseFont = MessageTextScale.baseFont let markerWidth = (marker as NSString) .size(withAttributes: [.font: baseFont]).width let basePad: CGFloat = 6.0 @@ -523,7 +523,7 @@ private struct MatrixHTMLParser { // swiftlint:disable:this type_body_length } } // Apply paragraph style for blockquote wrapping. - let baseFont = NSFont.systemFont(ofSize: NSFont.systemFontSize) + let baseFont = MessageTextScale.baseFont let barWidth = ("\u{2502} " as NSString) .size(withAttributes: [.font: baseFont]).width let style = NSMutableParagraphStyle() @@ -571,7 +571,7 @@ private struct MatrixHTMLParser { // swiftlint:disable:this type_body_length let level = Int(String(tag.last!))! let scales: [CGFloat] = [1.5, 1.35, 1.2, 1.1, 1.05, 1.0] let scale = scales[min(level - 1, scales.count - 1)] - let headingSize = NSFont.systemFontSize * scale + let headingSize = MessageTextScale.baseFontSize * scale let headingFont = NSFont.boldSystemFont(ofSize: headingSize) result.addAttribute(.font, value: headingFont, range: range) let style = NSMutableParagraphStyle() @@ -599,7 +599,7 @@ private struct MatrixHTMLParser { // swiftlint:disable:this type_body_length // Re-derive the paragraph style for this list depth. let depth = listStack.count if depth > 0 { - let baseFont = NSFont.systemFont(ofSize: NSFont.systemFontSize) + let baseFont = MessageTextScale.baseFont // Use a placeholder marker to measure width consistently. let sampleMarker = listStack[depth - 1].ordered ? "0. " : "\u{2022} " let markerWidth = (sampleMarker as NSString) @@ -664,7 +664,7 @@ private struct MatrixHTMLParser { // swiftlint:disable:this type_body_length // swiftlint:disable:next cyclomatic_complexity function_body_length private func buildAttributes(from style: Style) -> [NSAttributedString.Key: Any] { - let baseSize = NSFont.systemFontSize + let baseSize = MessageTextScale.baseFontSize var attrs: [NSAttributedString.Key: Any] = [:] // Font diff --git a/Relay/Utilities/MessageTextScale.swift b/Relay/Utilities/MessageTextScale.swift new file mode 100644 index 00000000..d157b5c8 --- /dev/null +++ b/Relay/Utilities/MessageTextScale.swift @@ -0,0 +1,84 @@ +// Copyright 2026 Link Dupont +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import AppKit + +/// The user-adjustable zoom level for conversation and compose text. +/// +/// Message bodies, mention pills, and the compose field derive their point size +/// from ``baseFont`` rather than `NSFont.systemFontSize` directly, so the whole +/// reading and typing area scales together when the View ▸ Make Text +/// Bigger/Smaller commands change the scale. The factor is persisted in +/// `UserDefaults`; ``increase()``/``decrease()``/``reset()`` update it, drop the +/// now-stale parse caches, and broadcast ``didChangeNotification`` so the +/// timeline can re-measure its rows and the compose bar can re-apply its font. +enum MessageTextScale { + /// `UserDefaults` key holding the scale factor as a `Double`. + nonisolated static let userDefaultsKey = "timeline.textScale" + + /// Posted after the scale changes and the parse caches are cleared. + static let didChangeNotification = Notification.Name("relay.messageTextScaleDidChange") + + /// Neutral scale — message text renders at the system font size. + nonisolated static let defaultScale: CGFloat = 1 + nonisolated static let minScale: CGFloat = 0.8 + nonisolated static let maxScale: CGFloat = 2.4 + + /// Additive step applied by ``increase()`` / ``decrease()``. + private static let step: CGFloat = 0.1 + + /// The current scale factor (1.0 = system size), clamped to a sane range. + /// + /// `nonisolated` so message parsing can read it off the main actor; the + /// backing `UserDefaults` read is itself thread-safe. + nonisolated static var scale: CGFloat { + let stored = UserDefaults.standard.object(forKey: userDefaultsKey) as? Double + let value = stored.map { CGFloat($0) } ?? defaultScale + return min(max(value, minScale), maxScale) + } + + /// The base message/compose font point size at the current scale. + nonisolated static var baseFontSize: CGFloat { + NSFont.systemFontSize * scale + } + + /// The base message/compose font at the current scale. + nonisolated static var baseFont: NSFont { + NSFont.systemFont(ofSize: baseFontSize) + } + + @MainActor static func increase() { apply(scale + step) } + @MainActor static func decrease() { apply(scale - step) } + @MainActor static func reset() { apply(defaultScale) } + + /// Persists `newValue` (clamped), invalidates the caches that hold text laid + /// out at the old size, and notifies observers. A no-op when the clamped + /// value is unchanged, so hitting the limit doesn't churn the timeline. + @MainActor private static func apply(_ newValue: CGFloat) { + let clamped = min(max(newValue, minScale), maxScale) + guard abs(clamped - scale) > 0.001 else { return } + UserDefaults.standard.set(Double(clamped), forKey: userDefaultsKey) + invalidateCaches() + NotificationCenter.default.post(name: didChangeNotification, object: nil) + } + + /// Drops every cached parse result, since each was laid out at the previous + /// base font size. + @MainActor private static func invalidateCaches() { + MessageBubbleContent.htmlCache.removeAll() + MessageBubbleContent.markdownCache.removeAll() + MessageBubbleContent.emoteHtmlCache.removeAll() + ReplyPreviewBubble.replyTextCache.removeAll() + } +} diff --git a/Relay/Views/Compose/ComposeTextView.swift b/Relay/Views/Compose/ComposeTextView.swift index b0446684..58851c72 100644 --- a/Relay/Views/Compose/ComposeTextView.swift +++ b/Relay/Views/Compose/ComposeTextView.swift @@ -60,9 +60,9 @@ struct ComposeTextView: NSViewRepresentable { textView.isVerticallyResizable = true textView.isHorizontallyResizable = false textView.textContainerInset = NSSize(width: 8, height: 10) - textView.font = .systemFont(ofSize: NSFont.systemFontSize) + textView.font = MessageTextScale.baseFont textView.typingAttributes = [ - .font: NSFont.systemFont(ofSize: NSFont.systemFontSize), + .font: MessageTextScale.baseFont, .foregroundColor: NSColor.textColor, ] textView.placeholderString = "Message" @@ -78,6 +78,7 @@ struct ComposeTextView: NSViewRepresentable { scrollView.linkedTextView = textView context.coordinator.textView = textView + context.coordinator.startObservingTextScale() // Expose the mention insertion closure to the parent view. // Safe to assign synchronously because `insertMentionHandler` is @@ -170,6 +171,35 @@ struct ComposeTextView: NSViewRepresentable { self.parent = parent } + deinit { + NotificationCenter.default.removeObserver(self) + } + + /// Re-applies the compose font and re-measures the field whenever the + /// timeline text-zoom level changes, so the compose bar scales in step + /// with the conversation. + func startObservingTextScale() { + NotificationCenter.default.removeObserver( + self, name: MessageTextScale.didChangeNotification, object: nil + ) + NotificationCenter.default.addObserver( + self, + selector: #selector(textScaleDidChange), + name: MessageTextScale.didChangeNotification, + object: nil + ) + } + + @objc private func textScaleDidChange() { + guard let textView, let storage = textView.textStorage else { return } + let font = Coordinator.composeFontForText(parent.text) + applyFont(font, to: storage) + textView.typingAttributes[.font] = font + textView.font = font + textView.recalculateHeight() + parent.onHeightChange?(textView.cachedHeight) + } + // MARK: - Plain Text Extraction /// Extracts plain text from the text storage, replacing pill attachments @@ -220,9 +250,9 @@ struct ComposeTextView: NSViewRepresentable { static func composeFontForText(_ text: String) -> NSFont { let trimmed = text.trimmingCharacters(in: .whitespacesAndNewlines) if !trimmed.isEmpty && trimmed.isEmojiOnly { - return .systemFont(ofSize: emojiFontSize) + return .systemFont(ofSize: emojiFontSize * MessageTextScale.scale) } - return .systemFont(ofSize: NSFont.systemFontSize) + return MessageTextScale.baseFont } /// Applies a font to the entire text storage, skipping pill attachments. diff --git a/Relay/Views/Message/MessageAttributeResolver.swift b/Relay/Views/Message/MessageAttributeResolver.swift index 474efc09..c9615966 100644 --- a/Relay/Views/Message/MessageAttributeResolver.swift +++ b/Relay/Views/Message/MessageAttributeResolver.swift @@ -46,7 +46,7 @@ extension MessageTextView { let result = NSMutableAttributedString(attributedString: source) let fullRange = NSRange(location: 0, length: result.length) let keys = NSAttributedString.Key.self - let baseFont = NSFont.systemFont(ofSize: NSFont.systemFontSize) + let baseFont = MessageTextScale.baseFont // Muted color for blockquote text content. let mutedForeground = foreground.withAlphaComponent(0.75) diff --git a/Relay/Views/Message/MessageBubbleContent.swift b/Relay/Views/Message/MessageBubbleContent.swift index 18fb4127..97fb1b50 100644 --- a/Relay/Views/Message/MessageBubbleContent.swift +++ b/Relay/Views/Message/MessageBubbleContent.swift @@ -274,7 +274,7 @@ struct MessageBubbleContent: View { let cached = Self.emoteHtmlCache.value(forKey: cacheKey) { guard let parsed = NSAttributedString(matrixHTML: html) else { return nil } let emoteResult = NSMutableAttributedString() - let nameFont = NSFont.systemFont(ofSize: NSFont.systemFontSize) + let nameFont = MessageTextScale.baseFont let italicDesc = nameFont.fontDescriptor.withSymbolicTraits(.italic) let italicFont = NSFont(descriptor: italicDesc, size: nameFont.pointSize) ?? nameFont emoteResult.append(NSAttributedString( diff --git a/Relay/Views/Timeline/TimelineTableView.swift b/Relay/Views/Timeline/TimelineTableView.swift index c5db14f7..d8ed33db 100644 --- a/Relay/Views/Timeline/TimelineTableView.swift +++ b/Relay/Views/Timeline/TimelineTableView.swift @@ -381,6 +381,14 @@ final class TimelineTableViewController: NSViewController { // Link previews use a fixed-size card, so no height re-measurement // is needed when metadata loads. + + // Re-render and re-measure every row when the message text-zoom changes. + NotificationCenter.default.addObserver( + self, + selector: #selector(messageTextScaleDidChange), + name: MessageTextScale.didChangeNotification, + object: nil + ) } // MARK: - Data Source @@ -794,15 +802,6 @@ final class TimelineTableViewController: NSViewController { } } - /// Re-measures a single row whose content height changed without any - /// change to the underlying message data — specifically when a collapsed - /// system-event group is expanded or collapsed. - /// - /// `updateRows` only re-measures when `rows` diff, which they don't here - /// (only the shared ``ExpandedGroupsState`` flipped). We invalidate the - /// cached height for this row and note its new height; `heightOfRow`'s - /// measurement host rebuilds the row reading the now-updated expansion - /// state, so it returns the full expanded (or collapsed) height. /// Message IDs awaiting a debounced height re-measure. private var pendingRemeasureIDs: Set = [] /// Coalesces a burst of ``remeasureRow(forMessageID:)`` calls into one pass. @@ -811,6 +810,16 @@ final class TimelineTableViewController: NSViewController { /// trailing debounce can't be reset indefinitely. private var remeasureMaxWaitTask: Task? + /// Re-measures a row whose content height changed without any change to the + /// underlying message data — when a collapsed system-event group is expanded + /// or collapsed, or when a link-preview card resolves to its final size. + /// + /// `updateRows` only re-measures when `rows` diff, which they don't in these + /// cases. We invalidate the row's cached height and note its new height; the + /// measurement host rebuilds the row reading the current state, so it returns + /// the correct height. Calls are debounced (trailing, with a max-wait) so a + /// burst — e.g. several link-preview cards resolving at once — collapses into + /// a single pass. func remeasureRow(forMessageID id: String) { let wasEmpty = pendingRemeasureIDs.isEmpty pendingRemeasureIDs.insert(id) @@ -871,6 +880,28 @@ final class TimelineTableViewController: NSViewController { } } + // MARK: - Text Zoom + + /// Re-renders and re-measures every row when the message text-zoom level + /// changes. ``MessageTextScale`` has already dropped the parse caches, so + /// reloading the row views re-parses their text at the new base font size, + /// and clearing the height cache forces a fresh measurement per row. + @objc private func messageTextScaleDidChange() { + guard !rows.isEmpty else { return } + heightCache.removeAll() + let wasNearBottom = isNearBottom + let all = IndexSet(integersIn: 0 ..< rows.count) + tableView.reloadData(forRowIndexes: all, columnIndexes: IndexSet(integer: 0)) + NSAnimationContext.runAnimationGroup { context in + context.duration = 0 + context.allowsImplicitAnimation = false + tableView.noteHeightOfRows(withIndexesChanged: all) + } + if wasNearBottom { + scrollToBottom(animated: false) + } + } + // MARK: - Resize Handling @objc private func viewDidResize(_ notification: Notification) { From b53c555def694a5c670e0d7473c72ca95518f800 Mon Sep 17 00:00:00 2001 From: Lukas Frias Santos Date: Sun, 19 Jul 2026 11:32:00 +0200 Subject: [PATCH 06/11] Re-measure timeline rows at the new width after a resize The resize handler pre-cached each visible cell's live fittingSize, but during a resize the cell's frame width has already changed while its SwiftUI content may not have re-flowed yet, so fittingSize still reports the pre-resize height. Caching that left the row at its old height, too short for the now-rewrapped text (it kept its size and clipped). Invalidate the visible rows and let heightOfRow re-measure them through the measurement host at the exact new width instead. Co-Authored-By: Claude Opus 4.8 --- Relay/Views/Timeline/TimelineTableView.swift | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/Relay/Views/Timeline/TimelineTableView.swift b/Relay/Views/Timeline/TimelineTableView.swift index d8ed33db..0fd6b697 100644 --- a/Relay/Views/Timeline/TimelineTableView.swift +++ b/Relay/Views/Timeline/TimelineTableView.swift @@ -912,12 +912,14 @@ final class TimelineTableViewController: NSViewController { // Capture whether the user is at the bottom before heights change. let wasNearBottom = isNearBottom - // Defer the height update so that live NSHostingView cells have - // time to re-layout their SwiftUI content at the new column width. - // On the next run-loop pass we walk the visible cells, read their - // `fittingSize` (which now reflects the new width), and pre-populate - // the height cache. Then `noteHeightOfRows` triggers `heightOfRow` - // which returns the cached value — no measurement host needed. + // Defer the height update so live cells settle at the new column width, + // then re-measure. We *invalidate* the visible rows' cached heights and + // let `heightOfRow` recompute them through the measurement host at the + // exact new width, rather than reading a live cell's `fittingSize`. + // During a resize the cell's frame width has already changed but its + // SwiftUI content may not have re-flowed yet, so `fittingSize` still + // reports the pre-resize height; caching that would leave the row too + // short for the now-rewrapped text (it keeps its old height and clips). // // Cancel any previously scheduled resize work so that rapid // resize events (live window drag) coalesce into a single @@ -928,7 +930,9 @@ final class TimelineTableViewController: NSViewController { let visible = self.tableView.rows(in: self.tableView.visibleRect) guard visible.length > 0 else { return } - self.preCacheHeights(for: visible) + for idx in visible.lowerBound ..< visible.upperBound where idx < self.rows.count { + self.invalidateHeight(for: self.rows[idx].id) + } NSAnimationContext.runAnimationGroup { context in context.duration = 0 From 5db6623a2674059e607a4cc2cee799b2faf42a65 Mon Sep 17 00:00:00 2001 From: Lukas Frias Santos Date: Sun, 19 Jul 2026 11:32:28 +0200 Subject: [PATCH 07/11] Refine text zoom: chrome scaling, debounce, and typed compose pills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Compose: drop the redundant applyFont() in the text-scale handler. NSText.font already re-fonts every character and sets the default, so the pill-skipping applyFont was dead code that contradicted the following set. - Timeline: anchor the first visible row when re-measuring after a zoom, so changing the text size while scrolled up keeps the same content in view instead of shifting. - Add a scaledChromeFont modifier that reads the zoom factor via @AppStorage and applies a system font at the text style's size times the scale. Use it for the sender-name label and the date-section separators, so the timeline chrome tracks the message text size. The detached measurement host reads the same value so row heights stay correct. - Debounce the timeline's text-zoom re-measure (60ms trailing) so holding or repeating ⌘+/⌘− collapses into a single reload+measure pass instead of one per step; chrome still re-renders live via @AppStorage in between. - Rebuild any mention pills typed into the compose field at the new base font size on zoom, since a pill's attachment image is rendered once at creation and doesn't otherwise resize. Co-Authored-By: Claude Opus 4.8 --- Relay/Utilities/MessageTextScale.swift | 26 +++++++++++++ Relay/Views/Compose/ComposeTextView.swift | 34 +++++++++++++++-- Relay/Views/Message/MessageView.swift | 3 +- Relay/Views/Timeline/TimelineRowView.swift | 3 +- Relay/Views/Timeline/TimelineTableView.swift | 39 +++++++++++++++++--- 5 files changed, 93 insertions(+), 12 deletions(-) diff --git a/Relay/Utilities/MessageTextScale.swift b/Relay/Utilities/MessageTextScale.swift index d157b5c8..2cd4f95a 100644 --- a/Relay/Utilities/MessageTextScale.swift +++ b/Relay/Utilities/MessageTextScale.swift @@ -13,6 +13,7 @@ // limitations under the License. import AppKit +import SwiftUI /// The user-adjustable zoom level for conversation and compose text. /// @@ -82,3 +83,28 @@ enum MessageTextScale { ReplyPreviewBubble.replyTextCache.removeAll() } } + +// MARK: - Scaled Chrome Font + +private struct ScaledChromeFont: ViewModifier { + let textStyle: NSFont.TextStyle + let weight: Font.Weight + + @AppStorage(MessageTextScale.userDefaultsKey) private var scale = Double(MessageTextScale.defaultScale) + + func body(content: Content) -> some View { + let base = NSFont.preferredFont(forTextStyle: textStyle).pointSize + content.font(.system(size: base * CGFloat(scale), weight: weight)) + } +} + +extension View { + /// Applies a system font for `textStyle` scaled by the current message + /// text-zoom level — for chrome (sender names, timestamps) that should track + /// the conversation text. Reading the scale through `@AppStorage` re-renders + /// on zoom regardless of any `Equatable` view optimization, and the detached + /// row-measurement host reads the same value so heights stay correct. + func scaledChromeFont(_ textStyle: NSFont.TextStyle, weight: Font.Weight = .regular) -> some View { + modifier(ScaledChromeFont(textStyle: textStyle, weight: weight)) + } +} diff --git a/Relay/Views/Compose/ComposeTextView.swift b/Relay/Views/Compose/ComposeTextView.swift index 58851c72..43a4406c 100644 --- a/Relay/Views/Compose/ComposeTextView.swift +++ b/Relay/Views/Compose/ComposeTextView.swift @@ -191,15 +191,43 @@ struct ComposeTextView: NSViewRepresentable { } @objc private func textScaleDidChange() { - guard let textView, let storage = textView.textStorage else { return } + guard let textView else { return } let font = Coordinator.composeFontForText(parent.text) - applyFont(font, to: storage) - textView.typingAttributes[.font] = font + // `NSText.font` re-fonts all existing characters and sets the default + // for typing and the placeholder, so it covers the empty field too. textView.font = font + textView.typingAttributes[.font] = font + if let storage = textView.textStorage { + rescalePillAttachments(in: storage) + } textView.recalculateHeight() parent.onHeightChange?(textView.cachedHeight) } + /// Rebuilds any inline mention pills at the new base font size so typed + /// mentions scale with the zoom. A pill's attachment image is rendered + /// once at creation, so it doesn't otherwise resize. + private func rescalePillAttachments(in storage: NSTextStorage) { + let fullRange = NSRange(location: 0, length: storage.length) + var replacements: [(NSRange, PillTextAttachment)] = [] + storage.enumerateAttribute(.attachment, in: fullRange, options: []) { value, range, _ in + guard let old = value as? PillTextAttachment else { return } + replacements.append(( + range, + PillTextAttachment( + userId: old.userId, displayName: old.displayName, + font: MessageTextScale.baseFont + ) + )) + } + guard !replacements.isEmpty else { return } + storage.beginEditing() + for (range, pill) in replacements { + storage.addAttribute(.attachment, value: pill, range: range) + } + storage.endEditing() + } + // MARK: - Plain Text Extraction /// Extracts plain text from the text storage, replacing pill attachments diff --git a/Relay/Views/Message/MessageView.swift b/Relay/Views/Message/MessageView.swift index ee4ed509..1ef43be0 100644 --- a/Relay/Views/Message/MessageView.swift +++ b/Relay/Views/Message/MessageView.swift @@ -114,8 +114,7 @@ struct MessageView: View { if showSenderName && !message.isOutgoing { Text(message.displayName) - .font(.caption) - .fontWeight(.medium) + .scaledChromeFont(.caption1, weight: .medium) .foregroundStyle(.secondary) .padding(.leading, BubbleStyle.horizontalPadding) .padding(.bottom, 2) diff --git a/Relay/Views/Timeline/TimelineRowView.swift b/Relay/Views/Timeline/TimelineRowView.swift index 24641a96..a6223e0a 100644 --- a/Relay/Views/Timeline/TimelineRowView.swift +++ b/Relay/Views/Timeline/TimelineRowView.swift @@ -132,8 +132,7 @@ struct TimelineRowView: View, Equatable { if info.showDateHeader { Text(dateSectionLabel(for: message.timestamp)) - .font(.caption2) - .fontWeight(.medium) + .scaledChromeFont(.caption2, weight: .medium) .foregroundStyle(.secondary) .padding(.top, info.isFirst ? 4 : 12) .padding(.bottom, 4) diff --git a/Relay/Views/Timeline/TimelineTableView.swift b/Relay/Views/Timeline/TimelineTableView.swift index 0fd6b697..6f45576b 100644 --- a/Relay/Views/Timeline/TimelineTableView.swift +++ b/Relay/Views/Timeline/TimelineTableView.swift @@ -260,6 +260,8 @@ final class TimelineTableViewController: NSViewController { /// Coalesces rapid resize events so only the final one runs. private var resizeWorkItem: DispatchWorkItem? + /// Coalesces bursts of text-zoom changes into one re-measure pass. + private var textScaleRemeasureTask: Task? /// A reusable hosting controller used to measure SwiftUI row heights /// for rows that don't have a live cell on screen. Only used as a @@ -317,6 +319,7 @@ final class TimelineTableViewController: NSViewController { paginateTask?.cancel() remeasureDebounceTask?.cancel() remeasureMaxWaitTask?.cancel() + textScaleRemeasureTask?.cancel() } } @@ -882,14 +885,34 @@ final class TimelineTableViewController: NSViewController { // MARK: - Text Zoom - /// Re-renders and re-measures every row when the message text-zoom level - /// changes. ``MessageTextScale`` has already dropped the parse caches, so - /// reloading the row views re-parses their text at the new base font size, - /// and clearing the height cache forces a fresh measurement per row. + /// Coalesces a burst of text-zoom changes (e.g. holding or repeating ⌘+) + /// into a single re-measure, since re-rendering and re-measuring every row + /// is main-thread work. Chrome already re-renders live via `@AppStorage`; + /// this trailing pass fixes the row heights once the scale settles. @objc private func messageTextScaleDidChange() { + textScaleRemeasureTask?.cancel() + textScaleRemeasureTask = Task { @MainActor [weak self] in + try? await Task.sleep(for: .milliseconds(60)) + guard let self, !Task.isCancelled else { return } + self.remeasureAllRowsForTextScale() + } + } + + /// Re-renders and re-measures every row at the current text-zoom level. + /// ``MessageTextScale`` has already dropped the parse caches, so reloading + /// the row views re-parses their text at the new base font size, and + /// clearing the height cache forces a fresh measurement per row. The first + /// visible row is anchored so zooming while scrolled up keeps the same + /// content in view rather than jumping (every row's height changes). + private func remeasureAllRowsForTextScale() { guard !rows.isEmpty else { return } - heightCache.removeAll() let wasNearBottom = isNearBottom + let anchorRow = tableView.rows(in: tableView.visibleRect).location + let anchorOffset = anchorRow >= 0 + ? tableView.rect(ofRow: anchorRow).minY - tableView.visibleRect.minY + : 0 + + heightCache.removeAll() let all = IndexSet(integersIn: 0 ..< rows.count) tableView.reloadData(forRowIndexes: all, columnIndexes: IndexSet(integer: 0)) NSAnimationContext.runAnimationGroup { context in @@ -897,8 +920,14 @@ final class TimelineTableViewController: NSViewController { context.allowsImplicitAnimation = false tableView.noteHeightOfRows(withIndexesChanged: all) } + tableView.layoutSubtreeIfNeeded() + if wasNearBottom { scrollToBottom(animated: false) + } else if anchorRow >= 0, anchorRow < rows.count { + let targetY = tableView.rect(ofRow: anchorRow).minY - anchorOffset + scrollView.contentView.scroll(to: CGPoint(x: 0, y: targetY)) + scrollView.reflectScrolledClipView(scrollView.contentView) } } From eec6d297538f735a586863017c7cd5e5d7e539f2 Mon Sep 17 00:00:00 2001 From: Lukas Frias Santos Date: Sun, 19 Jul 2026 11:33:21 +0200 Subject: [PATCH 08/11] Fix timeline bubble clipping and row-height measurement across resize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several linked fixes for bubbles clipping at narrow widths and after resize. Root cause: the timeline table ignores safe areas and spans the full window; the overlay sidebar contributes a leading safe-area inset (~170pt) that the live NSHostingView cells respect when laying out their SwiftUI content. The detached measurement host used by heightOfRow knew nothing about that inset, so it measured rows at the full column width — proposing a wider text wrap than the live cells actually used. At narrow window widths the live text wrapped into more lines than the measured height allowed, and the too-tall content clipped symmetrically top and bottom — cutting bubble backgrounds, the bottom-aligned avatars, and trailing "edited" labels. At wide windows both paths clamp at the 500pt bubble cap and agree, which is why the bug only showed after narrowing or at narrow launch. - heightOfRow now measures at the effective content width (column width minus horizontal safe-area insets), matching the live layout. Watch the effective width from the table's layout() hook as well as viewDidLayout: a sidebar (safe-area) change re-lays the cells without resizing the scroll view or column, so neither resize path saw it. Don't latch a width while rows are still empty, and re-check once the first rows land. - Drop the preCacheHeights fittingSize shortcut: NSHostingView's fittingSize reports the stale frame height, not the content's needed height, and re-poisoned the cache with pre-inset values after every structural update. - Recycled cells kept a stale text-container width on reuse, because reassigning a cell's rootView to the *same* cached attributed string leaves MessageTextView's updateNSView an unchanged input, so it early-returns without re-syncing its container. Drop the message parse caches on a full re-measure so each row's attributed string is rebuilt as a new instance and the cell re-resolves and re-wraps at the current width. - MessageTextView.sizeThatFits now restores the text container to its pre-measurement width instead of leaking a measurement width (or chasing the live bounds). SwiftUI runs an unconstrained ideal-size query after setFrameSize; leaking that stranded the container wide (horizontal clip) or, chased to bounds, narrow (a feedback loop). setFrameSize stays the sole display-width authority. Discard the shared measurement host at the start of a full re-measure and add a generation-based size-cache invalidation, since the host re-measures on a content change but returns the previous height when only the width proposal changes — on a window resize the text re-wrapped but rows kept their old (clipped) height. - Pin the "edited" label's ideal height so it isn't compressed below its measured height. - Window resize routes through a full reload+re-measure pass (debounced so a live drag coalesces once it settles) instead of only re-measuring visible rows, so every cell re-lays-out at the new width; base the resize guard on the scroll view's width, which is current when the frame-change notification fires. Keep live drags responsive: mid-drag, run a throttled visible-rows height pass; on drag end, settle the whole timeline immediately instead of waiting out the debounce. Co-Authored-By: Claude Opus 4.8 Co-Authored-By: Claude Fable 5 --- Relay/Utilities/MessageTextScale.swift | 11 +- Relay/Views/Message/MessageBodyParser.swift | 16 + .../Views/Message/MessageBubbleContent.swift | 1 + Relay/Views/Message/MessageTextView.swift | 40 ++- Relay/Views/Timeline/TimelineTableView.swift | 287 +++++++++++++----- .../TimelineHeightMeasurementTests.swift | 25 ++ 6 files changed, 294 insertions(+), 86 deletions(-) diff --git a/Relay/Utilities/MessageTextScale.swift b/Relay/Utilities/MessageTextScale.swift index 2cd4f95a..26327064 100644 --- a/Relay/Utilities/MessageTextScale.swift +++ b/Relay/Utilities/MessageTextScale.swift @@ -70,18 +70,9 @@ enum MessageTextScale { let clamped = min(max(newValue, minScale), maxScale) guard abs(clamped - scale) > 0.001 else { return } UserDefaults.standard.set(Double(clamped), forKey: userDefaultsKey) - invalidateCaches() + MessageBubbleContent.invalidateParseCaches() NotificationCenter.default.post(name: didChangeNotification, object: nil) } - - /// Drops every cached parse result, since each was laid out at the previous - /// base font size. - @MainActor private static func invalidateCaches() { - MessageBubbleContent.htmlCache.removeAll() - MessageBubbleContent.markdownCache.removeAll() - MessageBubbleContent.emoteHtmlCache.removeAll() - ReplyPreviewBubble.replyTextCache.removeAll() - } } // MARK: - Scaled Chrome Font diff --git a/Relay/Views/Message/MessageBodyParser.swift b/Relay/Views/Message/MessageBodyParser.swift index 34499755..db8decde 100644 --- a/Relay/Views/Message/MessageBodyParser.swift +++ b/Relay/Views/Message/MessageBodyParser.swift @@ -26,6 +26,22 @@ extension MessageBubbleContent { /// LRU cache for parsed emote HTML bodies. Shared across all `MessageBubbleContent` instances. static let emoteHtmlCache = ParseCache(capacity: 64) + + /// Drops every message parse cache. + /// + /// Call when a global change must force every row to re-render from scratch: + /// a text-zoom step, or a window resize (recycled cells otherwise keep a + /// stale text-container width and clip). Because the caches key by content, + /// the returned attributed string is a *new* instance, which makes + /// ``MessageTextView``'s `updateNSView` re-resolve and re-sync its container + /// to the current width instead of early-returning on an unchanged instance. + @MainActor + static func invalidateParseCaches() { + htmlCache.removeAll() + markdownCache.removeAll() + emoteHtmlCache.removeAll() + ReplyPreviewBubble.replyTextCache.removeAll() + } } // MARK: - Parse Caches (ReplyPreviewBubble) diff --git a/Relay/Views/Message/MessageBubbleContent.swift b/Relay/Views/Message/MessageBubbleContent.swift index 97fb1b50..6b0a714d 100644 --- a/Relay/Views/Message/MessageBubbleContent.swift +++ b/Relay/Views/Message/MessageBubbleContent.swift @@ -94,6 +94,7 @@ struct MessageBubbleContent: View { Text("edited") .font(.caption2) .foregroundStyle(.tertiary) + .fixedSize(horizontal: false, vertical: true) .padding(.horizontal, BubbleStyle.horizontalPadding) } } diff --git a/Relay/Views/Message/MessageTextView.swift b/Relay/Views/Message/MessageTextView.swift index d032d2c3..4c67852d 100644 --- a/Relay/Views/Message/MessageTextView.swift +++ b/Relay/Views/Message/MessageTextView.swift @@ -85,8 +85,22 @@ struct MessageTextView: NSViewRepresentable { var cachedSizeResult: CGSize? var cachedSizeTextLength: Int? var cachedSizeTextHash: Int? + /// The ``sizeCacheGeneration`` the cached size was measured under. A + /// width change bumps the generation to invalidate every cell's cache. + var cachedSizeGeneration: Int = -1 } + /// Bumped whenever the timeline re-lays-out at a new width. Recycled cells + /// keep their `Coordinator` (and its size cache) across a width change; a + /// cell measured narrow mid-drag would otherwise return that stale, narrower + /// width from `sizeThatFits`, so its bubble hugs the narrow width and wraps + /// an extra line that clips. Invalidating the caches forces a fresh measure + /// at the settled width. See ``invalidateSizeCaches()``. + @MainActor static var sizeCacheGeneration: Int = 0 + + /// Invalidates every cell's `sizeThatFits` cache (see ``sizeCacheGeneration``). + @MainActor static func invalidateSizeCaches() { sizeCacheGeneration &+= 1 } + func makeCoordinator() -> Coordinator { Coordinator() } func makeNSView(context: Context) -> MessageTextContent { @@ -202,15 +216,36 @@ struct MessageTextView: NSViewRepresentable { let coordinator = context.coordinator let textHash = nsView.textStorage?.string.hashValue ?? 0 if let cached = coordinator.cachedSizeResult, + coordinator.cachedSizeGeneration == Self.sizeCacheGeneration, coordinator.cachedSizeTextLength == textLength, coordinator.cachedSizeTextHash == textHash, coordinator.cachedSizeProposedWidth == proposedWidth { return cached } - // Prevent setFrameSize from constraining the container while we measure. + // Prevent setFrameSize from constraining the container while we measure, + // then leave the container at the view's actual render (frame) width. + // `sizeThatFits` sets the container to several widths — including the + // unconstrained natural width for an ideal-size query — and SwiftUI + // issues those queries in an order we don't control. Whatever width is + // left in the container is the width the live text wraps at, so it must + // end up equal to the frame: leaving it wider strands the text past its + // frame (horizontal clip); leaving it narrower — which is what restoring + // the *pre-measurement* container width did for a recycled or resized + // cell whose container was already stale — wraps an extra line that eats + // the bubble padding and clips the top and bottom. The frame width is + // authoritative: `setFrameSize` keeps it current, and it is exactly the + // width SwiftUI renders the text at. nsView.suppressContainerSync = true - defer { nsView.suppressContainerSync = false } + defer { + let renderWidth = nsView.frame.width + if renderWidth > 0 { + container.containerSize = NSSize( + width: renderWidth, height: CGFloat.greatestFiniteMagnitude + ) + } + nsView.suppressContainerSync = false + } // Natural layout (unconstrained) to find the intrinsic text width. container.containerSize = NSSize( @@ -288,6 +323,7 @@ struct MessageTextView: NSViewRepresentable { coordinator.cachedSizeResult = result coordinator.cachedSizeTextLength = textLength coordinator.cachedSizeTextHash = textHash + coordinator.cachedSizeGeneration = Self.sizeCacheGeneration return result } diff --git a/Relay/Views/Timeline/TimelineTableView.swift b/Relay/Views/Timeline/TimelineTableView.swift index 6f45576b..d67d8ac7 100644 --- a/Relay/Views/Timeline/TimelineTableView.swift +++ b/Relay/Views/Timeline/TimelineTableView.swift @@ -68,6 +68,29 @@ final class TimelineTableProxy { final class BottomAnchoredTableView: NSTableView { override var isFlipped: Bool { false } + /// Called after every layout pass. The timeline controller uses this to + /// detect effective-content-width changes (column width minus horizontal + /// safe-area insets). A safe-area change — the overlay sidebar appearing, + /// disappearing, or resizing — re-lays the cells without resizing the + /// scroll view or the column, so neither `viewDidResize` nor the + /// controller's `viewDidLayout` is guaranteed to see it. + var onLayout: (() -> Void)? + + /// Called when a live window-resize drag ends, so the controller can run + /// the full-timeline height pass immediately instead of waiting out a + /// debounce. + var onLiveResizeEnd: (() -> Void)? + + override func layout() { + super.layout() + onLayout?() + } + + override func viewDidEndLiveResize() { + super.viewDidEndLiveResize() + onLiveResizeEnd?() + } + // MARK: - Swipe-to-Reply Gesture /// Called with `(row, offsetX)` during a horizontal swipe. @@ -258,8 +281,16 @@ final class TimelineTableViewController: NSViewController { /// Tracks the last column width so we can invalidate row heights on resize. private var lastColumnWidth: CGFloat = 0 + /// The table *column* width the rows were last measured at. Rows are measured + /// at this width (the width the cell content actually renders at), which can + /// change without the scroll view's frame changing — e.g. a vertical scroller + /// appearing/disappearing, or the initial layout settling after launch. + /// ``viewDidLayout`` watches it so those changes still trigger a re-measure. + private var lastRenderWidth: CGFloat = 0 + /// Coalesces rapid resize events so only the final one runs. - private var resizeWorkItem: DispatchWorkItem? + /// Coalesces a live window-resize drag into one re-measure once it settles. + private var resizeRemeasureTask: Task? /// Coalesces bursts of text-zoom changes into one re-measure pass. private var textScaleRemeasureTask: Task? @@ -268,7 +299,7 @@ final class TimelineTableViewController: NSViewController { /// fallback when no cached height exists. Using the concrete /// ``TimelineRowView`` type avoids `AnyView` type-erasure overhead /// and lets SwiftUI reuse the internal view hierarchy between measurements. - private var measurementHost: NSHostingController? + private var measurementHost: NSHostingController? /// Caches measured row heights keyed on `(messageID, roundedWidth)`. /// Avoids redundant `NSHostingController.sizeThatFits` calls during @@ -320,6 +351,7 @@ final class TimelineTableViewController: NSViewController { remeasureDebounceTask?.cancel() remeasureMaxWaitTask?.cancel() textScaleRemeasureTask?.cancel() + resizeRemeasureTask?.cancel() } } @@ -353,6 +385,16 @@ final class TimelineTableViewController: NSViewController { tableView.lockedOffset = { [weak self] in self?.swipeState.offset ?? 0 } + tableView.onLayout = { [weak self] in + self?.scheduleRemeasureIfEffectiveWidthChanged() + } + tableView.onLiveResizeEnd = { [weak self] in + guard let self else { return } + // The drag is over — settle the whole timeline now rather than + // waiting out a debounce, so the release doesn't feel laggy. + self.resizeRemeasureTask?.cancel() + self.remeasureAllRows(reloadCells: true, dropParseCaches: false) + } scrollView.documentView = tableView scrollView.hasVerticalScroller = true @@ -612,9 +654,13 @@ final class TimelineTableViewController: NSViewController { let hasNewlyAppended = !newlyAppendedMessageIDs.isEmpty DispatchQueue.main.async { [weak self] in guard let self else { return } + // The first rows may arrive after the initial layout passes have + // all run (the width check refuses to latch while the table is + // empty) — re-check now so those rows get measured at the current + // effective width. + self.scheduleRemeasureIfEffectiveWidthChanged() let visible = self.tableView.rows(in: self.tableView.visibleRect) if visible.length > 0 { - self.preCacheHeights(for: visible) self.tableView.noteHeightOfRows( withIndexesChanged: IndexSet(integersIn: visible.lowerBound ..< visible.upperBound) ) @@ -784,27 +830,6 @@ final class TimelineTableViewController: NSViewController { } } - /// Walks visible live cells and writes their current `fittingSize` into - /// the height cache. Call this *outside* of `heightOfRow` (e.g. from a - /// deferred block) so that the subsequent `noteHeightOfRows` can return - /// cached values without hitting the measurement host. - private func preCacheHeights(for visible: NSRange) { - var targetWidth = tableView.tableColumns.first?.width ?? 0 - if targetWidth < 1 { targetWidth = scrollView.frame.width } - let roundedWidth = targetWidth.rounded() - - for idx in visible.lowerBound ..< visible.upperBound { - guard idx < rows.count else { continue } - if let cell = tableView.view(atColumn: 0, row: idx, makeIfNecessary: false) - as? NSHostingView { - let h = cell.fittingSize.height - if h > 0 { - heightCache[HeightCacheKey(rows[idx].id, roundedWidth)] = h - } - } - } - } - /// Message IDs awaiting a debounced height re-measure. private var pendingRemeasureIDs: Set = [] /// Coalesces a burst of ``remeasureRow(forMessageID:)`` calls into one pass. @@ -894,17 +919,34 @@ final class TimelineTableViewController: NSViewController { textScaleRemeasureTask = Task { @MainActor [weak self] in try? await Task.sleep(for: .milliseconds(60)) guard let self, !Task.isCancelled else { return } - self.remeasureAllRowsForTextScale() + // Zoom changes the font but not the width, so nothing re-renders on + // its own — reload the cells and drop the (now stale-font) parse caches. + self.remeasureAllRows(reloadCells: true, dropParseCaches: true) } } - /// Re-renders and re-measures every row at the current text-zoom level. - /// ``MessageTextScale`` has already dropped the parse caches, so reloading - /// the row views re-parses their text at the new base font size, and - /// clearing the height cache forces a fresh measurement per row. The first - /// visible row is anchored so zooming while scrolled up keeps the same - /// content in view rather than jumping (every row's height changes). - private func remeasureAllRowsForTextScale() { + /// Re-measures every row and applies the new heights, anchoring the first + /// visible row so the viewport keeps the same content. + /// + /// - Parameters: + /// - reloadCells: When `true`, reassigns every recycled cell's `rootView` + /// so its content re-lays-out from scratch. Needed on a width change: + /// NSTableView re-wraps visible cells live as the column changes, but a + /// recycled `MessageTextView` can be left wrapping at a stale (narrower) + /// width from mid-drag, rendering an extra line that eats the bubble + /// padding and clips top/bottom. A fresh render at the settled width + /// restores correct wrapping. (Cheap: it re-renders, it does not re-parse.) + /// - dropParseCaches: When `true`, also drops the parsed-text caches — only + /// needed when the text *content/font* changed (a text-zoom step), not on + /// a resize, where re-parsing every message would needlessly churn the + /// main thread since parsing is width-independent. + /// + /// The shared measurement host is discarded regardless: an + /// `NSHostingController` re-measures on a content change but returns the + /// previous height when only the width *proposal* changes, so reusing it + /// across a resize would keep the old height. A fresh host measures at the + /// new width. + private func remeasureAllRows(reloadCells: Bool, dropParseCaches: Bool) { guard !rows.isEmpty else { return } let wasNearBottom = isNearBottom let anchorRow = tableView.rows(in: tableView.visibleRect).location @@ -912,9 +954,18 @@ final class TimelineTableViewController: NSViewController { ? tableView.rect(ofRow: anchorRow).minY - tableView.visibleRect.minY : 0 + if dropParseCaches { + MessageBubbleContent.invalidateParseCaches() + } + // Invalidate per-cell size caches so a cell measured narrow mid-drag + // re-measures at the settled width instead of hugging the stale width. + MessageTextView.invalidateSizeCaches() + measurementHost = nil heightCache.removeAll() let all = IndexSet(integersIn: 0 ..< rows.count) - tableView.reloadData(forRowIndexes: all, columnIndexes: IndexSet(integer: 0)) + if reloadCells { + tableView.reloadData(forRowIndexes: all, columnIndexes: IndexSet(integer: 0)) + } NSAnimationContext.runAnimationGroup { context in context.duration = 0 context.allowsImplicitAnimation = false @@ -933,52 +984,122 @@ final class TimelineTableViewController: NSViewController { // MARK: - Resize Handling - @objc private func viewDidResize(_ notification: Notification) { - let newWidth = tableView.tableColumns.first?.width ?? scrollView.frame.width - guard abs(newWidth - lastColumnWidth) > 1 else { return } - lastColumnWidth = newWidth + override func viewDidLayout() { + super.viewDidLayout() + scheduleRemeasureIfEffectiveWidthChanged() + } - // Capture whether the user is at the bottom before heights change. - let wasNearBottom = isNearBottom + /// Re-measures every row when the *effective content width* changes — the + /// column width minus the horizontal safe-area insets, i.e. the width the + /// cell's SwiftUI content actually renders and wraps at. + /// + /// Rows are measured at this width. It can change without the scroll view's + /// frame changing, so `viewDidResize` (which watches the scroll-view frame) + /// misses it: a vertical scroller appearing or disappearing, the initial + /// layout settling right after launch, and — because the table ignores safe + /// areas and spans the full window under the overlay sidebar — the sidebar + /// appearing, disappearing, or resizing. When that happens the visible + /// cells re-wrap live but keep their old (too-short) measured heights, + /// clipping the re-wrapped content — and it never self-corrects because + /// the scroll-view frame never changes again. + /// + /// Called from `viewDidLayout` and from the table view's own `layout()` + /// hook (a safe-area change re-lays the cells without laying out the + /// controller's view). The width guard makes this a no-op on the frequent + /// layout passes that don't change the width (scrolling, the re-measure's + /// own `layoutSubtreeIfNeeded`), so there is no feedback loop. + private func scheduleRemeasureIfEffectiveWidthChanged() { + // Don't latch a width while the table is still empty: the initial rows + // arrive after the first layout passes, and a latched width would + // suppress the re-measure those rows need at this same width. + guard !rows.isEmpty else { return } + let columnWidth = tableView.tableColumns.first?.width ?? 0 + let renderWidth = columnWidth - tableView.safeAreaInsets.left - tableView.safeAreaInsets.right + guard columnWidth > 1, renderWidth > 1, abs(renderWidth - lastRenderWidth) > 0.5 else { return } + lastRenderWidth = renderWidth + + if tableView.inLiveResize { + // Mid-drag: keep the *visible* rows' heights tracking the drag so + // the resize feels live. A full-timeline pass here would re-measure + // every row per throttle tick and stutter the drag; the full pass + // runs once, immediately, from `viewDidEndLiveResize`. + if Date().timeIntervalSince(lastLiveResizeRemeasure) > 0.1 { + lastLiveResizeRemeasure = Date() + // Defer one turn: this runs from inside the table's layout() + // pass, and noteHeightOfRows must not re-enter layout. + Task { @MainActor [weak self] in + self?.remeasureVisibleRows() + } + } + return + } - // Defer the height update so live cells settle at the new column width, - // then re-measure. We *invalidate* the visible rows' cached heights and - // let `heightOfRow` recompute them through the measurement host at the - // exact new width, rather than reading a live cell's `fittingSize`. - // During a resize the cell's frame width has already changed but its - // SwiftUI content may not have re-flowed yet, so `fittingSize` still - // reports the pre-resize height; caching that would leave the row too - // short for the now-rewrapped text (it keeps its old height and clips). - // - // Cancel any previously scheduled resize work so that rapid - // resize events (live window drag) coalesce into a single - // update after the last frame change settles. - resizeWorkItem?.cancel() - let work = DispatchWorkItem { [weak self] in - guard let self else { return } - let visible = self.tableView.rows(in: self.tableView.visibleRect) - guard visible.length > 0 else { return } + // A settled, single-shot width change (sidebar toggled or resized, + // scroller appeared, programmatic window resize): coalesce layout + // bursts for one frame, then run the full pass. + resizeRemeasureTask?.cancel() + resizeRemeasureTask = Task { @MainActor [weak self] in + try? await Task.sleep(for: .milliseconds(16)) + guard let self, !Task.isCancelled else { return } + self.remeasureAllRows(reloadCells: true, dropParseCaches: false) + } + } - for idx in visible.lowerBound ..< visible.upperBound where idx < self.rows.count { - self.invalidateHeight(for: self.rows[idx].id) - } + /// Timestamp of the last mid-drag visible-row height pass (throttle). + private var lastLiveResizeRemeasure = Date.distantPast - NSAnimationContext.runAnimationGroup { context in - context.duration = 0 - context.allowsImplicitAnimation = false - self.tableView.noteHeightOfRows( - withIndexesChanged: IndexSet(integersIn: visible.lowerBound ..< visible.upperBound) - ) - } + /// Lightweight height correction for the rows currently on screen, used + /// while a live window-resize drag is in progress. Skips the cell reload + /// (the live cells re-wrap on their own as the column tracks the drag) and + /// leaves off-screen rows to the full pass on drag end. + private func remeasureVisibleRows() { + let visible = tableView.rows(in: tableView.visibleRect) + guard visible.length > 0 else { return } + let wasNearBottom = isNearBottom - // Re-anchor to the bottom so the newest row stays above the - // compose bar after row heights change. - if wasNearBottom { - self.scrollToBottom(animated: false) - } + // The shared host caches its height when only the width proposal + // changes, so it must be rebuilt for the new width. + measurementHost = nil + let upper = min(visible.upperBound, rows.count) + guard visible.lowerBound < upper else { return } + for idx in visible.lowerBound ..< upper { + invalidateHeight(for: rows[idx].id) + } + NSAnimationContext.runAnimationGroup { context in + context.duration = 0 + context.allowsImplicitAnimation = false + tableView.noteHeightOfRows( + withIndexesChanged: IndexSet(integersIn: visible.lowerBound ..< upper) + ) + } + if wasNearBottom { + scrollToBottom(animated: false) + } + } + + @objc private func viewDidResize(_ notification: Notification) { + // Use the scroll view's width: it is current the moment the frame-change + // notification fires, whereas the column width may not have autoresized + // yet. The deferred re-measure reads the (settled) column width. + let newWidth = scrollView.frame.width + guard abs(newWidth - lastColumnWidth) > 1 else { return } + lastColumnWidth = newWidth + + // During a live drag the layout hook already tracks the width change + // (visible rows live, full pass on drag end) — scheduling the full + // pass here too would stutter the drag. + guard !tableView.inLiveResize else { return } + + // NSTableView re-wraps the visible cells live as the column width + // changes; only the row heights lag (they aren't re-queried on a width + // change), leaving the rewrapped text clipped. Coalesce the layout + // burst for one frame, then re-measure. + resizeRemeasureTask?.cancel() + resizeRemeasureTask = Task { @MainActor [weak self] in + try? await Task.sleep(for: .milliseconds(16)) + guard let self, !Task.isCancelled else { return } + self.remeasureAllRows(reloadCells: true, dropParseCaches: false) } - resizeWorkItem = work - DispatchQueue.main.async(execute: work) } // MARK: - Scroll Detection @@ -1024,6 +1145,15 @@ extension TimelineTableViewController: NSTableViewDelegate { var targetWidth = tableView.tableColumns.first?.width ?? 0 if targetWidth < 1 { targetWidth = scrollView.frame.width } if targetWidth < 1 { targetWidth = 600 } + // Measure at the width the cell's SwiftUI content actually lays out at. + // The table ignores safe areas and spans the full window, so the column + // is wider than the visible pane; the overlay sidebar contributes a + // leading safe-area inset that the live NSHostingView cells respect + // when laying out their content, but a detached measurement host knows + // nothing about. Measuring at the full column width proposes a wider + // text wrap than the live cell uses, under-measuring the row height and + // clipping the (more-wrapped, taller) live content top and bottom. + targetWidth -= tableView.safeAreaInsets.left + tableView.safeAreaInsets.right let messageRow = rows[messageIndex] let cacheKey = HeightCacheKey(messageRow.id, targetWidth) @@ -1039,7 +1169,16 @@ extension TimelineTableViewController: NSTableViewDelegate { "heightOfRow" as StaticString, "cache miss: \(messageRow.id.prefix(8))" ) - let rowView = callbacks.makeRowView(messageRow, false, 0, false) + // Pin the row to the exact cell width so the bubble wraps identically to + // the live cell. Without the fixed frame, the row's `maxWidth: .infinity` + // makes `sizeThatFits` return the *ideal* size — the bubble hugging its + // content at a width that can exceed the cell's — which wraps to fewer + // lines and under-measures the height, clipping the live (more-wrapped) + // text's last line and the "edited" label. + let rowView = AnyView( + callbacks.makeRowView(messageRow, false, 0, false) + .frame(width: targetWidth) + ) if let host = measurementHost { host.rootView = rowView } else { diff --git a/RelayTests/TimelineHeightMeasurementTests.swift b/RelayTests/TimelineHeightMeasurementTests.swift index 7e059cd6..b56892bd 100644 --- a/RelayTests/TimelineHeightMeasurementTests.swift +++ b/RelayTests/TimelineHeightMeasurementTests.swift @@ -241,6 +241,31 @@ struct TimelineHeightMeasurementTests { ) } + /// A fresh `NSHostingController` measures a wrapping row taller at a narrower + /// width. The timeline reuses one measurement host for speed, but reusing it + /// *across a width change* returns the previous width's height — an + /// `NSHostingController` re-measures on a content change (why text-zoom works) + /// but not on a bare proposal change. That is why the timeline discards its + /// host before a full re-measure on window resize; this guards the primitive + /// that fix relies on. + @Test + func measurementHostIsWidthSensitiveWhenFresh() { + func measure(width: CGFloat) -> CGFloat { + let host = NSHostingController(rootView: AnyView( + Text("the quick brown fox jumps over the lazy dog again and again " + + "so that this message wraps onto several lines when it is narrow") + .frame(maxWidth: 500, alignment: .leading) + .fixedSize(horizontal: false, vertical: true) + )) + host.sizingOptions = [.standardBounds] + return host.sizeThatFits(in: CGSize(width: width, height: .greatestFiniteMagnitude)).height + } + #expect( + measure(width: 180) > measure(width: 480), + "A narrower width must yield a taller measured row." + ) + } + // MARK: - 4. Link-preview card height determinism /// A variable-height link card must derive its height synchronously from the From 32dd31a2f21f26454705971da4021b11347612d7 Mon Sep 17 00:00:00 2001 From: Lukas Frias Santos Date: Sun, 19 Jul 2026 11:33:57 +0200 Subject: [PATCH 09/11] Keep the timeline responsive during live resize and zoom bursts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Full-timeline reload+re-measure passes are the right end state after a resize or zoom settles, but running one on every intermediate step made a live window drag or a held zoom key feel laggy — each step reloaded and re-measured every row before the next could land. During a live resize drag or a zoom-key burst, do a cheap visible-rows-only refresh immediately (throttled to ~10/s so a drag or repeat-key doesn't trigger a pass every tick), and defer the expensive full-timeline pass until the burst settles: viewDidEndLiveResize for a drag release, a short trailing timer for a zoom burst. Generalized the narrower remeasureVisibleRows() into refreshVisibleRows(reloadCells:) so both paths share the same visible-range/scroll-anchor logic. Co-Authored-By: Claude Sonnet 5 --- Relay/Views/Timeline/TimelineTableView.swift | 63 ++++++++++++++------ 1 file changed, 46 insertions(+), 17 deletions(-) diff --git a/Relay/Views/Timeline/TimelineTableView.swift b/Relay/Views/Timeline/TimelineTableView.swift index d67d8ac7..bd99c71f 100644 --- a/Relay/Views/Timeline/TimelineTableView.swift +++ b/Relay/Views/Timeline/TimelineTableView.swift @@ -910,17 +910,28 @@ final class TimelineTableViewController: NSViewController { // MARK: - Text Zoom - /// Coalesces a burst of text-zoom changes (e.g. holding or repeating ⌘+) - /// into a single re-measure, since re-rendering and re-measuring every row - /// is main-thread work. Chrome already re-renders live via `@AppStorage`; - /// this trailing pass fixes the row heights once the scale settles. + /// Timestamp of the last immediate zoom-step viewport refresh (throttle). + private var lastZoomRefresh = Date.distantPast + + /// Responds to a text-zoom step. The chrome scales on the same frame via + /// `@AppStorage`, so the message text lagging behind it reads as jank: + /// drop the stale-font caches and refresh the *visible* rows immediately + /// (throttled so holding ⌘+ doesn't re-parse the viewport at key-repeat + /// rate). The full-timeline pass — re-parsing and re-measuring every row + /// is main-thread work — runs once, after the burst settles. It drops the + /// parse caches again because a throttle-skipped final step leaves + /// intermediate-scale parses in the cache. @objc private func messageTextScaleDidChange() { + if Date().timeIntervalSince(lastZoomRefresh) > 0.1 { + lastZoomRefresh = Date() + MessageBubbleContent.invalidateParseCaches() + MessageTextView.invalidateSizeCaches() + refreshVisibleRows(reloadCells: true) + } textScaleRemeasureTask?.cancel() textScaleRemeasureTask = Task { @MainActor [weak self] in - try? await Task.sleep(for: .milliseconds(60)) + try? await Task.sleep(for: .milliseconds(250)) guard let self, !Task.isCancelled else { return } - // Zoom changes the font but not the width, so nothing re-renders on - // its own — reload the cells and drop the (now stale-font) parse caches. self.remeasureAllRows(reloadCells: true, dropParseCaches: true) } } @@ -1028,7 +1039,7 @@ final class TimelineTableViewController: NSViewController { // Defer one turn: this runs from inside the table's layout() // pass, and noteHeightOfRows must not re-enter layout. Task { @MainActor [weak self] in - self?.remeasureVisibleRows() + self?.refreshVisibleRows(reloadCells: false) } } return @@ -1048,32 +1059,50 @@ final class TimelineTableViewController: NSViewController { /// Timestamp of the last mid-drag visible-row height pass (throttle). private var lastLiveResizeRemeasure = Date.distantPast - /// Lightweight height correction for the rows currently on screen, used - /// while a live window-resize drag is in progress. Skips the cell reload - /// (the live cells re-wrap on their own as the column tracks the drag) and - /// leaves off-screen rows to the full pass on drag end. - private func remeasureVisibleRows() { + /// Lightweight refresh of just the rows currently on screen, used mid-burst + /// (a live window-resize drag, a text-zoom step) so the viewport responds + /// immediately while the full-timeline pass waits for the burst to end. + /// + /// - Parameter reloadCells: When `true`, reassigns the visible cells' + /// `rootView` so their content re-renders (needed when the *font* changed + /// on a zoom step). A resize drag passes `false`: the live cells re-wrap + /// on their own as the column tracks the drag, and reloading would snap + /// them back to a stale layout mid-drag. + private func refreshVisibleRows(reloadCells: Bool) { let visible = tableView.rows(in: tableView.visibleRect) guard visible.length > 0 else { return } let wasNearBottom = isNearBottom + let anchorRow = visible.location + let anchorOffset = anchorRow >= 0 + ? tableView.rect(ofRow: anchorRow).minY - tableView.visibleRect.minY + : 0 // The shared host caches its height when only the width proposal - // changes, so it must be rebuilt for the new width. + // changes, so it must be rebuilt for the new width/scale. measurementHost = nil let upper = min(visible.upperBound, rows.count) guard visible.lowerBound < upper else { return } + let indexes = IndexSet(integersIn: visible.lowerBound ..< upper) for idx in visible.lowerBound ..< upper { invalidateHeight(for: rows[idx].id) } + if reloadCells { + tableView.reloadData(forRowIndexes: indexes, columnIndexes: IndexSet(integer: 0)) + } NSAnimationContext.runAnimationGroup { context in context.duration = 0 context.allowsImplicitAnimation = false - tableView.noteHeightOfRows( - withIndexesChanged: IndexSet(integersIn: visible.lowerBound ..< upper) - ) + tableView.noteHeightOfRows(withIndexesChanged: indexes) } if wasNearBottom { scrollToBottom(animated: false) + } else if anchorRow >= 0, anchorRow < rows.count { + let targetY = tableView.rect(ofRow: anchorRow).minY - anchorOffset + let current = scrollView.contentView.bounds.origin + if abs(current.y - targetY) > 0.5 { + scrollView.contentView.scroll(to: CGPoint(x: 0, y: targetY)) + scrollView.reflectScrolledClipView(scrollView.contentView) + } } } From d45d66dcd783f8718ad2a9be9f4286295b0eddcf Mon Sep 17 00:00:00 2001 From: Lukas Frias Santos Date: Sun, 19 Jul 2026 12:01:35 +0200 Subject: [PATCH 10/11] Fix style/comment inconsistencies flagged in review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - RelayTests/TimelineHeightMeasurementTests.swift: collapse each `@Test` attribute onto the same line as its `func`, matching the single-line form used throughout MatrixHTMLParserTests.swift (the only other Swift Testing suite in the repo). - TimelineTableView.swift: drop the stale doc-comment line left over from resizeWorkItem (the prior DispatchWorkItem-based debounce) that was stacked above its Task-based replacement, resizeRemeasureTask. - TimelineTableView.swift: fix measurementHost's doc comment, which still claimed the concrete TimelineRowView type avoided AnyView type-erasure overhead — stale since the property's type was changed to NSHostingController to support pinning a measured row's width. Co-Authored-By: Claude Sonnet 5 --- Relay/Views/Timeline/TimelineTableView.swift | 7 +++-- .../TimelineHeightMeasurementTests.swift | 27 +++++++------------ 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/Relay/Views/Timeline/TimelineTableView.swift b/Relay/Views/Timeline/TimelineTableView.swift index bd99c71f..2ca943ca 100644 --- a/Relay/Views/Timeline/TimelineTableView.swift +++ b/Relay/Views/Timeline/TimelineTableView.swift @@ -288,7 +288,6 @@ final class TimelineTableViewController: NSViewController { /// ``viewDidLayout`` watches it so those changes still trigger a re-measure. private var lastRenderWidth: CGFloat = 0 - /// Coalesces rapid resize events so only the final one runs. /// Coalesces a live window-resize drag into one re-measure once it settles. private var resizeRemeasureTask: Task? /// Coalesces bursts of text-zoom changes into one re-measure pass. @@ -296,9 +295,9 @@ final class TimelineTableViewController: NSViewController { /// A reusable hosting controller used to measure SwiftUI row heights /// for rows that don't have a live cell on screen. Only used as a - /// fallback when no cached height exists. Using the concrete - /// ``TimelineRowView`` type avoids `AnyView` type-erasure overhead - /// and lets SwiftUI reuse the internal view hierarchy between measurements. + /// fallback when no cached height exists. Wrapped in `AnyView` so the + /// measured row can be pinned to a fixed `.frame(width:)` matching the + /// live cell's wrap width. private var measurementHost: NSHostingController? /// Caches measured row heights keyed on `(messageID, roundedWidth)`. diff --git a/RelayTests/TimelineHeightMeasurementTests.swift b/RelayTests/TimelineHeightMeasurementTests.swift index b56892bd..951c28cd 100644 --- a/RelayTests/TimelineHeightMeasurementTests.swift +++ b/RelayTests/TimelineHeightMeasurementTests.swift @@ -114,8 +114,7 @@ struct TimelineHeightMeasurementTests { /// Capping the attachment to the line box keeps a pill line the same height /// as a normal text line. (TextKit "grow-and-shift" keeps the pill inside the /// used rect either way, so line growth — not a draw-overhang — is the defect.) - @Test - func mentionPillDoesNotGrowLineHeight() { + @Test func mentionPillDoesNotGrowLineHeight() { // The same short message on one line (600pt-wide container) as plain // text and with a leading mention pill. let plain = layout( @@ -141,8 +140,7 @@ struct TimelineHeightMeasurementTests { /// Direct unit invariant: a pill's attachment bounds must fit inside the /// font line box (top ≤ ascender, bottom ≥ descender) so it can never /// overhang whatever line it lands on. - @Test - func pillAttachmentBoundsFitWithinFontLineBox() { + @Test func pillAttachmentBoundsFitWithinFontLineBox() { let pill = PillTextAttachment( userId: "@sample:matrix.org", displayName: "Sample User", font: baseFont, style: .messageDefault @@ -170,8 +168,7 @@ struct TimelineHeightMeasurementTests { /// sized for a larger font, TextKit upscales the small bitmap into the large /// bounds and the capsule reads as stretched/blurry — the defect that surfaces /// once the timeline text-zoom enlarges the message font. - @Test - func mentionPillContentScalesWithFontSize() { + @Test func mentionPillContentScalesWithFontSize() { func renderedPixelHeight(fontSize: CGFloat) -> Int { let view = MentionPillView( displayName: "Sample User", style: .messageDefault, fontSize: fontSize @@ -198,8 +195,7 @@ struct TimelineHeightMeasurementTests { /// /// Here we assert the measured height (at `floor(w)`) fully contains the text /// laid out at that same render width across a sweep of fractional widths. - @Test - func textHeightMeasuredAtRenderWidthAcrossFractionalWidths() { + @Test func textHeightMeasuredAtRenderWidthAcrossFractionalWidths() { let body = "the quick brown fox jumps over the lazy dog again " + "and again to make this message wrap onto several lines" let attributed = NSAttributedString(matrixMarkdown: body) @@ -228,8 +224,7 @@ struct TimelineHeightMeasurementTests { /// width. A message that wraps must report a *taller* height at a narrower /// width — the regression guard for rows keeping their old (too-short) /// height after a resize and clipping the re-wrapped text. - @Test - func wrappingMessageHeightGrowsAsWidthShrinks() { + @Test func wrappingMessageHeightGrowsAsWidthShrinks() { let body = "the quick brown fox jumps over the lazy dog again and again " + "so that this message must wrap onto several lines when it is narrow" let attributed = NSAttributedString(matrixMarkdown: body) @@ -248,8 +243,7 @@ struct TimelineHeightMeasurementTests { /// but not on a bare proposal change. That is why the timeline discards its /// host before a full re-measure on window resize; this guards the primitive /// that fix relies on. - @Test - func measurementHostIsWidthSensitiveWhenFresh() { + @Test func measurementHostIsWidthSensitiveWhenFresh() { func measure(width: CGFloat) -> CGFloat { let host = NSHostingController(rootView: AnyView( Text("the quick brown fox jumps over the lazy dog again and again " @@ -271,8 +265,7 @@ struct TimelineHeightMeasurementTests { /// A variable-height link card must derive its height synchronously from the /// shared card cache, so the detached measurement host (whose async image /// load never runs) computes the same card height the live cell renders. - @Test - func linkCardHeightIsDeterministicFromCache() { + @Test func linkCardHeightIsDeterministicFromCache() { let url = URL(string: "https://example.com/deterministic-\(UUID().uuidString)")! // Unresolved: placeholder height. @@ -304,8 +297,7 @@ struct TimelineHeightMeasurementTests { } /// An unavailable link resolves to a hidden (zero-height) card. - @Test - func unavailableLinkCardIsHidden() { + @Test func unavailableLinkCardIsHidden() { let url = URL(string: "https://example.com/gone-\(UUID().uuidString)")! LinkPreviewView.cardCache.set(.unavailable, forKey: url) let height = measuredHeight( @@ -316,8 +308,7 @@ struct TimelineHeightMeasurementTests { /// A compact (favicon/globe) card has a fixed, deterministic height distinct /// from a hidden card. - @Test - func compactLinkCardHeightIsFixed() { + @Test func compactLinkCardHeightIsFixed() { let url = URL(string: "https://example.com/compact-\(UUID().uuidString)")! LinkPreviewView.cardCache.set(.compact, forKey: url) let a = measuredHeight( From 7a33639cda57ad9e3648253bda0a2f62bf26d512 Mon Sep 17 00:00:00 2001 From: Lukas Frias Santos Date: Sun, 19 Jul 2026 18:24:41 +0200 Subject: [PATCH 11/11] Fix code-review findings: perf, correctness, and test coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Performance: - remeasureAllRows() cleared the height cache for every loaded row and eagerly re-measured all of them (via a full noteHeightOfRows pass) on every resize/zoom settle, not just the visible ones — a regression from main's visible-range-only behavior. In a heavily-paginated room this forced a synchronous NSHostingController layout pass per off-screen row on the main thread. Scope the eager pass to refreshVisibleRows() (the same path live-resize-drag and zoom-throttle already use); off-screen rows are left with a cleared cache entry, not a stale one, and measure fresh at the new width the moment they're actually about to be shown. - invalidateHeight(for:) rebuilt the whole heightCache dict via .filter (O(n)) on every remeasureRow/resize/zoom call. Add a per-message reverse index of cached widths so invalidation only touches the affected entries. - ParseCache's LRU recency tracking used an array + firstIndex/remove(at:) (O(n)). Rewrite it as a proper doubly-linked-list LRU so get/set/evict are all O(1). - Factor the three independent hand-rolled cancel/sleep/isCancelled debounce blocks (resize ×2, text-zoom, row-remeasure-flush) into one `debounce(_:milliseconds:action:)` helper, and the two duplicated scroll-anchor preserve/restore implementations into one `preservingScrollAnchor(_:)`. Correctness: - LinkPreviewView.resolve() decided whether to trigger a remeasure by checking the *shared, URL-keyed* card cache's previous value instead of this row's own prior state — so a second message sharing a URL with an already-resolved message never got remeasured and stayed clipped at placeholder height. Compare against the instance's own state instead. - heightOfRow's safe-area-inset subtraction had no floor, unlike the sibling width computation in scheduleRemeasureIfEffectiveWidthChanged — a narrow window with the sidebar open could drive it to zero/negative. Extracted the shared arithmetic into a testable `effectiveContentWidth(columnWidth:safeAreaInsets:)` and floor the result at 1pt before measuring. - emoteParsedBody's markdown-fallback branch still built the italic sender-name prefix at a fixed, unscaled system font size while the HTML branch a few lines above had been migrated to MessageTextScale.baseFont — a zoomed /me message with no formatted_body rendered a mismatched name size. - ComposeInputTextView's initial cachedHeight was computed from the unscaled font before the persisted zoom-scaled font was applied, so launching at a non-default zoom rendered the compose bar too short until the first keystroke. - LinkPreviewView fell back to fetching the favicon whenever the OG image failed to load or decoded to a degenerate size, not just when no image was offered at all — letting a page force a second, potentially different-origin fetch merely by serving a broken og:image. Restored the original "only fall back when absent" semantics. Architecture (nits): - MessageTextScale.apply() called MessageBubbleContent.invalidateParseCaches() directly, a Utilities→Views reach that duplicated the invalidation TimelineTableView already performs in response to the same notification. Removed; cache invalidation is left entirely to the observer that owns the cache. - ScaledChromeFont read the persisted scale through @AppStorage without clamping it, unlike MessageTextScale.scale. Added a shared MessageTextScale.clamp(_:) both go through. - Fixed a doc comment left over from a prior implementation of invalidateParseCaches() that no longer matched its callers. Tests: - Add ParseCacheTests: LRU eviction under sustained pressure, recency promotion via value(forKey:)/set(_:forKey:), peek()'s no-promote contract, removeAll() resetting recency bookkeeping. - Add MessageTextScaleTests: increase/decrease/reset clamping and symmetry, the no-op-when-unchanged notification guard, baseFontSize/ baseFont tracking scale. Redirects MessageTextScale to a private, throwaway UserDefaults suite (MessageTextScale.userDefaults is now injectable) since this is the one suite that *mutates* the scale, and RelayTests shares UserDefaults.standard with the app. - Fixed a real, confirmed test-isolation bug: MatrixHTMLParserTests and TimelineHeightMeasurementTests read MessageTextScale-derived sizes without resetting the persisted scale first, so headingFontSizes() could intermittently fail (reproduced: read a scale of 2.4 mid-run) whenever it happened to run concurrently with a scale-mutating test, or after a developer had zoomed the app during manual testing in the same container. Both suites now reset the (real, shared) UserDefaults key in init(). - Add regression tests for the new effectiveContentWidth helper, including the case that motivated the floor fix (insets exceeding the column). Verified: 99/99 tests pass across 5 consecutive runs (confirming the fixed race), and manually exercised live window-resize (both directions) and text-zoom in the built app with no clipping or crashes. Co-Authored-By: Claude Sonnet 5 --- Relay/Utilities/MessageTextScale.swift | 37 ++- Relay/Utilities/ParseCache.swift | 104 +++++-- Relay/Views/Compose/ComposeBar.swift | 2 +- Relay/Views/Compose/ComposeTextView.swift | 7 +- Relay/Views/Media/LinkPreviewView.swift | 29 +- Relay/Views/Message/MessageBodyParser.swift | 15 +- .../Views/Message/MessageBubbleContent.swift | 2 +- Relay/Views/Timeline/TimelineTableView.swift | 255 ++++++++++-------- RelayTests/MatrixHTMLParserTests.swift | 9 + RelayTests/MessageTextScaleTests.swift | 109 ++++++++ RelayTests/ParseCacheTests.swift | 123 +++++++++ .../TimelineHeightMeasurementTests.swift | 46 ++++ 12 files changed, 572 insertions(+), 166 deletions(-) create mode 100644 RelayTests/MessageTextScaleTests.swift create mode 100644 RelayTests/ParseCacheTests.swift diff --git a/Relay/Utilities/MessageTextScale.swift b/Relay/Utilities/MessageTextScale.swift index 26327064..feeeee75 100644 --- a/Relay/Utilities/MessageTextScale.swift +++ b/Relay/Utilities/MessageTextScale.swift @@ -28,6 +28,15 @@ enum MessageTextScale { /// `UserDefaults` key holding the scale factor as a `Double`. nonisolated static let userDefaultsKey = "timeline.textScale" + /// The `UserDefaults` suite the scale is persisted to and read from. + /// Defaults to `.standard`. A fully-serialized test suite that exercises + /// ``increase()``/``decrease()``/``reset()`` may override this to a + /// private, throwaway suite so its writes can't race with any other test + /// (or the app's own persisted value) reading `.standard` concurrently — + /// `UserDefaults` itself is thread-safe, but swapping *which store* + /// every reader/writer here uses is not. + nonisolated(unsafe) static var userDefaults: UserDefaults = .standard + /// Posted after the scale changes and the parse caches are cleared. static let didChangeNotification = Notification.Name("relay.messageTextScaleDidChange") @@ -44,9 +53,17 @@ enum MessageTextScale { /// `nonisolated` so message parsing can read it off the main actor; the /// backing `UserDefaults` read is itself thread-safe. nonisolated static var scale: CGFloat { - let stored = UserDefaults.standard.object(forKey: userDefaultsKey) as? Double + let stored = userDefaults.object(forKey: userDefaultsKey) as? Double let value = stored.map { CGFloat($0) } ?? defaultScale - return min(max(value, minScale), maxScale) + return clamp(value) + } + + /// Clamps a raw scale value to `[minScale, maxScale]`. Shared by ``scale`` + /// and ``ScaledChromeFont`` so both apply the same bound even though the + /// latter reads the persisted value through `@AppStorage` (for SwiftUI + /// reactivity) rather than through ``scale`` itself. + nonisolated static func clamp(_ value: CGFloat) -> CGFloat { + min(max(value, minScale), maxScale) } /// The base message/compose font point size at the current scale. @@ -63,14 +80,16 @@ enum MessageTextScale { @MainActor static func decrease() { apply(scale - step) } @MainActor static func reset() { apply(defaultScale) } - /// Persists `newValue` (clamped), invalidates the caches that hold text laid - /// out at the old size, and notifies observers. A no-op when the clamped - /// value is unchanged, so hitting the limit doesn't churn the timeline. + /// Persists `newValue` (clamped) and notifies observers. A no-op when the + /// clamped value is unchanged, so hitting the limit doesn't churn the + /// timeline. Cache invalidation is left to the observer that owns the + /// affected cache (``TimelineTableViewController``) rather than done here, + /// so this Utilities-layer type doesn't need to know about a specific + /// Views-layer cache. @MainActor private static func apply(_ newValue: CGFloat) { - let clamped = min(max(newValue, minScale), maxScale) + let clamped = clamp(newValue) guard abs(clamped - scale) > 0.001 else { return } - UserDefaults.standard.set(Double(clamped), forKey: userDefaultsKey) - MessageBubbleContent.invalidateParseCaches() + userDefaults.set(Double(clamped), forKey: userDefaultsKey) NotificationCenter.default.post(name: didChangeNotification, object: nil) } } @@ -85,7 +104,7 @@ private struct ScaledChromeFont: ViewModifier { func body(content: Content) -> some View { let base = NSFont.preferredFont(forTextStyle: textStyle).pointSize - content.font(.system(size: base * CGFloat(scale), weight: weight)) + content.font(.system(size: base * MessageTextScale.clamp(CGFloat(scale)), weight: weight)) } } diff --git a/Relay/Utilities/ParseCache.swift b/Relay/Utilities/ParseCache.swift index 219ff659..cf2926cf 100644 --- a/Relay/Utilities/ParseCache.swift +++ b/Relay/Utilities/ParseCache.swift @@ -17,11 +17,30 @@ import Foundation /// A simple LRU cache for expensive parse results (HTML, Markdown, URL detection). /// /// Thread-safe via `NSLock`. Designed for main-thread hot paths where the same -/// content is re-parsed on every SwiftUI body evaluation. +/// content is re-parsed on every SwiftUI body evaluation. Backed by a +/// dictionary of doubly-linked-list nodes so every operation (get, set, +/// recency promotion, eviction) is O(1) rather than scanning a recency array. final class ParseCache: @unchecked Sendable { + /// `prev` is `weak` so the list's only strong ownership chain runs + /// `head -> next -> ... -> tail`; dropping a node from `nodes` and + /// unlinking it from that chain lets ARC deallocate it immediately, + /// instead of the two directions retaining each other forever. + private final class Node { + let key: Key + var value: Value + weak var prev: Node? + var next: Node? + + init(key: Key, value: Value) { + self.key = key + self.value = value + } + } + private let capacity: Int - private var storage: [Key: Value] = [:] - private var order: [Key] = [] + private var nodes: [Key: Node] = [:] + private var head: Node? + private var tail: Node? private let lock = NSLock() init(capacity: Int) { @@ -31,11 +50,9 @@ final class ParseCache: @unchecked Sendable { /// Returns the cached value for `key`, or computes and caches it using `compute`. func value(forKey key: Key, compute: () -> Value) -> Value { lock.lock() - if let cached = storage[key] { - // Move to end (most recently used). - if let idx = order.firstIndex(of: key) { - order.append(order.remove(at: idx)) - } + if let node = nodes[key] { + moveToFront(node) + let cached = node.value lock.unlock() return cached } @@ -44,25 +61,26 @@ final class ParseCache: @unchecked Sendable { let result = compute() lock.lock() - storage[key] = result - order.append(key) - if order.count > capacity { - let evicted = order.removeFirst() - storage.removeValue(forKey: evicted) + defer { lock.unlock() } + // A concurrent caller may have inserted this key while `compute()` + // ran unlocked; keep the existing value (first writer wins) rather + // than inserting a second node for the same key. + if let existing = nodes[key] { + moveToFront(existing) + return existing.value } - lock.unlock() - + insert(key: key, value: result) return result } /// Returns the cached value for `key` without computing or promoting it — /// an O(1) read safe to call from hot paths such as SwiftUI `body`. Recency - /// is updated only by ``set(_:_:)``, which suffices for caches that write on - /// resolution. Returns `nil` on a miss. + /// is updated only by ``set(_:forKey:)``, which suffices for caches that + /// write on resolution. Returns `nil` on a miss. func peek(_ key: Key) -> Value? { lock.lock() defer { lock.unlock() } - return storage[key] + return nodes[key]?.value } /// Removes every cached entry. Used when a global input the cached values @@ -71,8 +89,9 @@ final class ParseCache: @unchecked Sendable { func removeAll() { lock.lock() defer { lock.unlock() } - storage.removeAll() - order.removeAll() + nodes.removeAll() + head = nil + tail = nil } /// Stores `value` for `key`, evicting the least-recently-used entry when the @@ -80,15 +99,44 @@ final class ParseCache: @unchecked Sendable { func set(_ value: Value, forKey key: Key) { lock.lock() defer { lock.unlock() } - if storage[key] == nil { - order.append(key) - } else if let idx = order.firstIndex(of: key) { - order.append(order.remove(at: idx)) + if let node = nodes[key] { + node.value = value + moveToFront(node) + } else { + insert(key: key, value: value) } - storage[key] = value - if order.count > capacity { - let evicted = order.removeFirst() - storage.removeValue(forKey: evicted) + } + + // MARK: - Linked-list bookkeeping (call only while holding `lock`) + + private func insert(key: Key, value: Value) { + let node = Node(key: key, value: value) + nodes[key] = node + node.next = head + head?.prev = node + head = node + if tail == nil { tail = node } + evictIfNeeded() + } + + private func moveToFront(_ node: Node) { + guard head !== node else { return } + node.prev?.next = node.next + node.next?.prev = node.prev + if tail === node { tail = node.prev } + node.prev = nil + node.next = head + head?.prev = node + head = node + if tail == nil { tail = node } + } + + private func evictIfNeeded() { + while nodes.count > capacity, let evicted = tail { + nodes.removeValue(forKey: evicted.key) + tail = evicted.prev + tail?.next = nil + if head === evicted { head = nil } } } } diff --git a/Relay/Views/Compose/ComposeBar.swift b/Relay/Views/Compose/ComposeBar.swift index 80e5ee41..837b50c7 100644 --- a/Relay/Views/Compose/ComposeBar.swift +++ b/Relay/Views/Compose/ComposeBar.swift @@ -104,7 +104,7 @@ struct ComposeBar: View { private struct ComposeBarContent: View { @Bindable var compose: ComposeViewModel @Binding var mentionSuggestionsHeight: CGFloat - @State private var textViewHeight: CGFloat = NSFont.systemFontSize * 1.2 + 20 + @State private var textViewHeight: CGFloat = MessageTextScale.baseFontSize * 1.2 + 20 var onSend: () async -> Void var body: some View { diff --git a/Relay/Views/Compose/ComposeTextView.swift b/Relay/Views/Compose/ComposeTextView.swift index 43a4406c..3de0cfb1 100644 --- a/Relay/Views/Compose/ComposeTextView.swift +++ b/Relay/Views/Compose/ComposeTextView.swift @@ -87,6 +87,11 @@ struct ComposeTextView: NSViewRepresentable { self.insertMentionHandler = { [weak coordinator] userId, displayName in coordinator?.insertMention(userId: userId, displayName: displayName) } + // cachedHeight's stored-property default is seeded at the unscaled + // system font size; recompute it now that `font` is set to the + // persisted zoom scale so a launch at a non-default zoom doesn't + // report a too-short initial height. + textView.recalculateHeight() let heightCallback = onHeightChange let initialHeight = textView.cachedHeight Task { @MainActor in @@ -567,7 +572,7 @@ final class ComposeInputTextView: NSTextView { /// Cached content height, updated after every text change. /// Initialized to a sensible single-line height (lineHeight + insets). - private(set) var cachedHeight: CGFloat = NSFont.systemFontSize * 1.2 + 20 + private(set) var cachedHeight: CGFloat = MessageTextScale.baseFontSize * 1.2 + 20 private var isRecalculatingHeight = false override var intrinsicContentSize: NSSize { diff --git a/Relay/Views/Media/LinkPreviewView.swift b/Relay/Views/Media/LinkPreviewView.swift index 043dfd68..da63dbcc 100644 --- a/Relay/Views/Media/LinkPreviewView.swift +++ b/Relay/Views/Media/LinkPreviewView.swift @@ -261,12 +261,20 @@ struct LinkPreviewView: View { title = metadata.title // A real Open-Graph image drives a full-bleed banner card. Otherwise fall - // back to a compact card showing the favicon (or a globe if none). - if let imageProvider = metadata.imageProvider, - let loaded = await loadImage(from: imageProvider), - loaded.size.width > 0, loaded.size.height > 0 { - image = loaded - resolve(.banner(aspect: loaded.size.width / loaded.size.height)) + // back to a compact card showing the favicon (or a globe if none). Only + // fall back to the favicon when no banner image was offered at all — + // not merely because the offered one failed to load or decoded to a + // degenerate size — so a page can't force a second, potentially + // different-origin fetch just by serving a broken `og:image`. + if let imageProvider = metadata.imageProvider { + if let loaded = await loadImage(from: imageProvider), + loaded.size.width > 0, loaded.size.height > 0 { + image = loaded + resolve(.banner(aspect: loaded.size.width / loaded.size.height)) + } else { + image = nil + resolve(.compact) + } } else if let iconProvider = metadata.iconProvider, let icon = await loadImage(from: iconProvider), icon.size.width > 0, icon.size.height > 0 { @@ -281,7 +289,12 @@ struct LinkPreviewView: View { /// Publishes the resolved card to the shared cache and this instance, and /// re-measures the row when the resolved *height* can change. private func resolve(_ resolved: LinkPreviewCard) { - let previous = Self.cardCache.peek(url) + // Compare against this instance's own prior state, not the shared, + // URL-keyed cache: two rows referencing the same URL both resolve + // independently, and the cache's previous value may already have + // been overwritten by a sibling row's resolution — comparing against + // it would skip this row's own placeholder-to-final remeasure. + let instancePrevious = card Self.cardCache.set(resolved, forKey: url) card = resolved // Re-measure on any height-changing transition: the first resolution, a @@ -289,7 +302,7 @@ struct LinkPreviewView: View { // image-load failure), or a changed banner aspect. Re-resolving to the // same card — including a globe→favicon swap, which keeps the compact // height — needs no re-measure. - if previous != resolved { + if instancePrevious != resolved { actions.remeasureRow?(messageID) } } diff --git a/Relay/Views/Message/MessageBodyParser.swift b/Relay/Views/Message/MessageBodyParser.swift index db8decde..62468352 100644 --- a/Relay/Views/Message/MessageBodyParser.swift +++ b/Relay/Views/Message/MessageBodyParser.swift @@ -29,12 +29,15 @@ extension MessageBubbleContent { /// Drops every message parse cache. /// - /// Call when a global change must force every row to re-render from scratch: - /// a text-zoom step, or a window resize (recycled cells otherwise keep a - /// stale text-container width and clip). Because the caches key by content, - /// the returned attributed string is a *new* instance, which makes - /// ``MessageTextView``'s `updateNSView` re-resolve and re-sync its container - /// to the current width instead of early-returning on an unchanged instance. + /// Call when a global change must force every row to re-render from + /// scratch — a text-zoom step, where the cached attributed strings were + /// built at the old font size. A window resize does *not* need this: the + /// text container's stale-width problem is handled separately, by + /// ``MessageTextView``'s size-cache generation counter. Because the + /// caches key by content, the returned attributed string is a *new* + /// instance, which makes ``MessageTextView``'s `updateNSView` re-resolve + /// and re-sync its container to the current width instead of + /// early-returning on an unchanged instance. @MainActor static func invalidateParseCaches() { htmlCache.removeAll() diff --git a/Relay/Views/Message/MessageBubbleContent.swift b/Relay/Views/Message/MessageBubbleContent.swift index 6b0a714d..f16aecd0 100644 --- a/Relay/Views/Message/MessageBubbleContent.swift +++ b/Relay/Views/Message/MessageBubbleContent.swift @@ -288,7 +288,7 @@ struct MessageBubbleContent: View { if let result = cached { return result } } // Markdown fallback with italic name prefix. - let nameFont = NSFont.systemFont(ofSize: NSFont.systemFontSize) + let nameFont = MessageTextScale.baseFont let italicDesc = nameFont.fontDescriptor.withSymbolicTraits(.italic) let italicFont = NSFont(descriptor: italicDesc, size: nameFont.pointSize) ?? nameFont let result = NSMutableAttributedString( diff --git a/Relay/Views/Timeline/TimelineTableView.swift b/Relay/Views/Timeline/TimelineTableView.swift index 2ca943ca..84f5bf4e 100644 --- a/Relay/Views/Timeline/TimelineTableView.swift +++ b/Relay/Views/Timeline/TimelineTableView.swift @@ -198,6 +198,18 @@ final class TimelineTableViewController: NSViewController { enum Section { case main } + /// The column width minus horizontal safe-area insets — the width the + /// cell's SwiftUI content actually renders and wraps at (the overlay + /// sidebar contributes a leading inset the live cells respect). Pure and + /// unit-testable without a live `NSTableView`; callers apply their own + /// clamp/guard semantics on the result (``heightOfRow`` floors it since + /// it must return *some* height regardless; + /// ``scheduleRemeasureIfEffectiveWidthChanged()`` skips entirely below a + /// threshold rather than measuring at a near-zero width). + static func effectiveContentWidth(columnWidth: CGFloat, safeAreaInsets: NSEdgeInsets) -> CGFloat { + columnWidth - safeAreaInsets.left - safeAreaInsets.right + } + /// Callbacks from the table view controller back to the SwiftUI layer. struct Callbacks { var onNearBottomChanged: (Bool) -> Void = { _ in } @@ -305,6 +317,11 @@ final class TimelineTableViewController: NSViewController { /// resize, scroll, and content-only updates. private var heightCache: [HeightCacheKey: CGFloat] = [:] + /// Reverse index from message ID to the (rounded) widths cached for it, + /// so ``invalidateHeight(for:)`` can remove exactly the affected entries + /// in O(1) instead of scanning the whole cache. + private var cachedWidthsByMessageID: [String: Set] = [:] + private struct HeightCacheKey: Hashable { let messageID: String let width: CGFloat @@ -392,7 +409,7 @@ final class TimelineTableViewController: NSViewController { // The drag is over — settle the whole timeline now rather than // waiting out a debounce, so the release doesn't feel laggy. self.resizeRemeasureTask?.cancel() - self.remeasureAllRows(reloadCells: true, dropParseCaches: false) + self.remeasureAllRows(dropParseCaches: false) } scrollView.documentView = tableView @@ -818,15 +835,14 @@ final class TimelineTableViewController: NSViewController { /// Removes all cached heights for the given message ID (at any width). private func invalidateHeight(for messageID: String) { - let beforeCount = heightCache.count - heightCache = heightCache.filter { $0.key.messageID != messageID } - let removed = beforeCount - heightCache.count - if removed > 0 { - Self.perfSignposter.emitEvent( - "invalidateHeight" as StaticString, - "\(messageID.prefix(8)): removed \(removed) from \(beforeCount) entries" - ) + guard let widths = cachedWidthsByMessageID.removeValue(forKey: messageID), !widths.isEmpty else { return } + for width in widths { + heightCache.removeValue(forKey: HeightCacheKey(messageID, width)) } + Self.perfSignposter.emitEvent( + "invalidateHeight" as StaticString, + "\(messageID.prefix(8)): removed \(widths.count) entries" + ) } /// Message IDs awaiting a debounced height re-measure. @@ -854,11 +870,8 @@ final class TimelineTableViewController: NSViewController { // re-render, and so several rows changing in the same window (e.g. // multiple link-preview cards resolving at once) collapse into a single // height pass instead of one per row. - remeasureDebounceTask?.cancel() - remeasureDebounceTask = Task { @MainActor [weak self] in - try? await Task.sleep(for: .milliseconds(16)) - guard let self, !Task.isCancelled else { return } - self.flushPendingRemeasures() + debounce(&remeasureDebounceTask, milliseconds: 16) { [weak self] in + self?.flushPendingRemeasures() } // Max-wait, anchored to the first queued request: a continuous stream of // calls can't keep resetting the trailing timer past this bound. @@ -891,19 +904,60 @@ final class TimelineTableViewController: NSViewController { } guard !indices.isEmpty else { return } - let scrollBefore = scrollView.contentView.bounds.origin - NSAnimationContext.runAnimationGroup { context in - context.duration = 0 - context.allowsImplicitAnimation = false - tableView.noteHeightOfRows(withIndexesChanged: indices) + preservingScrollAnchor { + NSAnimationContext.runAnimationGroup { context in + context.duration = 0 + context.allowsImplicitAnimation = false + tableView.noteHeightOfRows(withIndexesChanged: indices) + } + } + } + + // MARK: - Debounce and Scroll-Anchor Helpers + + /// Cancels `task`, then schedules a new one that runs `action` after + /// `milliseconds` unless superseded by a later call before it fires — the + /// common shape behind every trailing debounce in this controller + /// (resize, text-zoom, and this row-remeasure flush). + private func debounce( + _ task: inout Task?, + milliseconds: Int, + action: @escaping @MainActor () -> Void + ) { + task?.cancel() + task = Task { @MainActor in + try? await Task.sleep(for: .milliseconds(milliseconds)) + guard !Task.isCancelled else { return } + action() } - // Preserve the scroll position; growing a row above the viewport would - // otherwise shift the visible content. - if isNearBottom { + } + + /// Captures the row at the top of the viewport (or whether the viewport + /// was pinned to the bottom), runs `body` — expected to change one or + /// more rows' heights — then restores the same content to the same + /// on-screen position. Every height change that can shift the + /// surrounding layout (a resize, a zoom, a burst of `remeasureRow` calls) + /// routes through this so there's a single implementation of "keep + /// what's on screen, on screen", instead of each call site re-deriving + /// it slightly differently. + private func preservingScrollAnchor(_ body: () -> Void) { + let wasNearBottom = isNearBottom + let anchorRow = tableView.rows(in: tableView.visibleRect).location + let anchorOffset = anchorRow >= 0 + ? tableView.rect(ofRow: anchorRow).minY - tableView.visibleRect.minY + : 0 + + body() + + if wasNearBottom { scrollToBottom(animated: false) - } else if abs(scrollBefore.y - scrollView.contentView.bounds.origin.y) > 0.5 { - scrollView.contentView.scroll(to: scrollBefore) - scrollView.reflectScrolledClipView(scrollView.contentView) + } else if anchorRow >= 0, anchorRow < rows.count { + let targetY = tableView.rect(ofRow: anchorRow).minY - anchorOffset + let current = scrollView.contentView.bounds.origin + if abs(current.y - targetY) > 0.5 { + scrollView.contentView.scroll(to: CGPoint(x: 0, y: targetY)) + scrollView.reflectScrolledClipView(scrollView.contentView) + } } } @@ -927,43 +981,40 @@ final class TimelineTableViewController: NSViewController { MessageTextView.invalidateSizeCaches() refreshVisibleRows(reloadCells: true) } - textScaleRemeasureTask?.cancel() - textScaleRemeasureTask = Task { @MainActor [weak self] in - try? await Task.sleep(for: .milliseconds(250)) - guard let self, !Task.isCancelled else { return } - self.remeasureAllRows(reloadCells: true, dropParseCaches: true) + debounce(&textScaleRemeasureTask, milliseconds: 250) { [weak self] in + self?.remeasureAllRows(dropParseCaches: true) } } - /// Re-measures every row and applies the new heights, anchoring the first - /// visible row so the viewport keeps the same content. + /// Invalidates every row's cached height and re-measures the rows + /// currently on screen, anchoring the viewport so it keeps the same + /// content after the pass. /// - /// - Parameters: - /// - reloadCells: When `true`, reassigns every recycled cell's `rootView` - /// so its content re-lays-out from scratch. Needed on a width change: - /// NSTableView re-wraps visible cells live as the column changes, but a - /// recycled `MessageTextView` can be left wrapping at a stale (narrower) - /// width from mid-drag, rendering an extra line that eats the bubble - /// padding and clips top/bottom. A fresh render at the settled width - /// restores correct wrapping. (Cheap: it re-renders, it does not re-parse.) - /// - dropParseCaches: When `true`, also drops the parsed-text caches — only - /// needed when the text *content/font* changed (a text-zoom step), not on - /// a resize, where re-parsing every message would needlessly churn the - /// main thread since parsing is width-independent. + /// - Parameter dropParseCaches: When `true`, also drops the parsed-text + /// caches — only needed when the text *content/font* changed (a + /// text-zoom step), not on a resize, where re-parsing every message + /// would needlessly churn the main thread since parsing is + /// width-independent. + /// + /// Every row's height-cache entry is cleared (a cheap dictionary reset), + /// but only the *visible* rows are eagerly re-measured and re-noted to + /// `NSTableView` — via ``refreshVisibleRows(reloadCells:)``, the same + /// path the live-resize-drag and zoom-throttle passes already use. A row + /// that's scrolled out of view is left with no cached entry, not a stale + /// one: `heightOfRow` cache-misses and measures it fresh, at the new + /// width, the moment it's actually about to be displayed. Eagerly + /// forcing an `NSHostingController.sizeThatFits` pass for every loaded + /// row — including thousands scrolled out of view in a long-paginated + /// room — on every resize/zoom settle would block the main thread for + /// work nothing on screen needs yet. /// /// The shared measurement host is discarded regardless: an /// `NSHostingController` re-measures on a content change but returns the /// previous height when only the width *proposal* changes, so reusing it /// across a resize would keep the old height. A fresh host measures at the /// new width. - private func remeasureAllRows(reloadCells: Bool, dropParseCaches: Bool) { + private func remeasureAllRows(dropParseCaches: Bool) { guard !rows.isEmpty else { return } - let wasNearBottom = isNearBottom - let anchorRow = tableView.rows(in: tableView.visibleRect).location - let anchorOffset = anchorRow >= 0 - ? tableView.rect(ofRow: anchorRow).minY - tableView.visibleRect.minY - : 0 - if dropParseCaches { MessageBubbleContent.invalidateParseCaches() } @@ -972,24 +1023,8 @@ final class TimelineTableViewController: NSViewController { MessageTextView.invalidateSizeCaches() measurementHost = nil heightCache.removeAll() - let all = IndexSet(integersIn: 0 ..< rows.count) - if reloadCells { - tableView.reloadData(forRowIndexes: all, columnIndexes: IndexSet(integer: 0)) - } - NSAnimationContext.runAnimationGroup { context in - context.duration = 0 - context.allowsImplicitAnimation = false - tableView.noteHeightOfRows(withIndexesChanged: all) - } - tableView.layoutSubtreeIfNeeded() - - if wasNearBottom { - scrollToBottom(animated: false) - } else if anchorRow >= 0, anchorRow < rows.count { - let targetY = tableView.rect(ofRow: anchorRow).minY - anchorOffset - scrollView.contentView.scroll(to: CGPoint(x: 0, y: targetY)) - scrollView.reflectScrolledClipView(scrollView.contentView) - } + cachedWidthsByMessageID.removeAll() + refreshVisibleRows(reloadCells: true) } // MARK: - Resize Handling @@ -1024,7 +1059,9 @@ final class TimelineTableViewController: NSViewController { // suppress the re-measure those rows need at this same width. guard !rows.isEmpty else { return } let columnWidth = tableView.tableColumns.first?.width ?? 0 - let renderWidth = columnWidth - tableView.safeAreaInsets.left - tableView.safeAreaInsets.right + let renderWidth = Self.effectiveContentWidth( + columnWidth: columnWidth, safeAreaInsets: tableView.safeAreaInsets + ) guard columnWidth > 1, renderWidth > 1, abs(renderWidth - lastRenderWidth) > 0.5 else { return } lastRenderWidth = renderWidth @@ -1047,11 +1084,18 @@ final class TimelineTableViewController: NSViewController { // A settled, single-shot width change (sidebar toggled or resized, // scroller appeared, programmatic window resize): coalesce layout // bursts for one frame, then run the full pass. - resizeRemeasureTask?.cancel() - resizeRemeasureTask = Task { @MainActor [weak self] in - try? await Task.sleep(for: .milliseconds(16)) - guard let self, !Task.isCancelled else { return } - self.remeasureAllRows(reloadCells: true, dropParseCaches: false) + scheduleResizeRemeasure() + } + + /// Coalesces a resize-driven width change into one full-timeline + /// re-measure, one frame after the last change. Shared by + /// ``scheduleRemeasureIfEffectiveWidthChanged()`` (the general + /// effective-width watcher) and ``viewDidResize(_:)`` (the scroll-view + /// frame watcher) — both can observe the same resize, and each used to + /// schedule this identical debounce independently. + private func scheduleResizeRemeasure() { + debounce(&resizeRemeasureTask, milliseconds: 16) { [weak self] in + self?.remeasureAllRows(dropParseCaches: false) } } @@ -1069,38 +1113,24 @@ final class TimelineTableViewController: NSViewController { /// them back to a stale layout mid-drag. private func refreshVisibleRows(reloadCells: Bool) { let visible = tableView.rows(in: tableView.visibleRect) - guard visible.length > 0 else { return } - let wasNearBottom = isNearBottom - let anchorRow = visible.location - let anchorOffset = anchorRow >= 0 - ? tableView.rect(ofRow: anchorRow).minY - tableView.visibleRect.minY - : 0 - - // The shared host caches its height when only the width proposal - // changes, so it must be rebuilt for the new width/scale. - measurementHost = nil let upper = min(visible.upperBound, rows.count) - guard visible.lowerBound < upper else { return } + guard visible.length > 0, visible.lowerBound < upper else { return } let indexes = IndexSet(integersIn: visible.lowerBound ..< upper) - for idx in visible.lowerBound ..< upper { - invalidateHeight(for: rows[idx].id) - } - if reloadCells { - tableView.reloadData(forRowIndexes: indexes, columnIndexes: IndexSet(integer: 0)) - } - NSAnimationContext.runAnimationGroup { context in - context.duration = 0 - context.allowsImplicitAnimation = false - tableView.noteHeightOfRows(withIndexesChanged: indexes) - } - if wasNearBottom { - scrollToBottom(animated: false) - } else if anchorRow >= 0, anchorRow < rows.count { - let targetY = tableView.rect(ofRow: anchorRow).minY - anchorOffset - let current = scrollView.contentView.bounds.origin - if abs(current.y - targetY) > 0.5 { - scrollView.contentView.scroll(to: CGPoint(x: 0, y: targetY)) - scrollView.reflectScrolledClipView(scrollView.contentView) + + preservingScrollAnchor { + // The shared host caches its height when only the width proposal + // changes, so it must be rebuilt for the new width/scale. + measurementHost = nil + for idx in visible.lowerBound ..< upper { + invalidateHeight(for: rows[idx].id) + } + if reloadCells { + tableView.reloadData(forRowIndexes: indexes, columnIndexes: IndexSet(integer: 0)) + } + NSAnimationContext.runAnimationGroup { context in + context.duration = 0 + context.allowsImplicitAnimation = false + tableView.noteHeightOfRows(withIndexesChanged: indexes) } } } @@ -1122,12 +1152,7 @@ final class TimelineTableViewController: NSViewController { // changes; only the row heights lag (they aren't re-queried on a width // change), leaving the rewrapped text clipped. Coalesce the layout // burst for one frame, then re-measure. - resizeRemeasureTask?.cancel() - resizeRemeasureTask = Task { @MainActor [weak self] in - try? await Task.sleep(for: .milliseconds(16)) - guard let self, !Task.isCancelled else { return } - self.remeasureAllRows(reloadCells: true, dropParseCaches: false) - } + scheduleResizeRemeasure() } // MARK: - Scroll Detection @@ -1181,7 +1206,12 @@ extension TimelineTableViewController: NSTableViewDelegate { // nothing about. Measuring at the full column width proposes a wider // text wrap than the live cell uses, under-measuring the row height and // clipping the (more-wrapped, taller) live content top and bottom. - targetWidth -= tableView.safeAreaInsets.left + tableView.safeAreaInsets.right + // Floored at 1: a narrow window with the sidebar's safe-area inset open + // can otherwise drive this to zero or negative, which would hand a + // degenerate width to the measurement host below. + targetWidth = max(1, Self.effectiveContentWidth( + columnWidth: targetWidth, safeAreaInsets: tableView.safeAreaInsets + )) let messageRow = rows[messageIndex] let cacheKey = HeightCacheKey(messageRow.id, targetWidth) @@ -1221,6 +1251,7 @@ extension TimelineTableViewController: NSTableViewDelegate { )) let height = max(size.height, 1) heightCache[cacheKey] = height + cachedWidthsByMessageID[messageRow.id, default: []].insert(cacheKey.width) Self.perfSignposter.endInterval( "heightOfRow" as StaticString, measureState, diff --git a/RelayTests/MatrixHTMLParserTests.swift b/RelayTests/MatrixHTMLParserTests.swift index 5fa62c2e..b02a6482 100644 --- a/RelayTests/MatrixHTMLParserTests.swift +++ b/RelayTests/MatrixHTMLParserTests.swift @@ -21,6 +21,15 @@ import Testing struct MatrixHTMLParserTests { + init() { + // MatrixHTMLParser derives heading/base sizes from + // MessageTextScale.baseFontSize. This test target shares UserDefaults + // with the app (bundle_loader), so a real, persisted text-zoom level + // from manual testing would otherwise make size-comparison tests here + // fail for reasons unrelated to the change under test. + UserDefaults.standard.removeObject(forKey: MessageTextScale.userDefaultsKey) + } + // MARK: - Helpers /// Returns attributes at a character offset. diff --git a/RelayTests/MessageTextScaleTests.swift b/RelayTests/MessageTextScaleTests.swift new file mode 100644 index 00000000..ef21d87d --- /dev/null +++ b/RelayTests/MessageTextScaleTests.swift @@ -0,0 +1,109 @@ +// Copyright 2026 Link Dupont +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import AppKit +import Testing + +@testable import Relay + +// MARK: - MessageTextScaleTests +// +// These tests mutate MessageTextScale's persisted state via increase() / +// decrease() / reset(), unlike other suites that only ever read it. Reading +// and writing a real, shared UserDefaults.standard key from concurrently +// running tests is a genuine race (confirmed: an early version of this file +// intermittently pushed MatrixHTMLParserTests.headingFontSizes() to read a +// scale of 2.4 mid-run). Redirect MessageTextScale to a private, throwaway +// UserDefaults suite for the duration of this suite, and serialize its own +// tests so they can't race the swap against each other either. +@Suite(.serialized) +@MainActor +struct MessageTextScaleTests { + + init() { + MessageTextScale.userDefaults = UserDefaults(suiteName: "MessageTextScaleTests.\(UUID())")! + } + + @Test func resetRestoresDefaultScale() { + MessageTextScale.increase() + MessageTextScale.increase() + MessageTextScale.reset() + #expect(MessageTextScale.scale == MessageTextScale.defaultScale) + } + + @Test func increaseAndDecreaseAreSymmetric() { + MessageTextScale.reset() + let base = MessageTextScale.scale + MessageTextScale.increase() + MessageTextScale.decrease() + #expect(abs(MessageTextScale.scale - base) < 0.001, "One increase followed by one decrease must return to the starting scale.") + } + + @Test func increaseClampsAtMaxScaleWithoutOvershoot() { + MessageTextScale.reset() + for _ in 0 ..< 100 { + MessageTextScale.increase() + } + #expect(MessageTextScale.scale == MessageTextScale.maxScale, "Repeated increases must clamp at maxScale, not overshoot it.") + } + + @Test func decreaseClampsAtMinScaleWithoutUndershoot() { + MessageTextScale.reset() + for _ in 0 ..< 100 { + MessageTextScale.decrease() + } + #expect(MessageTextScale.scale == MessageTextScale.minScale, "Repeated decreases must clamp at minScale, not undershoot it.") + } + + @Test func changeAtLimitDoesNotRepostNotificationForANoOp() async { + MessageTextScale.reset() + for _ in 0 ..< 100 { + MessageTextScale.increase() + } + // At the ceiling, `scale` is already `maxScale`. + #expect(MessageTextScale.scale == MessageTextScale.maxScale) + + var notified = false + let observer = NotificationCenter.default.addObserver( + forName: MessageTextScale.didChangeNotification, object: nil, queue: nil + ) { _ in notified = true } + defer { NotificationCenter.default.removeObserver(observer) } + + MessageTextScale.increase() // Already at the ceiling: must be a no-op. + #expect(!notified, "Hitting the clamp again must not repost didChangeNotification (would needlessly churn the timeline).") + } + + @Test func baseFontSizeTracksScale() { + MessageTextScale.reset() + let unscaled = MessageTextScale.baseFontSize + #expect(unscaled == NSFont.systemFontSize) + + MessageTextScale.increase() + #expect(MessageTextScale.baseFontSize > unscaled, "baseFontSize must grow as the scale increases.") + #expect(MessageTextScale.baseFontSize == NSFont.systemFontSize * MessageTextScale.scale) + } + + @Test func baseFontPointSizeMatchesBaseFontSize() { + MessageTextScale.reset() + MessageTextScale.increase() + #expect(MessageTextScale.baseFont.pointSize == MessageTextScale.baseFontSize) + } + + @Test func clampBoundsRawValuesToMinAndMax() { + #expect(MessageTextScale.clamp(MessageTextScale.minScale - 10) == MessageTextScale.minScale) + #expect(MessageTextScale.clamp(MessageTextScale.maxScale + 10) == MessageTextScale.maxScale) + let mid = (MessageTextScale.minScale + MessageTextScale.maxScale) / 2 + #expect(MessageTextScale.clamp(mid) == mid) + } +} diff --git a/RelayTests/ParseCacheTests.swift b/RelayTests/ParseCacheTests.swift new file mode 100644 index 00000000..3e74baa3 --- /dev/null +++ b/RelayTests/ParseCacheTests.swift @@ -0,0 +1,123 @@ +// Copyright 2026 Link Dupont +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import Testing + +@testable import Relay + +// MARK: - ParseCacheTests + +struct ParseCacheTests { + + @Test func valueComputesAndCachesOnMiss() { + let cache = ParseCache(capacity: 4) + var computeCount = 0 + let first = cache.value(forKey: "a") { computeCount += 1; return 1 } + let second = cache.value(forKey: "a") { computeCount += 1; return 2 } + #expect(first == 1) + #expect(second == 1, "A cache hit must return the originally-computed value, not recompute.") + #expect(computeCount == 1) + } + + @Test func peekReturnsNilOnMissWithoutComputing() { + let cache = ParseCache(capacity: 4) + #expect(cache.peek("missing") == nil) + // A peek must not have inserted anything a subsequent `set` would collide with. + #expect(cache.peek("missing") == nil) + } + + @Test func peekDoesNotPromoteRecency() { + // Fill to capacity, then peek the oldest key repeatedly — peek must not + // count as a "use" for eviction purposes, unlike `value(forKey:compute:)`. + let cache = ParseCache(capacity: 2) + cache.set(1, forKey: "oldest") + cache.set(2, forKey: "newer") + for _ in 0 ..< 5 { _ = cache.peek("oldest") } + cache.set(3, forKey: "evictor") + #expect(cache.peek("oldest") == nil, "peek() must not promote recency; 'oldest' should still be evicted.") + #expect(cache.peek("newer") == 2) + #expect(cache.peek("evictor") == 3) + } + + @Test func setEvictsLeastRecentlyUsedPastCapacity() { + let cache = ParseCache(capacity: 3) + cache.set("a", forKey: 1) + cache.set("b", forKey: 2) + cache.set("c", forKey: 3) + cache.set("d", forKey: 4) // Evicts 1 (least recently touched). + #expect(cache.peek(1) == nil) + #expect(cache.peek(2) == "b") + #expect(cache.peek(3) == "c") + #expect(cache.peek(4) == "d") + } + + @Test func setOnExistingKeyRefreshesRecencyAndDoesNotGrow() { + let cache = ParseCache(capacity: 3) + cache.set("a", forKey: 1) + cache.set("b", forKey: 2) + cache.set("c", forKey: 3) + // Touch key 1 so it's most-recently-used; key 2 becomes the LRU entry. + cache.set("a-updated", forKey: 1) + cache.set("d", forKey: 4) // Should evict 2, not 1. + #expect(cache.peek(1) == "a-updated", "Re-setting an existing key must update its value.") + #expect(cache.peek(2) == nil, "Key 2 was least-recently-used after key 1 was refreshed.") + #expect(cache.peek(3) == "c") + #expect(cache.peek(4) == "d") + } + + @Test func valueForKeyPromotesRecencyOnHit() { + let cache = ParseCache(capacity: 3) + cache.set("a", forKey: 1) + cache.set("b", forKey: 2) + cache.set("c", forKey: 3) + // Reading key 1 via value(forKey:) should count as a use, sparing it + // from eviction in favor of key 2 (now the LRU entry). + _ = cache.value(forKey: 1) { "unused" } + cache.set("d", forKey: 4) + #expect(cache.peek(1) == "a") + #expect(cache.peek(2) == nil) + } + + @Test func removeAllEmptiesCacheAndRecency() { + let cache = ParseCache(capacity: 2) + cache.set("a", forKey: 1) + cache.set("b", forKey: 2) + cache.removeAll() + #expect(cache.peek(1) == nil) + #expect(cache.peek(2) == nil) + // Recency bookkeeping must also be reset — three fresh inserts after a + // clear on a capacity-2 cache should evict the first of the three, not + // silently mis-evict based on stale pre-clear order entries. + cache.set("x", forKey: 10) + cache.set("y", forKey: 11) + cache.set("z", forKey: 12) + #expect(cache.peek(10) == nil) + #expect(cache.peek(11) == "y") + #expect(cache.peek(12) == "z") + } + + @Test func evictionUnderSustainedPressureKeepsOnlyMostRecentCapacityEntries() { + let capacity = 8 + let cache = ParseCache(capacity: capacity) + for key in 0 ..< 256 { + cache.set(key, forKey: key) + } + for key in 0 ..< (256 - capacity) { + #expect(cache.peek(key) == nil, "Key \(key) should have been evicted long before the cache filled 256 entries at capacity \(capacity).") + } + for key in (256 - capacity) ..< 256 { + #expect(cache.peek(key) == key, "The most recent \(capacity) entries must all survive.") + } + } +} diff --git a/RelayTests/TimelineHeightMeasurementTests.swift b/RelayTests/TimelineHeightMeasurementTests.swift index 951c28cd..3636ca36 100644 --- a/RelayTests/TimelineHeightMeasurementTests.swift +++ b/RelayTests/TimelineHeightMeasurementTests.swift @@ -34,6 +34,14 @@ import Testing @MainActor struct TimelineHeightMeasurementTests { + init() { + // Several tests parse through MatrixHTMLParser/MessageTextScale. + // This test target shares UserDefaults with the app (bundle_loader), + // so a real, persisted text-zoom level from manual testing could + // otherwise leak into a comparison here. Reset for hermeticity. + UserDefaults.standard.removeObject(forKey: MessageTextScale.userDefaultsKey) + } + // MARK: - TextKit Layout Harness /// A faithful replica of the `NSTextView`/`NSLayoutManager`/`NSTextContainer` @@ -260,6 +268,44 @@ struct TimelineHeightMeasurementTests { ) } + // MARK: - 3b. Safe-area-aware effective content width + + /// The ordinary case: the column is wider than the combined insets, so + /// the effective width is simply the difference. + @Test func effectiveContentWidthSubtractsSafeAreaInsets() { + let width = TimelineTableViewController.effectiveContentWidth( + columnWidth: 610, safeAreaInsets: NSEdgeInsets(top: 0, left: 170, bottom: 0, right: 0) + ) + #expect(width == 440) + } + + /// A narrow window with the overlay sidebar's safe-area inset open can + /// drive the raw subtraction to zero or negative — the exact scenario + /// this branch's root-cause fix targets. `effectiveContentWidth` itself + /// reports the (possibly negative) raw value; callers are responsible + /// for their own floor/skip behavior on top of it. This guards the + /// primitive `heightOfRow`'s `max(1, ...)` floor and + /// `scheduleRemeasureIfEffectiveWidthChanged`'s `> 1` skip guard both + /// build on, so a regression in either caller's clamp shows up as this + /// raw value going unexpectedly non-negative instead. + @Test func effectiveContentWidthCanGoNonPositiveWhenInsetsExceedColumn() { + let width = TimelineTableViewController.effectiveContentWidth( + columnWidth: 150, safeAreaInsets: NSEdgeInsets(top: 0, left: 170, bottom: 0, right: 0) + ) + #expect(width <= 0, "Expected a non-positive raw width when insets exceed the column, got \(width).") + } + + /// Regression guard for the specific floor `heightOfRow` applies: a + /// negative effective width must never reach the measurement host as + /// anything less than 1pt. + @Test func flooredEffectiveContentWidthNeverGoesBelowOnePoint() { + let raw = TimelineTableViewController.effectiveContentWidth( + columnWidth: 100, safeAreaInsets: NSEdgeInsets(top: 0, left: 170, bottom: 0, right: 0) + ) + #expect(raw < 1) + #expect(max(1, raw) == 1) + } + // MARK: - 4. Link-preview card height determinism /// A variable-height link card must derive its height synchronously from the