diff --git a/PReek.xcodeproj/project.pbxproj b/PReek.xcodeproj/project.pbxproj index d647b63..a75b5ca 100644 --- a/PReek.xcodeproj/project.pbxproj +++ b/PReek.xcodeproj/project.pbxproj @@ -509,7 +509,7 @@ "@executable_path/../Frameworks", ); MACOSX_DEPLOYMENT_TARGET = 26.0; - MARKETING_VERSION = 0.6.0; + MARKETING_VERSION = 0.6.1; PRODUCT_BUNDLE_IDENTIFIER = "de.max-heidinger.PReek"; PRODUCT_NAME = "$(TARGET_NAME)"; PROVISIONING_PROFILE_SPECIFIER = ""; @@ -552,7 +552,7 @@ "@executable_path/../Frameworks", ); MACOSX_DEPLOYMENT_TARGET = 26.0; - MARKETING_VERSION = 0.6.0; + MARKETING_VERSION = 0.6.1; PRODUCT_BUNDLE_IDENTIFIER = "de.max-heidinger.PReek"; PRODUCT_NAME = "$(TARGET_NAME)"; PROVISIONING_PROFILE_SPECIFIER = ""; diff --git a/PReek/Views/PullRequestViews/CommitsView.swift b/PReek/Views/PullRequestViews/CommitsView.swift index b24bcae..56bf205 100644 --- a/PReek/Views/PullRequestViews/CommitsView.swift +++ b/PReek/Views/PullRequestViews/CommitsView.swift @@ -4,7 +4,10 @@ struct CommitsView: View { let commits: [Commit] var body: some View { - LazyVStack(alignment: .leading) { + // Plain VStack (not Lazy): commits-per-push are few and this is already nested several + // lazy stacks deep. Nested LazyVStacks inside a ScrollView amplify layout cost without any + // laziness benefit for this bounded content. + VStack(alignment: .leading) { ForEach(commits) { commit in if let url = commit.url { HoverableLink(destination: url) { diff --git a/PReek/Views/PullRequestViews/DisclosureGroupList/PullRequestContentView.swift b/PReek/Views/PullRequestViews/DisclosureGroupList/PullRequestContentView.swift index f1ea56a..3b3e032 100644 --- a/PReek/Views/PullRequestViews/DisclosureGroupList/PullRequestContentView.swift +++ b/PReek/Views/PullRequestViews/DisclosureGroupList/PullRequestContentView.swift @@ -27,7 +27,10 @@ struct PullRequestContentView: View, Equatable { } var eventsBody: some View { - LazyVStack { + // Plain VStack (not Lazy): this is nested inside the list's outer LazyVStack and renders a + // bounded, small number of events (eventLimit is capped). Nesting LazyVStacks inside a + // ScrollView causes expensive, conflicting size negotiation without any laziness benefit. + VStack { DividedView(pullRequest.events[0 ..< eventLimit]) { event in EventView(event) } shouldHighlight: { event in diff --git a/PReek/Views/PullRequestViews/EventView.swift b/PReek/Views/PullRequestViews/EventView.swift index 54eaf18..13067ab 100644 --- a/PReek/Views/PullRequestViews/EventView.swift +++ b/PReek/Views/PullRequestViews/EventView.swift @@ -62,7 +62,7 @@ struct EventView: View { } var eventHeader: some View { - HStack(alignment: .firstTextBaseline) { + HStack { Text(event.user.displayName) Spacer() Text(eventDataToActionLabel(data: event.data)) diff --git a/PReek/Views/PullRequestViews/List/PullRequestDetailView.swift b/PReek/Views/PullRequestViews/List/PullRequestDetailView.swift index fdbbc35..01fbb53 100644 --- a/PReek/Views/PullRequestViews/List/PullRequestDetailView.swift +++ b/PReek/Views/PullRequestViews/List/PullRequestDetailView.swift @@ -24,7 +24,10 @@ struct PullRequestDetailView: View { Divider() - LazyVStack { + // Plain VStack (not Lazy): the outer LazyVStack is the ScrollView's lazy child. + // Nesting a second LazyVStack inside it causes expensive, conflicting size + // negotiation with no laziness benefit. + VStack { DividedView(pullRequest.events) { event in EventView(event) } shouldHighlight: { event in diff --git a/PReek/Views/UtilityViews/ClippedMarkdownView.swift b/PReek/Views/UtilityViews/ClippedMarkdownView.swift index fef292d..00fe84e 100644 --- a/PReek/Views/UtilityViews/ClippedMarkdownView.swift +++ b/PReek/Views/UtilityViews/ClippedMarkdownView.swift @@ -20,8 +20,8 @@ private struct NoneInlineImageProvider: InlineImageProvider { struct ClippedMarkdownView: View { let rawMarkdown: String - @State private var contentHeight: CGFloat = 0 @State private var isExpanded = false + @State private var isOverflowing = false let maxHeight: CGFloat = 100 @@ -29,6 +29,10 @@ struct ClippedMarkdownView: View { MarkdownContentCache.content(rawMarkdown: rawMarkdown) } + private var showClippedAffordances: Bool { + isOverflowing && !isExpanded + } + var body: some View { Markdown(content) .markdownImageProvider(NoneImageProvider()) @@ -42,32 +46,29 @@ struct ClippedMarkdownView: View { } ) .onPreferenceChange(HeightPreferenceKey.self) { height in - contentHeight = height + isOverflowing = height > maxHeight } - .frame(height: isExpanded ? nil : min(contentHeight, maxHeight), alignment: .top) + .frame(maxHeight: isExpanded ? nil : maxHeight, alignment: .top) .clipped() - .if(!isExpanded && contentHeight > maxHeight) { view in - view - .mask { - LinearGradient( - colors: [.black, .clear], startPoint: .center, endPoint: .bottom - ) - } + .mask(alignment: .top) { + LinearGradient( + colors: [.black, showClippedAffordances ? .clear : .black], + startPoint: .center, + endPoint: .bottom + ) } - .if(contentHeight > maxHeight) { view in - view - .overlay(alignment: .bottomTrailing) { - Button(action: { isExpanded = !isExpanded }) { - Image( - systemName: isExpanded - ? "arrowtriangle.up.square" : "arrowtriangle.down.square" - ) - .imageScale(.large) - } - .buttonStyle(PlainButtonStyle()) + .overlay(alignment: .bottomTrailing) { + if isOverflowing { + Button(action: { isExpanded.toggle() }) { + Image( + systemName: isExpanded + ? "arrowtriangle.up.square" : "arrowtriangle.down.square" + ) + .imageScale(.large) } + .buttonStyle(PlainButtonStyle()) + } } - .frame(height: isExpanded || contentHeight <= maxHeight ? contentHeight : maxHeight) } } diff --git a/PReek/Views/UtilityViews/TimeSensitiveText.swift b/PReek/Views/UtilityViews/TimeSensitiveText.swift index ebef8b9..8078f88 100644 --- a/PReek/Views/UtilityViews/TimeSensitiveText.swift +++ b/PReek/Views/UtilityViews/TimeSensitiveText.swift @@ -1,17 +1,32 @@ import SwiftUI -/// Displays a time-derived string (e.g. "5 minutes ago") that needs to refresh as time passes. +/// Displays a time-derived string (e.g. "5 minutes ago") that refreshes as time passes. /// -/// Each instance owns its own `TimelineView` clock instead of subscribing to a shared timer, so -/// labels refresh independently while on screen (the timeline pauses when the view leaves the -/// hierarchy) and the per-instance `from: .now` phase staggers the work instead of recomputing -/// every label in one synchronized frame. +/// Renders a plain `Text` rather than wrapping each label in a `TimelineView`. A `TimelineView` is +/// a layout container with dynamic content, so SwiftUI cannot cache the subtree's size/alignment; +/// with one in every PR/event row nested inside alignment-resolving stacks, that re-resolution +/// compounds super-linearly and can hang layout. A plain `Text` keeps the row layout static. +/// +/// `getText()` is recomputed directly in `body` so the value is always current on any re-render. +/// A per-instance timer simply forces a re-render periodically so the label keeps advancing while +/// the view is otherwise idle. The text is intentionally NOT cached in `@State`: caching it made +/// labels stick at their initial value whenever the timer did not fire. struct TimeSensitiveText: View { let getText: () -> String + @State private var refreshToken = 0 + private let timer = Timer.publish(every: 30, on: .main, in: .common).autoconnect() + + init(getText: @escaping () -> String) { + self.getText = getText + } + var body: some View { - TimelineView(.periodic(from: .now, by: 60)) { _ in - Text(getText()) - } + // Mutating `refreshToken` on each tick invalidates this view, so `body` re-evaluates and + // `getText()` is recomputed against the current date. + Text(getText()) + .onReceive(timer) { _ in + refreshToken &+= 1 + } } }