Fix timeline rendering, text zoom, and resize responsiveness - #167
Merged
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…ard cache and favicons 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
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 <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
- 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<AnyView> to support pinning a measured row's width. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
lukaaas176
marked this pull request as ready for review
July 19, 2026 18:42
Owner
|
The row height recalculation still messes up from time to time, but this PR is definitely an improvement. |
Contributor
Author
Oh :o Somehow i totally forgot about the detail view, i will look into it! :) Is this the only case currently you found? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes bubble/text clipping in the timeline (at narrow widths, after resize, and with the overlay sidebar open), adds a user-adjustable text zoom for messages and compose, and keeps the UI responsive during live resize and zoom bursts. Also lands variable-height link preview cards, font-scaled mention pills, and a pass of perf/correctness fixes and new regression tests from code review.
Text zoom — View menu gains Increase/Decrease/Reset Text Size (⌘+ / ⌘− / ⌥⌘0). Message bodies, mention pills, emote name prefixes, and the compose field all derive their size from a new
MessageTextScale(persisted, clamped) instead of the system font size directly. Chrome (sender names, date separators) scales with ascaledChromeFontmodifier so the whole timeline tracks zoom consistently, and typed compose pills rebuild at the new size instead of staying stuck at their original bitmap.Row-height measurement — Root-caused the clipping: the detached measurement host used for
heightOfRowignored the sidebar's safe-area inset, so it measured a wider column than the live cells actually laid out in, undercounting wrapped lines. Rows now measure ateffectiveContentWidth(column width minus safe-area insets, floored at 1pt), re-checked on both resize and sidebar (safe-area) changes. Also fixed a stale-width bug on recycled cells, afittingSizeshortcut that re-poisoned the cache with pre-inset values, and pinned the "edited" label's height so it isn't compressed.Live resize / zoom responsiveness — Full reload+re-measure passes are correct at rest but were running on every intermediate resize/zoom step, causing visible lag. Now a cheap visible-rows-only refresh runs throttled during the drag or key-repeat burst, and the expensive full pass is deferred until the burst settles (
viewDidEndLiveResize, or a short trailing timer for zoom).Link previews — Cards now show the Open Graph image edge-to-edge at its native aspect ratio instead of cropping into a fixed square, with a bounded LRU cache (
ParseCache) so the async-loaded aspect ratio is available to the synchronous row-height measurement path. Falls back to the site favicon (not a generic globe) when no OG image is available.Mention pills — Sized to the surrounding font's line box (were ~2pt taller than the line, stretching the pill image and clipping on a message's first line); pill bitmaps now render 1:1 and scale cleanly with text zoom.
Perf/correctness pass from code review — Scoped the eager re-measure pass to visible rows only (off-screen rows measure lazily when shown, restoring main's original behavior); replaced an O(n) cache-invalidation filter with a per-message reverse index; rewrote
ParseCache's LRU from array+firstIndexto an O(1) doubly-linked list; deduplicated three hand-rolled debounce blocks and two scroll-anchor implementations into shared helpers. Fixed a same-URL-different-message remeasure bug, an unfloored width computation, an unscaled emote-prefix font, an under-measured initial compose height, and an overly broad favicon fallback.Tests — Added
TimelineHeightMeasurementTests(headless TextKit layout checks: wrapped-width flooring, pill bounds, link-card height determinism, effective-content-width edge cases),ParseCacheTests(LRU eviction/recency/peek semantics), andMessageTextScaleTests(clamping, symmetry, notification behavior). Fixed a real test- isolation bug where two suites read scale-derived sizes without resetting the persisted scale first.Note: this branch was worked on over two days; a regrouping of related commits into logical units afterward means the commit timestamps don't reflect when the work actually happened.
AI Disclaimer: This pull request is heavily AI assisted, but I looked over every code change and checked myself.