Fix inline code being rendered as blocks. (#5017)

* Fix inline code being rendered as blocks.

And make blocks non-greedy as well as only scrolling when needed.

* Rename the bubble layout priorities.

* Add an InlineCode attribute so that the builder also strips links from these too.

* Split up the snapshot tests into individual cases.

This should make it much easier to see *what* has changed when regenerating.

ElementX/Sources/Other/HTMLParsing/AttributedStringBuilder.swift

@@ -32,6 +32,7 @@ extension NSAttributedString.Key {
     static let MatrixEventOnRoomAlias: NSAttributedString.Key = .init(rawValue: EventOnRoomAliasAttribute.name)
     static let MatrixAllUsersMention: NSAttributedString.Key = .init(rawValue: AllUsersMentionAttribute.name)
     static let CodeBlock: NSAttributedString.Key = .init(rawValue: CodeBlockAttribute.name)
+    static let InlineCode: NSAttributedString.Key = .init(rawValue: InlineCodeAttribute.name)
 }
 
 struct AttributedStringBuilder: AttributedStringBuilderProtocol {
@@ -197,15 +198,24 @@ struct AttributedStringBuilder: AttributedStringBuilderProtocol {
                 content.addAttribute(.MatrixBlockquote, value: true, range: NSRange(location: 0, length: content.length))
                 
             case "code", "pre":
-                let preserveFormatting = preserveFormatting || tag == "pre"
+                let isCodeBlock = tag == "pre"
+                
+                let preserveFormatting = preserveFormatting || isCodeBlock
                 content = attributedString(element: childElement, documentBody: documentBody, preserveFormatting: preserveFormatting, listTag: listTag, listIndex: &childIndex, indentLevel: indentLevel)
                 
                 let fontPointSize = fontPointSize * 0.9 // Intentionally shrink code blocks by 10%
                 content.setFontPreservingSymbolicTraits(UIFont.monospacedSystemFont(ofSize: fontPointSize, weight: .regular))
                 
-                content.addAttribute(.CodeBlock, value: true, range: NSRange(location: 0, length: content.length))
+                if isCodeBlock {
+                    content.addAttribute(.CodeBlock, value: true, range: NSRange(location: 0, length: content.length))
+                    // The scroll view provides the background colour for code blocks.
+                } else {
+                    content.addAttribute(.InlineCode, value: true, range: NSRange(location: 0, length: content.length))
+                    // But inline code is (obviously) inline so it's much easier to set the background colour here.
+                    content.addAttribute(.backgroundColor, value: UIColor.compound._bgCodeBlock as Any, range: NSRange(location: 0, length: content.length))
+                }
                 
-                // Don't allow identifiers or links in code blocks
+                // Don't allow identifiers or links in code.
                 content.removeAttribute(.MatrixRoomID, range: NSRange(location: 0, length: content.length))
                 content.removeAttribute(.MatrixRoomAlias, range: NSRange(location: 0, length: content.length))
                 content.removeAttribute(.MatrixUserID, range: NSRange(location: 0, length: content.length))
@@ -353,14 +363,15 @@ struct AttributedStringBuilder: AttributedStringBuilderProtocol {
         // Sort the links by length so the longest one always takes priority
         matches.sorted { $0.range.length > $1.range.length }.forEach { [attributedString] match in
             // Don't highlight links within codeblocks
-            let isInCodeBlock = attributedString.attribute(.CodeBlock, at: match.range.location, effectiveRange: nil) != nil
-            if isInCodeBlock {
+            let isCode = attributedString.attribute(.CodeBlock, at: match.range.location, effectiveRange: nil) != nil
+                || attributedString.attribute(.InlineCode, at: match.range.location, effectiveRange: nil) != nil
+            if isCode {
                 return
             }
             
             var hasLink = false
             attributedString.enumerateAttribute(.link, in: match.range, options: []) { value, _, stop in
-                if value != nil, !isInCodeBlock {
+                if value != nil, !isCode {
                     hasLink = true
                     stop.pointee = true
                 }

ElementX/Sources/Other/HTMLParsing/ElementXAttributeScope.swift

@@ -68,6 +68,11 @@ enum CodeBlockAttribute: AttributedStringKey {
     static let name = "MXCodeBlockAttribute"
 }
 
+enum InlineCodeAttribute: AttributedStringKey {
+    typealias Value = Bool
+    static let name = "MXInlineCodeAttribute"
+}
+
 // periphery: ignore - required to make NSAttributedString to AttributedString conversion even if not used directly
 extension AttributeScopes {
     struct ElementXAttributes: AttributeScope {
@@ -84,6 +89,7 @@ extension AttributeScopes {
         let allUsersMention: AllUsersMentionAttribute
         
         let codeBlock: CodeBlockAttribute
+        let inlineCode: InlineCodeAttribute
         
         let swiftUI: SwiftUIAttributes
         let uiKit: UIKitAttributes

ElementX/Sources/Other/HTMLParsing/HTMLFixtures.swift

@@ -15,7 +15,8 @@ enum HTMLFixtures: String, CaseIterable {
     case textFormatting
     case groupedBlockQuotes
     case separatedBlockQuotes
-    case codeBlocks
+    case code
+    case wideCodeBlock
     case unorderedList
     case orderedList
     
@@ -74,9 +75,9 @@ enum HTMLFixtures: String, CaseIterable {
             <blockquote>Some other blockquote</blockquote>\
             Text after second blockquote
             """
-        case .codeBlocks:
+        case .code:
             """
-            <pre>A preformatted code block
+            <pre>A pre-formatted code block
             <code>struct ContentView: View {
                 var body: some View {
                     VStack {
@@ -88,11 +89,17 @@ enum HTMLFixtures: String, CaseIterable {
                     .padding()
                 }
             }</code></pre></br>
-            Followed by some plain code blocks</br>
-            <code>Hello, world!</code>
-            <code><b>Hello</b>, <i>world!</i></code>
-            <code><a href="https://www.matrix.org">This link should not be interpreted as such</a></code>
-            <code>And this https://www.matrix.org should be not highlighted</code>
+            Followed by some inline code</br>
+            <p>Plain text <code>code here</code> more text</p>
+            <p><code>Hello, world!</code></p>
+            <p><code><b>Hello</b>, <i>world!</i></code></p>
+            <p><code>&lt;b&gt;Hello&lt;/b&gt;, &lt;i&gt;world!&lt;/i&gt;</code></p>
+            <p><code><a href="https://www.matrix.org">This link should not be interpreted as such</a></code></p>
+            <p><code>And this https://www.matrix.org should be not highlighted</code></p>
+            """
+        case .wideCodeBlock:
+            """
+            <pre><code>CHHapticPattern.mm:487   +[CHHapticPattern patternForKey:error:]: Failed to read pattern library data: Error Domain=NSCocoaErrorDomain Code=260 "The file “hapticpatternlibrary.plist” couldn’t be opened because there is no such file." UserInfo={NSFilePath=/Library/Audio/Tunings/Generic/Haptics/Library/hapticpatternlibrary.plist, NSURL=file:///Library/Audio/Tunings/Generic/Haptics/Library/hapticpatternlibrary.plist, NSUnderlyingError=0x600000da69d0 {Error Domain=NSPOSIXErrorDomain Code=2 "No such file or directory"}}</code></pre>
             """
         case .unorderedList:
             """

ElementX/Sources/Screens/Timeline/View/Style/TimelineBubbleLayout.swift

@@ -10,8 +10,8 @@ import SwiftUI
 
 /// A custom layout used for quotes and content when using the bubbles timeline style.
 ///
-/// A custom layout is required as the embedded quote bubbles should fill the entire width of
-/// the message bubble, without causing the width of the bubble to fill all of the available space.
+/// A custom layout is required as the embedded quote bubbles and code blocks should fill the entire
+/// width of the message bubble, without causing the width of the bubble to fill all of the available space.
 struct TimelineBubbleLayout: Layout {
     struct Cache {
         var sizes = [Int: [ProposedViewSize: CGSize]]()
@@ -24,12 +24,14 @@ struct TimelineBubbleLayout: Layout {
     /// `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 that are only used for layout calculations.
-        static let hiddenQuote: Double = -1
-        /// The priority of visible quote bubbles that are placed in the view with a full width.
-        static let visibleQuote: Double = 0
+        /// 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 regularText: Double = 1
+        static let nonGreedyComponent: Double = 1
     }
     
     func makeCache(subviews: Subviews) -> Cache {
@@ -45,7 +47,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.visibleQuote }
+        let layoutSubviews = subviews.filter { $0.priority != Priority.visibleGreedyComponent }
 
         let subviewSizes = layoutSubviews.map { size(for: $0, subviews: subviews, proposedSize: proposal, cache: &cache) }
         
@@ -59,12 +61,12 @@ struct TimelineBubbleLayout: Layout {
     func placeSubviews(in bounds: CGRect, proposal: ProposedViewSize, subviews: Subviews, cache: inout Cache) {
         guard !subviews.isEmpty else { return }
         
-        // Calculate the width using the regular text and the non-greedy quote bubbles.
-        let layoutSubviews = subviews.filter { $0.priority != Priority.visibleQuote }
+        // 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 maxWidth = layoutSubviews.map { size(for: $0, subviews: subviews, proposedSize: proposal, cache: &cache).width }.reduce(0, max)
         
-        // Place the regular text and greedy quote bubbles using the calculated width.
-        let visibleSubviews = subviews.filter { $0.priority != Priority.hiddenQuote }
+        // Place the regular text and greedy components using the calculated width.
+        let visibleSubviews = subviews.filter { $0.priority != Priority.hiddenGreedyComponent }
 
         let subviewSizes = visibleSubviews.map { size(for: $0, subviews: subviews, proposedSize: ProposedViewSize(width: maxWidth, height: proposal.height), cache: &cache) }
         

ElementX/Sources/Screens/Timeline/View/Style/TimelineItemBubbledStylerView.swift

@@ -190,7 +190,7 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
             if !context.viewState.timelineKind.isThread, timelineItem.properties.isThreaded {
                 ThreadDecorator()
                     .padding(.leading, 4)
-                    .layoutPriority(TimelineBubbleLayout.Priority.regularText)
+                    .layoutPriority(TimelineBubbleLayout.Priority.nonGreedyComponent)
             }
             
             if let replyDetails = timelineItem.properties.replyDetails {
@@ -203,7 +203,7 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
                     .frame(maxWidth: .infinity, alignment: .leading)
                     .background(Color.compound.bgCanvasDefault)
                     .cornerRadius(8)
-                    .layoutPriority(TimelineBubbleLayout.Priority.visibleQuote)
+                    .layoutPriority(TimelineBubbleLayout.Priority.visibleGreedyComponent)
                     .onTapGesture {
                         if context.viewState.timelineKind != .pinned {
                             context.send(viewAction: .focusOnEventID(replyDetails.eventID))
@@ -214,12 +214,12 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
                 TimelineReplyView(placement: .timeline, timelineItemReplyDetails: replyDetails)
                     .fixedSize(horizontal: false, vertical: true)
                     .padding(4.0)
-                    .layoutPriority(TimelineBubbleLayout.Priority.hiddenQuote)
+                    .layoutPriority(TimelineBubbleLayout.Priority.hiddenGreedyComponent)
                     .hidden()
             }
             
             content()
-                .layoutPriority(TimelineBubbleLayout.Priority.regularText)
+                .layoutPriority(TimelineBubbleLayout.Priority.nonGreedyComponent)
                 .cornerRadius(timelineItem.contentCornerRadius)
         }
     }

ElementX/Sources/Screens/Timeline/View/TimelineItemViews/FormattedBodyText.swift

@@ -97,18 +97,20 @@ struct FormattedBodyText: View {
                                     .foregroundColor(.compound.textSecondary)
                                     .padding(.vertical, 2)
                             }
-                            .layoutPriority(TimelineBubbleLayout.Priority.visibleQuote)
+                            .layoutPriority(TimelineBubbleLayout.Priority.visibleGreedyComponent)
                     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)
-                        .fixedSize(horizontal: false, vertical: true)
                         .padding(.horizontal, 4)
-                        .layoutPriority(TimelineBubbleLayout.Priority.regularText)
+                        .layoutPriority(TimelineBubbleLayout.Priority.visibleGreedyComponent)
                         .contextMenu {
                             Button(L10n.actionCopy) {
                                 UIPasteboard.general.string = component.attributedString.string
@@ -118,20 +120,27 @@ struct FormattedBodyText: View {
                         MessageText(attributedString: component.attributedString)
                             .padding(.horizontal, 4)
                             .fixedSize(horizontal: false, vertical: true)
-                            .layoutPriority(TimelineBubbleLayout.Priority.regularText)
+                            .layoutPriority(TimelineBubbleLayout.Priority.nonGreedyComponent)
                     }
                 }
             }
             
-            // Make a second iteration through the components adding fixed width blockquotes
+            // Make a second iteration through the components adding fixed width blockquotes/codeblocks
             // which are used for layout calculations but won't be rendered.
             ForEach(attributedComponents) { component in
-                if case .blockquote = component.type {
+                switch component.type {
+                case .blockquote:
                     MessageText(attributedString: component.attributedString.mergingAttributes(blockquoteAttributes))
                         .fixedSize(horizontal: false, vertical: true)
                         .padding(.leading, 12.0)
-                        .layoutPriority(TimelineBubbleLayout.Priority.hiddenQuote)
+                        .layoutPriority(TimelineBubbleLayout.Priority.hiddenGreedyComponent)
                         .hidden()
+                case .codeBlock:
+                    HiddenCodeBlockScrollView(attributedString: component.attributedString)
+                        .layoutPriority(TimelineBubbleLayout.Priority.hiddenGreedyComponent)
+                        .hidden()
+                case .plainText:
+                    EmptyView()
                 }
             }
         }
@@ -147,51 +156,101 @@ struct FormattedBodyText: View {
         
         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 {
+        let attributedString: AttributedString
+        
+        @State private var maxSize: CGSize = .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 }
+            }
+            .frame(maxWidth: maxSize.width)
+            .padding(.horizontal, 4)
+        }
+    }
 }
 
 // MARK: - Previews
 
 struct FormattedBodyText_Previews: PreviewProvider, TestablePreview {
+    static let attributedStringBuilder = AttributedStringBuilder(cacheKey: "FormattedBodyText", mentionBuilder: MentionBuilder())
     static var previews: some View {
-        body(AttributedStringBuilder(cacheKey: "FormattedBodyText", mentionBuilder: MentionBuilder()))
+        htmlFixtures
+        
+        basicText
             .previewLayout(.sizeThatFits)
+            .previewDisplayName("basicText")
+        
+        singleColumnComponents
+            .previewLayout(.sizeThatFits)
+            .previewDisplayName("singleColumnComponents")
+    }
+    
+    static var basicText: some View {
+        VStack(alignment: .leading, spacing: 4.0) {
+            FormattedBodyText(attributedString: AttributedString("Some plain text wrapped in an AttributedString."))
+                .bubbleBackground()
+            
+            FormattedBodyText(text: "Some plain text that's not an attributed component.")
+                .bubbleBackground()
+            
+            FormattedBodyText(text: "❤️", boostFontSize: true)
+                .bubbleBackground()
+        }
+        .padding()
+    }
+    
+    /// A preview to help ensure that none of the component types we support result
+    /// in a bubble's width becoming wider than the natural width of its contents.
+    @ViewBuilder
+    static var singleColumnComponents: some View {
+        let html = """
+        <blockquote>A</blockquote>
+        <pre><code>B</code></pre>
+        <p>C</p>
+        """
+        
+        if let attributedString = attributedStringBuilder.fromHTML(html) {
+            FormattedBodyText(attributedString: attributedString)
+                .bubbleBackground()
+                .padding(4.0)
+        }
     }
     
     @ViewBuilder
-    static func body(_ attributedStringBuilder: AttributedStringBuilderProtocol) -> some View {
-        let htmlStrings = HTMLFixtures.allCases.map(\.rawValue)
+    static var htmlFixtures: some View {
+        let htmlFixtures = HTMLFixtures.allCases
         
-        ScrollView {
-            VStack(alignment: .leading, spacing: 4.0) {
-                ForEach(htmlStrings, id: \.self) { htmlString in
-                    HStack(alignment: .top, spacing: 0) {
-                        Text(htmlString)
-                            .frame(minWidth: 0, maxWidth: .infinity, alignment: .leading)
-                            .padding(4.0)
-                        
-                        Divider()
-                            .background(.black)
-                        
-                        if let attributedString = attributedStringBuilder.fromHTML(htmlString) {
-                            FormattedBodyText(attributedString: attributedString)
-                                .frame(minWidth: 0, maxWidth: .infinity, alignment: .leading)
-                                .bubbleBackground()
-                                .padding(4.0)
-                        }
-                    }
-                    .border(.black)
+        ForEach(htmlFixtures, id: \.rawValue) { htmlFixture in
+            HStack(alignment: .top, spacing: 0) {
+                let htmlString = htmlFixture.rawValue
+                Text(htmlString)
+                    .frame(minWidth: 0, maxWidth: .infinity, alignment: .leading)
+                    .padding(4.0)
+                
+                Divider()
+                    .background(.black)
+                
+                if let attributedString = attributedStringBuilder.fromHTML(htmlString) {
+                    FormattedBodyText(attributedString: attributedString)
+                        .frame(minWidth: 0, maxWidth: .infinity, alignment: .leading)
+                        .bubbleBackground()
+                        .padding(4.0)
                 }
-                
-                FormattedBodyText(attributedString: AttributedString("Some plain text wrapped in an AttributedString."))
-                    .bubbleBackground()
-                
-                FormattedBodyText(text: "Some plain text that's not an attributed component.")
-                    .bubbleBackground()
-                
-                FormattedBodyText(text: "❤️", boostFontSize: true)
-                    .bubbleBackground()
             }
+            .fixedSize(horizontal: false, vertical: true)
+            .border(.black)
             .padding()
+            .previewLayout(.sizeThatFits)
+            .previewDisplayName("\(htmlFixture)")
         }
     }
 }

PreviewTests/Sources/__Snapshots__/PreviewTests/formattedBodyText.iPad-en-GB-0.png

@@ -1,3 +0,0 @@
-version https://git-lfs.github.com/spec/v1
-oid sha256:c6a74364c92fe9499ded638a5522fc8df5bb9b7905a835d1d6db0324dbd4c9e0
-size 1072804

PreviewTests/Sources/__Snapshots__/PreviewTests/formattedBodyText.iPad-pseudo-0.png

@@ -1,3 +0,0 @@
-version https://git-lfs.github.com/spec/v1
-oid sha256:c6a74364c92fe9499ded638a5522fc8df5bb9b7905a835d1d6db0324dbd4c9e0
-size 1072804

PreviewTests/Sources/__Snapshots__/PreviewTests/formattedBodyText.iPhone-en-GB-0.png

@@ -1,3 +0,0 @@
-version https://git-lfs.github.com/spec/v1
-oid sha256:ac247697b25608e642347847d808e4b88ed45dfdcdef279e7147a95e894b78ca
-size 1186697

PreviewTests/Sources/__Snapshots__/PreviewTests/formattedBodyText.iPhone-pseudo-0.png

@@ -1,3 +0,0 @@
-version https://git-lfs.github.com/spec/v1
-oid sha256:ac247697b25608e642347847d808e4b88ed45dfdcdef279e7147a95e894b78ca
-size 1186697

UnitTests/Sources/AttributedStringBuilderTests.swift

@@ -32,7 +32,7 @@ class AttributedStringBuilderTests: XCTestCase {
     }
     
     func testRenderHTMLStringWithPreCode() {
-        guard let attributedString = attributedStringBuilder.fromHTML(HTMLFixtures.codeBlocks.rawValue) else {
+        guard let attributedString = attributedStringBuilder.fromHTML(HTMLFixtures.code.rawValue) else {
             XCTFail("Could not build the attributed string")
             return
         }
@@ -46,7 +46,7 @@ class AttributedStringBuilderTests: XCTestCase {
             return
         }
         
-        XCTAssertEqual(regex.numberOfMatches(in: string, options: [], range: .init(location: 0, length: string.count)), 13)
+        XCTAssertEqual(regex.numberOfMatches(in: string, options: [], range: .init(location: 0, length: string.count)), 23)
     }
     
     func testRenderHTMLStringWithLink() {
@@ -349,7 +349,7 @@ class AttributedStringBuilderTests: XCTestCase {
         }
         
         let coalescedComponents = attributedString.formattedComponents
-        XCTAssertEqual(coalescedComponents.count, 3)
+        XCTAssertEqual(coalescedComponents.count, 1)
         
         guard let component = coalescedComponents.first else {
             XCTFail("Could not get the first component")