Skip to content

Commit fa200cf

Browse files
authored
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.
1 parent 3d5269f commit fa200cf

File tree

67 files changed

+323
-82
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

67 files changed

+323
-82
lines changed

ElementX/Sources/Other/HTMLParsing/AttributedStringBuilder.swift

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ extension NSAttributedString.Key {
3232
static let MatrixEventOnRoomAlias: NSAttributedString.Key = .init(rawValue: EventOnRoomAliasAttribute.name)
3333
static let MatrixAllUsersMention: NSAttributedString.Key = .init(rawValue: AllUsersMentionAttribute.name)
3434
static let CodeBlock: NSAttributedString.Key = .init(rawValue: CodeBlockAttribute.name)
35+
static let InlineCode: NSAttributedString.Key = .init(rawValue: InlineCodeAttribute.name)
3536
}
3637

3738
struct AttributedStringBuilder: AttributedStringBuilderProtocol {
@@ -197,15 +198,24 @@ struct AttributedStringBuilder: AttributedStringBuilderProtocol {
197198
content.addAttribute(.MatrixBlockquote, value: true, range: NSRange(location: 0, length: content.length))
198199

199200
case "code", "pre":
200-
let preserveFormatting = preserveFormatting || tag == "pre"
201+
let isCodeBlock = tag == "pre"
202+
203+
let preserveFormatting = preserveFormatting || isCodeBlock
201204
content = attributedString(element: childElement, documentBody: documentBody, preserveFormatting: preserveFormatting, listTag: listTag, listIndex: &childIndex, indentLevel: indentLevel)
202205

203206
let fontPointSize = fontPointSize * 0.9 // Intentionally shrink code blocks by 10%
204207
content.setFontPreservingSymbolicTraits(UIFont.monospacedSystemFont(ofSize: fontPointSize, weight: .regular))
205208

206-
content.addAttribute(.CodeBlock, value: true, range: NSRange(location: 0, length: content.length))
209+
if isCodeBlock {
210+
content.addAttribute(.CodeBlock, value: true, range: NSRange(location: 0, length: content.length))
211+
// The scroll view provides the background colour for code blocks.
212+
} else {
213+
content.addAttribute(.InlineCode, value: true, range: NSRange(location: 0, length: content.length))
214+
// But inline code is (obviously) inline so it's much easier to set the background colour here.
215+
content.addAttribute(.backgroundColor, value: UIColor.compound._bgCodeBlock as Any, range: NSRange(location: 0, length: content.length))
216+
}
207217

208-
// Don't allow identifiers or links in code blocks
218+
// Don't allow identifiers or links in code.
209219
content.removeAttribute(.MatrixRoomID, range: NSRange(location: 0, length: content.length))
210220
content.removeAttribute(.MatrixRoomAlias, range: NSRange(location: 0, length: content.length))
211221
content.removeAttribute(.MatrixUserID, range: NSRange(location: 0, length: content.length))
@@ -353,14 +363,15 @@ struct AttributedStringBuilder: AttributedStringBuilderProtocol {
353363
// Sort the links by length so the longest one always takes priority
354364
matches.sorted { $0.range.length > $1.range.length }.forEach { [attributedString] match in
355365
// Don't highlight links within codeblocks
356-
let isInCodeBlock = attributedString.attribute(.CodeBlock, at: match.range.location, effectiveRange: nil) != nil
357-
if isInCodeBlock {
366+
let isCode = attributedString.attribute(.CodeBlock, at: match.range.location, effectiveRange: nil) != nil
367+
|| attributedString.attribute(.InlineCode, at: match.range.location, effectiveRange: nil) != nil
368+
if isCode {
358369
return
359370
}
360371

361372
var hasLink = false
362373
attributedString.enumerateAttribute(.link, in: match.range, options: []) { value, _, stop in
363-
if value != nil, !isInCodeBlock {
374+
if value != nil, !isCode {
364375
hasLink = true
365376
stop.pointee = true
366377
}

ElementX/Sources/Other/HTMLParsing/ElementXAttributeScope.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,11 @@ enum CodeBlockAttribute: AttributedStringKey {
6868
static let name = "MXCodeBlockAttribute"
6969
}
7070

71+
enum InlineCodeAttribute: AttributedStringKey {
72+
typealias Value = Bool
73+
static let name = "MXInlineCodeAttribute"
74+
}
75+
7176
// periphery: ignore - required to make NSAttributedString to AttributedString conversion even if not used directly
7277
extension AttributeScopes {
7378
struct ElementXAttributes: AttributeScope {
@@ -84,6 +89,7 @@ extension AttributeScopes {
8489
let allUsersMention: AllUsersMentionAttribute
8590

8691
let codeBlock: CodeBlockAttribute
92+
let inlineCode: InlineCodeAttribute
8793

8894
let swiftUI: SwiftUIAttributes
8995
let uiKit: UIKitAttributes

ElementX/Sources/Other/HTMLParsing/HTMLFixtures.swift

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ enum HTMLFixtures: String, CaseIterable {
1515
case textFormatting
1616
case groupedBlockQuotes
1717
case separatedBlockQuotes
18-
case codeBlocks
18+
case code
19+
case wideCodeBlock
1920
case unorderedList
2021
case orderedList
2122

@@ -74,9 +75,9 @@ enum HTMLFixtures: String, CaseIterable {
7475
<blockquote>Some other blockquote</blockquote>\
7576
Text after second blockquote
7677
"""
77-
case .codeBlocks:
78+
case .code:
7879
"""
79-
<pre>A preformatted code block
80+
<pre>A pre-formatted code block
8081
<code>struct ContentView: View {
8182
var body: some View {
8283
VStack {
@@ -88,11 +89,17 @@ enum HTMLFixtures: String, CaseIterable {
8889
.padding()
8990
}
9091
}</code></pre></br>
91-
Followed by some plain code blocks</br>
92-
<code>Hello, world!</code>
93-
<code><b>Hello</b>, <i>world!</i></code>
94-
<code><a href="https://www.matrix.org">This link should not be interpreted as such</a></code>
95-
<code>And this https://www.matrix.org should be not highlighted</code>
92+
Followed by some inline code</br>
93+
<p>Plain text <code>code here</code> more text</p>
94+
<p><code>Hello, world!</code></p>
95+
<p><code><b>Hello</b>, <i>world!</i></code></p>
96+
<p><code>&lt;b&gt;Hello&lt;/b&gt;, &lt;i&gt;world!&lt;/i&gt;</code></p>
97+
<p><code><a href="https://www.matrix.org">This link should not be interpreted as such</a></code></p>
98+
<p><code>And this https://www.matrix.org should be not highlighted</code></p>
99+
"""
100+
case .wideCodeBlock:
101+
"""
102+
<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>
96103
"""
97104
case .unorderedList:
98105
"""

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

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ import SwiftUI
1010

1111
/// A custom layout used for quotes and content when using the bubbles timeline style.
1212
///
13-
/// A custom layout is required as the embedded quote bubbles should fill the entire width of
14-
/// the message bubble, without causing the width of the bubble to fill all of the available space.
13+
/// A custom layout is required as the embedded quote bubbles and code blocks should fill the entire
14+
/// width of the message bubble, without causing the width of the bubble to fill all of the available space.
1515
struct TimelineBubbleLayout: Layout {
1616
struct Cache {
1717
var sizes = [Int: [ProposedViewSize: CGSize]]()
@@ -24,12 +24,14 @@ struct TimelineBubbleLayout: Layout {
2424
/// `TimelineBubbleLayout` to create the layout we would like. They aren't
2525
/// used in the expected way that SwiftUI would normally use layout priorities.
2626
enum Priority {
27-
/// The priority of hidden quote bubbles that are only used for layout calculations.
28-
static let hiddenQuote: Double = -1
29-
/// The priority of visible quote bubbles that are placed in the view with a full width.
30-
static let visibleQuote: Double = 0
27+
/// The priority of hidden quote bubbles/code blocks that are only used for layout calculations.
28+
/// Any views given this priority should be made non-greedy for the calculations to work.
29+
static let hiddenGreedyComponent: Double = -1
30+
/// The priority of visible quote bubbles/code blocks that are placed in the view with a full width.
31+
/// Any views given this priority should remain in their normal greedy form.
32+
static let visibleGreedyComponent: Double = 0
3133
/// The priority of regular text that is used for layout calculations and placed in the view.
32-
static let regularText: Double = 1
34+
static let nonGreedyComponent: Double = 1
3335
}
3436

3537
func makeCache(subviews: Subviews) -> Cache {
@@ -45,7 +47,7 @@ struct TimelineBubbleLayout: Layout {
4547
guard !subviews.isEmpty else { return .zero }
4648

4749
// Calculate the natural size using the regular text and non-greedy quote bubbles.
48-
let layoutSubviews = subviews.filter { $0.priority != Priority.visibleQuote }
50+
let layoutSubviews = subviews.filter { $0.priority != Priority.visibleGreedyComponent }
4951

5052
let subviewSizes = layoutSubviews.map { size(for: $0, subviews: subviews, proposedSize: proposal, cache: &cache) }
5153

@@ -59,12 +61,12 @@ struct TimelineBubbleLayout: Layout {
5961
func placeSubviews(in bounds: CGRect, proposal: ProposedViewSize, subviews: Subviews, cache: inout Cache) {
6062
guard !subviews.isEmpty else { return }
6163

62-
// Calculate the width using the regular text and the non-greedy quote bubbles.
63-
let layoutSubviews = subviews.filter { $0.priority != Priority.visibleQuote }
64+
// Calculate the width using the regular text along with non-greedy versions of any greedy components.
65+
let layoutSubviews = subviews.filter { $0.priority != Priority.visibleGreedyComponent }
6466
let maxWidth = layoutSubviews.map { size(for: $0, subviews: subviews, proposedSize: proposal, cache: &cache).width }.reduce(0, max)
6567

66-
// Place the regular text and greedy quote bubbles using the calculated width.
67-
let visibleSubviews = subviews.filter { $0.priority != Priority.hiddenQuote }
68+
// Place the regular text and greedy components using the calculated width.
69+
let visibleSubviews = subviews.filter { $0.priority != Priority.hiddenGreedyComponent }
6870

6971
let subviewSizes = visibleSubviews.map { size(for: $0, subviews: subviews, proposedSize: ProposedViewSize(width: maxWidth, height: proposal.height), cache: &cache) }
7072

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
190190
if !context.viewState.timelineKind.isThread, timelineItem.properties.isThreaded {
191191
ThreadDecorator()
192192
.padding(.leading, 4)
193-
.layoutPriority(TimelineBubbleLayout.Priority.regularText)
193+
.layoutPriority(TimelineBubbleLayout.Priority.nonGreedyComponent)
194194
}
195195

196196
if let replyDetails = timelineItem.properties.replyDetails {
@@ -203,7 +203,7 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
203203
.frame(maxWidth: .infinity, alignment: .leading)
204204
.background(Color.compound.bgCanvasDefault)
205205
.cornerRadius(8)
206-
.layoutPriority(TimelineBubbleLayout.Priority.visibleQuote)
206+
.layoutPriority(TimelineBubbleLayout.Priority.visibleGreedyComponent)
207207
.onTapGesture {
208208
if context.viewState.timelineKind != .pinned {
209209
context.send(viewAction: .focusOnEventID(replyDetails.eventID))
@@ -214,12 +214,12 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
214214
TimelineReplyView(placement: .timeline, timelineItemReplyDetails: replyDetails)
215215
.fixedSize(horizontal: false, vertical: true)
216216
.padding(4.0)
217-
.layoutPriority(TimelineBubbleLayout.Priority.hiddenQuote)
217+
.layoutPriority(TimelineBubbleLayout.Priority.hiddenGreedyComponent)
218218
.hidden()
219219
}
220220

221221
content()
222-
.layoutPriority(TimelineBubbleLayout.Priority.regularText)
222+
.layoutPriority(TimelineBubbleLayout.Priority.nonGreedyComponent)
223223
.cornerRadius(timelineItem.contentCornerRadius)
224224
}
225225
}

0 commit comments

Comments
 (0)