From 8ef06cd2f4ec644211df80a6d3ae5eb4f1136da2 Mon Sep 17 00:00:00 2001 From: Doug <6060466+pixlwave@users.noreply.github.com> Date: Fri, 30 Jan 2026 10:26:36 +0000 Subject: [PATCH] Tidy-up FormattedBodyText and MessageBubbleLayout. (#5019) * Tidy-up FormattedBodyText. * Switch TimelineBubbleLayout.Priority from .layoutPriority to .layoutValue. * Rename TimelineBubbleLayout.Priority to TimelineBubbleLayout.Size --- .../View/Style/TimelineBubbleLayout.swift | 58 +++++--- .../Style/TimelineItemBubbledStylerView.swift | 8 +- .../TimelineItemViews/FormattedBodyText.swift | 126 +++++++++--------- 3 files changed, 112 insertions(+), 80 deletions(-) diff --git a/ElementX/Sources/Screens/Timeline/View/Style/TimelineBubbleLayout.swift b/ElementX/Sources/Screens/Timeline/View/Style/TimelineBubbleLayout.swift index d9ffd247a..b63a24cb3 100644 --- a/ElementX/Sources/Screens/Timeline/View/Style/TimelineBubbleLayout.swift +++ b/ElementX/Sources/Screens/Timeline/View/Style/TimelineBubbleLayout.swift @@ -20,18 +20,39 @@ struct TimelineBubbleLayout: Layout { /// The spacing between the components in the bubble. let spacing: CGFloat - /// Layout priority constants for the bubble content. These priorities are abused within - /// `TimelineBubbleLayout` to create the layout we would like. They aren't - /// used in the expected way that SwiftUI would normally use layout priorities. - enum Priority { - /// The priority of hidden quote bubbles/code blocks that are only used for layout calculations. - /// Any views given this priority should be made non-greedy for the calculations to work. - static let hiddenGreedyComponent: Double = -1 - /// The priority of visible quote bubbles/code blocks that are placed in the view with a full width. - /// Any views given this priority should remain in their normal greedy form. - static let visibleGreedyComponent: Double = 0 - /// The priority of regular text that is used for layout calculations and placed in the view. - static let nonGreedyComponent: Double = 1 + /// Layout size for the bubble content. These sizing types are used within + /// `TimelineBubbleLayout` to create the layout we would like. + enum Size: LayoutValueKey, Equatable { + static let defaultValue: Size = .natural + + /// Full width mode used for greedy components like blockquotes and code blocks. + enum BubbleWidthMode { + /// The view has its natural size and will be used for layout calculations only. + case layout + /// The view has a greedy width and will fill the available space within the bubble. + case rendering + } + + /// The view will fill the available width, with different behaviour depending on the mode. + /// Views using the `.layout` mode should be hidden and are used only for width calculations. + /// Views using the `.rendering` mode should be visible and are placed to fill the bubble's calculated width. + case bubbleWidth(mode: BubbleWidthMode) + /// The view uses its natural size for both layout calculations and rendering. + case natural + + var shouldLayout: Bool { + switch self { + case .natural, .bubbleWidth(mode: .layout): true + default: false + } + } + + var shouldRender: Bool { + switch self { + case .natural, .bubbleWidth(mode: .rendering): true + default: false + } + } } func makeCache(subviews: Subviews) -> Cache { @@ -47,7 +68,7 @@ struct TimelineBubbleLayout: Layout { guard !subviews.isEmpty else { return .zero } // Calculate the natural size using the regular text and non-greedy quote bubbles. - let layoutSubviews = subviews.filter { $0.priority != Priority.visibleGreedyComponent } + let layoutSubviews = subviews.filter { $0[Size.self].shouldLayout } let subviewSizes = layoutSubviews.map { size(for: $0, subviews: subviews, proposedSize: proposal, cache: &cache) } @@ -62,11 +83,11 @@ struct TimelineBubbleLayout: Layout { guard !subviews.isEmpty else { return } // Calculate the width using the regular text along with non-greedy versions of any greedy components. - let layoutSubviews = subviews.filter { $0.priority != Priority.visibleGreedyComponent } + let layoutSubviews = subviews.filter { $0[Size.self].shouldLayout } let maxWidth = layoutSubviews.map { size(for: $0, subviews: subviews, proposedSize: proposal, cache: &cache).width }.reduce(0, max) // Place the regular text and greedy components using the calculated width. - let visibleSubviews = subviews.filter { $0.priority != Priority.hiddenGreedyComponent } + let visibleSubviews = subviews.filter { $0[Size.self].shouldRender } let subviewSizes = visibleSubviews.map { size(for: $0, subviews: subviews, proposedSize: ProposedViewSize(width: maxWidth, height: proposal.height), cache: &cache) } @@ -101,3 +122,10 @@ struct TimelineBubbleLayout: Layout { return size } } + +extension View { + /// Sets the layout size for this view when placed within a `TimelineBubbleLayout`. + func timelineBubbleLayoutSize(_ size: TimelineBubbleLayout.Size) -> some View { + layoutValue(key: TimelineBubbleLayout.Size.self, value: size) + } +} diff --git a/ElementX/Sources/Screens/Timeline/View/Style/TimelineItemBubbledStylerView.swift b/ElementX/Sources/Screens/Timeline/View/Style/TimelineItemBubbledStylerView.swift index 955f92d7d..f56906fcc 100644 --- a/ElementX/Sources/Screens/Timeline/View/Style/TimelineItemBubbledStylerView.swift +++ b/ElementX/Sources/Screens/Timeline/View/Style/TimelineItemBubbledStylerView.swift @@ -190,7 +190,7 @@ struct TimelineItemBubbledStylerView: View { if !context.viewState.timelineKind.isThread, timelineItem.properties.isThreaded { ThreadDecorator() .padding(.leading, 4) - .layoutPriority(TimelineBubbleLayout.Priority.nonGreedyComponent) + .timelineBubbleLayoutSize(.natural) } if let replyDetails = timelineItem.properties.replyDetails { @@ -203,7 +203,7 @@ struct TimelineItemBubbledStylerView: View { .frame(maxWidth: .infinity, alignment: .leading) .background(Color.compound.bgCanvasDefault) .cornerRadius(8) - .layoutPriority(TimelineBubbleLayout.Priority.visibleGreedyComponent) + .timelineBubbleLayoutSize(.bubbleWidth(mode: .rendering)) .onTapGesture { if context.viewState.timelineKind != .pinned { context.send(viewAction: .focusOnEventID(replyDetails.eventID)) @@ -214,12 +214,12 @@ struct TimelineItemBubbledStylerView: View { TimelineReplyView(placement: .timeline, timelineItemReplyDetails: replyDetails) .fixedSize(horizontal: false, vertical: true) .padding(4.0) - .layoutPriority(TimelineBubbleLayout.Priority.hiddenGreedyComponent) + .timelineBubbleLayoutSize(.bubbleWidth(mode: .layout)) .hidden() } content() - .layoutPriority(TimelineBubbleLayout.Priority.nonGreedyComponent) + .timelineBubbleLayoutSize(.natural) .cornerRadius(timelineItem.contentCornerRadius) } } diff --git a/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/FormattedBodyText.swift b/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/FormattedBodyText.swift index 24404fc85..59409b5fc 100644 --- a/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/FormattedBodyText.swift +++ b/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/FormattedBodyText.swift @@ -63,14 +63,10 @@ struct FormattedBodyText: View { } var body: some View { - mainContent - .accessibilityElement(children: .ignore) - .accessibilityLabel(Text(attributedString)) - } - - var mainContent: some View { layout .tint(.compound.textLinkExternal) + .accessibilityElement(children: .ignore) + .accessibilityLabel(Text(attributedString)) } /// The attributed components laid out for the bubbles timeline style. @@ -83,61 +79,36 @@ struct FormattedBodyText: View { } else { switch component.type { case .blockquote: - // The rendered blockquote with a greedy width. The custom layout prevents the - // infinite width from increasing the overall width of the view. - MessageText(attributedString: component.attributedString.mergingAttributes(blockquoteAttributes)) - .fixedSize(horizontal: false, vertical: true) - .frame(maxWidth: .infinity, alignment: .leading) - .padding(.leading, 12.0) - .overlay(alignment: .leading) { - // User an overlay here so that the rectangle's infinite height doesn't take priority - Capsule() - .frame(width: 2.0) - .padding(.leading, 5.0) - .foregroundColor(.compound.textSecondary) - .padding(.vertical, 2) - } - .layoutPriority(TimelineBubbleLayout.Priority.visibleGreedyComponent) + BlockquoteView(attributedString: component.attributedString, mode: .rendering) + .timelineBubbleLayoutSize(.bubbleWidth(mode: .rendering)) case .codeBlock: - // The rendered codeblock with a greedy width (due to the scroll view). The custom - // layout prevents the scroll view from increasing the overall width of the view. - ScrollView(.horizontal) { - MessageText(attributedString: component.attributedString) - .padding([.horizontal, .top], 4) - .padding(.bottom, 8) - } - .background(.compound._bgCodeBlock) - .scrollBounceBehavior(.basedOnSize, axes: .horizontal) - .scrollIndicatorsFlash(onAppear: true) - .padding(.horizontal, 4) - .layoutPriority(TimelineBubbleLayout.Priority.visibleGreedyComponent) - .contextMenu { - Button(L10n.actionCopy) { - UIPasteboard.general.string = component.attributedString.string + CodeBlockView(attributedString: component.attributedString, mode: .rendering) + .timelineBubbleLayoutSize(.bubbleWidth(mode: .rendering)) + .contextMenu { + Button(L10n.actionCopy) { + UIPasteboard.general.string = component.attributedString.string + } } - } case .plainText: MessageText(attributedString: component.attributedString) .padding(.horizontal, 4) .fixedSize(horizontal: false, vertical: true) - .layoutPriority(TimelineBubbleLayout.Priority.nonGreedyComponent) + .timelineBubbleLayoutSize(.natural) } } } - // Make a second iteration through the components adding fixed width blockquotes/codeblocks - // which are used for layout calculations but won't be rendered. + // Make a second iteration through the components adding naturally sized versions of the + // block quotes and code blocks which are used for layout calculations but won't be rendered. ForEach(attributedComponents) { component in switch component.type { case .blockquote: - MessageText(attributedString: component.attributedString.mergingAttributes(blockquoteAttributes)) - .fixedSize(horizontal: false, vertical: true) - .padding(.leading, 12.0) - .layoutPriority(TimelineBubbleLayout.Priority.hiddenGreedyComponent) + BlockquoteView(attributedString: component.attributedString, mode: .layout) + .timelineBubbleLayoutSize(.bubbleWidth(mode: .layout)) .hidden() case .codeBlock: - HiddenCodeBlockScrollView(attributedString: component.attributedString) - .layoutPriority(TimelineBubbleLayout.Priority.hiddenGreedyComponent) + CodeBlockView(attributedString: component.attributedString, mode: .layout) + .timelineBubbleLayoutSize(.bubbleWidth(mode: .layout)) .hidden() case .plainText: EmptyView() @@ -146,33 +117,66 @@ struct FormattedBodyText: View { } } - private var blockquoteAttributes: AttributeContainer { - // The paragraph style removes the block style paragraph that the parser adds by default - // Set directly in the constructor to avoid `Conformance to 'Sendable'` warnings - var container = AttributeContainer([.paragraphStyle: NSParagraphStyle.default]) - // Sadly setting SwiftUI fonts do not work so we would need UIFont equivalents for compound, this one is bodyMD - container.font = UIFont.preferredFont(forTextStyle: .subheadline) - container.foregroundColor = UIColor.compound.textSecondary + // MARK: - Component Views + + /// The view used to render a blockquote component. It can be configured in one of 2 modes: + /// - `.layout`: The view is given it's natural size to be used for layout calculations. + /// - `.rendering`: The view has a greedy width that, in combination with the custom layout, + /// will fill any available space, whilst remaining constrained by the bubble's calculated width. + struct BlockquoteView: View { + let attributedString: AttributedString + let mode: TimelineBubbleLayout.Size.BubbleWidthMode - return container + var body: some View { + MessageText(attributedString: attributedString.mergingAttributes(blockquoteAttributes)) + .fixedSize(horizontal: false, vertical: true) + .frame(maxWidth: mode == .rendering ? .infinity : nil, alignment: .leading) + .padding(.leading, 12.0) + .overlay(alignment: .leading) { + // Use an overlay here so that the rectangle's infinite height doesn't take priority + if mode == .rendering { + Capsule() + .frame(width: 2.0) + .padding(.leading, 5.0) + .foregroundColor(.compound.textSecondary) + .padding(.vertical, 2) + } + } + } + + private var blockquoteAttributes: AttributeContainer { + // The paragraph style removes the block style paragraph that the parser adds by default + // Set directly in the constructor to avoid `Conformance to 'Sendable'` warnings + var container = AttributeContainer([.paragraphStyle: NSParagraphStyle.default]) + // Sadly setting SwiftUI fonts do not work so we would need UIFont equivalents for compound, this one is bodyMD + container.font = UIFont.preferredFont(forTextStyle: .subheadline) + container.foregroundColor = UIColor.compound.textSecondary + + return container + } } - /// A self-sizing version of the code block component's view, necessary - /// because unlike quote bubbles, code blocks don't wrap when the space - /// is constrained. - private struct HiddenCodeBlockScrollView: View { + /// The view used to render a code block component. It can be configured in one of 2 modes: + /// - `.layout`: The view is given it's natural size to be used for layout calculations. + /// - `.rendering`: The view has a greedy width that, in combination with the custom layout, + /// will fill any available space, whilst remaining constrained by the bubble's calculated width. + private struct CodeBlockView: View { let attributedString: AttributedString + let mode: TimelineBubbleLayout.Size.BubbleWidthMode - @State private var maxSize: CGSize = .zero + @State private var maxWidth: CGFloat = .zero var body: some View { ScrollView(.horizontal) { MessageText(attributedString: attributedString) .padding([.horizontal, .top], 4) .padding(.bottom, 8) - .onGeometryChange(for: CGSize.self) { $0.size } action: { maxSize = $0 } + .onGeometryChange(for: CGFloat.self) { $0.size.width } action: { maxWidth = $0 } } - .frame(maxWidth: maxSize.width) + .frame(maxWidth: mode == .layout ? maxWidth : nil) + .background(.compound._bgCodeBlock) + .scrollBounceBehavior(.basedOnSize, axes: .horizontal) + .scrollIndicatorsFlash(onAppear: true) .padding(.horizontal, 4) } }