From 760ac9f807531c61f141df432cab27d5f52bb040 Mon Sep 17 00:00:00 2001 From: Steve Streza Date: Thu, 13 Aug 2026 15:50:59 -0700 Subject: [PATCH 1/2] Add normalization process for RichTextElement --- .../RichTextElement+Normalizing.swift | 52 +++++++ .../RichTextGQLConversions.swift | 2 +- .../RichTextElement+NormalizingTests.swift | 143 ++++++++++++++++++ 3 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 ServerDrivenUI/Sources/ServerDrivenUI/RichTextElement+Normalizing.swift create mode 100644 ServerDrivenUI/Tests/ServerDrivenUITests/RichTextElement+NormalizingTests.swift diff --git a/ServerDrivenUI/Sources/ServerDrivenUI/RichTextElement+Normalizing.swift b/ServerDrivenUI/Sources/ServerDrivenUI/RichTextElement+Normalizing.swift new file mode 100644 index 0000000000..2d59c65ae7 --- /dev/null +++ b/ServerDrivenUI/Sources/ServerDrivenUI/RichTextElement+Normalizing.swift @@ -0,0 +1,52 @@ +import Foundation + +extension Array where Element == RichTextElement { + /// The server's rich text parser sits on top of an HTML parser, so a `
` containing an + /// inline `` (e.g. `
Here's an image: Isn't it nice?
`) is represented + /// as a single `Text` element whose `children` are `[Text, Photo, Text]`. Our conversion + /// architecture maps each GraphQL node to exactly one `RichTextElement`, and `TextBlock` + /// renders a `Text` element as a single attributed string, which can't host a `Photo`. This + /// splits any such `Text` into standalone sibling elements so the `Photo` renders on its own, + /// relying on the server's guarantee that this nesting never goes deeper than one level. + func withNormalizedNestedElements() -> [RichTextElement] { + self.flatMap { $0.withNormalizedNestedElements() } + } +} + +extension RichTextElement { + func withNormalizedNestedElements() -> [RichTextElement] { + guard case let .text(text, header) = self, text.children.contains(where: { $0.isPhoto }) else { + return [self] + } + + var elements: [RichTextElement] = [] + var currentChildren: [RichTextElement] = text.text.isEmpty + ? [] + : [.text(RichTextElement.Text(text: text.text, link: text.link, styles: text.styles), nil)] + + for child in text.children { + if child.isPhoto { + elements.append(.text(RichTextElement.Text(text: "", children: currentChildren), header)) + elements.append(child) + currentChildren = [] + } else { + currentChildren.append(child) + } + } + elements.append(.text(RichTextElement.Text(text: "", children: currentChildren), header)) + + return elements.filter { !$0.isEmptyText } + } + + private var isPhoto: Bool { + if case .photo = self { return true } + return false + } + + private var isEmptyText: Bool { + if case let .text(text, _) = self { + return text.text.isEmpty && text.children.isEmpty + } + return false + } +} diff --git a/ServerDrivenUI/Sources/ServerDrivenUI/RichTextGQLConversions.swift b/ServerDrivenUI/Sources/ServerDrivenUI/RichTextGQLConversions.swift index a0c2363d1d..c7c60aa4bb 100644 --- a/ServerDrivenUI/Sources/ServerDrivenUI/RichTextGQLConversions.swift +++ b/ServerDrivenUI/Sources/ServerDrivenUI/RichTextGQLConversions.swift @@ -3,7 +3,7 @@ import GraphAPI extension RichTextComponentFragment { public func asRichTextElements() -> [RichTextElement] { - self.items.map { $0.asRichTextElement } + self.items.map { $0.asRichTextElement }.withNormalizedNestedElements() } } diff --git a/ServerDrivenUI/Tests/ServerDrivenUITests/RichTextElement+NormalizingTests.swift b/ServerDrivenUI/Tests/ServerDrivenUITests/RichTextElement+NormalizingTests.swift new file mode 100644 index 0000000000..158a32f056 --- /dev/null +++ b/ServerDrivenUI/Tests/ServerDrivenUITests/RichTextElement+NormalizingTests.swift @@ -0,0 +1,143 @@ +import Foundation +@testable import ServerDrivenUI +import XCTest + +final class RichTextElementNormalizingTests: XCTestCase { + /* Text with no children is left unchanged */ + func testLeavesPlainTextUnchanged() throws { + let element = RichTextElement.text(.init(text: "hello"), nil) + let result = element.withNormalizedNestedElements() + XCTAssertEqual(result, [element]) + } + + /* Text whose children contain no Photo is left unchanged */ + func testLeavesTextWithNonPhotoChildrenUnchanged() throws { + let element = RichTextElement.text( + .init(text: "", children: [.text(.init(text: "a"), nil), .text(.init(text: "b"), nil)]), + nil + ) + let result = element.withNormalizedNestedElements() + XCTAssertEqual(result, [element]) + } + + /* Non-text elements are left unchanged */ + func testLeavesNonTextElementsUnchanged() throws { + let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil) + let element = RichTextElement.photo(photo) + XCTAssertEqual(element.withNormalizedNestedElements(), [element]) + } + + /* Text/Photo/Text children split into three sibling elements */ + func testSplitsTextPhotoTextIntoThreeElements() throws { + let photo = RichTextElement.Photo(altText: "alt", assetID: "1", caption: "cap", url: "https://img") + let element = RichTextElement.text( + .init(text: "", children: [ + .text(.init(text: "Here's an image: "), nil), + .photo(photo), + .text(.init(text: " Isn't it nice?"), nil) + ]), + nil + ) + + let result = element.withNormalizedNestedElements() + XCTAssertEqual(result.count, 3) + + guard case let .text(first, _) = result[0] else { return XCTFail("expected .text first") } + XCTAssertEqual(first.children, [.text(.init(text: "Here's an image: "), nil)]) + + guard case let .photo(middle) = result[1] else { return XCTFail("expected .photo second") } + XCTAssertEqual(middle, photo) + + guard case let .text(last, _) = result[2] else { return XCTFail("expected .text third") } + XCTAssertEqual(last.children, [.text(.init(text: " Isn't it nice?"), nil)]) + } + + /* Leading Photo with no preceding text does not produce an empty leading Text */ + func testLeadingPhotoProducesNoEmptyLeadingText() throws { + let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil) + let element = RichTextElement.text( + .init(text: "", children: [.photo(photo), .text(.init(text: "after"), nil)]), + nil + ) + + let result = element.withNormalizedNestedElements() + XCTAssertEqual(result.count, 2) + guard case .photo = result[0] else { return XCTFail("expected .photo first") } + guard case let .text(last, _) = result[1] else { return XCTFail("expected .text second") } + XCTAssertEqual(last.children, [.text(.init(text: "after"), nil)]) + } + + /* Trailing Photo with no following text does not produce an empty trailing Text */ + func testTrailingPhotoProducesNoEmptyTrailingText() throws { + let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil) + let element = RichTextElement.text( + .init(text: "", children: [.text(.init(text: "before"), nil), .photo(photo)]), + nil + ) + + let result = element.withNormalizedNestedElements() + XCTAssertEqual(result.count, 2) + guard case let .text(first, _) = result[0] else { return XCTFail("expected .text first") } + XCTAssertEqual(first.children, [.text(.init(text: "before"), nil)]) + guard case .photo = result[1] else { return XCTFail("expected .photo second") } + } + + /* Consecutive Photos produce no empty Text between them */ + func testConsecutivePhotosProduceNoEmptyTextBetween() throws { + let photo1 = RichTextElement.Photo(altText: "1", assetID: nil, caption: nil, url: nil) + let photo2 = RichTextElement.Photo(altText: "2", assetID: nil, caption: nil, url: nil) + let element = RichTextElement.text(.init(text: "", children: [.photo(photo1), .photo(photo2)]), nil) + + let result = element.withNormalizedNestedElements() + XCTAssertEqual(result, [.photo(photo1), .photo(photo2)]) + } + + /* The Text element's own text/link/styles are preserved as a leading segment */ + func testPreservesOwnTextAsLeadingSegment() throws { + let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil) + let link = URL(string: "https://kickstarter.com") + let element = RichTextElement.text( + .init(text: "prefix", link: link, styles: [.strong], children: [.photo(photo)]), + nil + ) + + let result = element.withNormalizedNestedElements() + XCTAssertEqual(result.count, 2) + guard case let .text(first, _) = result[0] else { return XCTFail("expected .text first") } + XCTAssertEqual(first.children, [.text(.init(text: "prefix", link: link, styles: [.strong]), nil)]) + guard case .photo = result[1] else { return XCTFail("expected .photo second") } + } + + /* Header level is preserved across all split segments */ + func testPreservesHeaderLevelAcrossSplits() throws { + let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil) + let element = RichTextElement.text( + .init(text: "", children: [.text(.init(text: "a"), nil), .photo(photo), .text(.init(text: "b"), nil)]), + .two + ) + + let result = element.withNormalizedNestedElements() + XCTAssertEqual(result.count, 3) + guard case let .text(_, headerA) = result[0] else { return XCTFail("expected .text first") } + XCTAssertEqual(headerA, .two) + guard case let .text(_, headerB) = result[2] else { return XCTFail("expected .text third") } + XCTAssertEqual(headerB, .two) + } + + /* Array-level splitting flattens across multiple top-level elements, preserving surrounding elements */ + func testArraySplittingFlattensAcrossElements() throws { + let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil) + let textWithNestedPhoto = RichTextElement.text( + .init(text: "", children: [.text(.init(text: "a"), nil), .photo(photo), .text(.init(text: "b"), nil)]), + nil + ) + let plainText = RichTextElement.text(.init(text: "plain"), nil) + let elements: [RichTextElement] = [plainText, textWithNestedPhoto, .listItemOpen] + + let result = elements.withNormalizedNestedElements() + + XCTAssertEqual(result.count, 5) + XCTAssertEqual(result[0], plainText) + XCTAssertEqual(result[4], .listItemOpen) + } +} From 12bfd4e781734d1fa38bb4368626b9bf206c7d15 Mon Sep 17 00:00:00 2001 From: Steve Streza Date: Wed, 19 Aug 2026 23:05:16 -0700 Subject: [PATCH 2/2] Fix linking --- .../ServerDrivenUI/Blocks/ImageBlock.swift | 16 +++++- .../RichTextElement+Normalizing.swift | 25 +++++++++- .../ServerDrivenUI/RichTextElement.swift | 6 +++ .../RichTextGQLConversions.swift | 12 +++-- .../Blocks/ImageBlockTests.swift | 34 ++++++++++++- .../RichTextElement+NormalizingTests.swift | 50 +++++++++++++++---- 6 files changed, 126 insertions(+), 17 deletions(-) diff --git a/ServerDrivenUI/Sources/ServerDrivenUI/Blocks/ImageBlock.swift b/ServerDrivenUI/Sources/ServerDrivenUI/Blocks/ImageBlock.swift index 21f4c92451..6e3f8439ac 100644 --- a/ServerDrivenUI/Sources/ServerDrivenUI/Blocks/ImageBlock.swift +++ b/ServerDrivenUI/Sources/ServerDrivenUI/Blocks/ImageBlock.swift @@ -5,6 +5,7 @@ import SwiftUI struct ImageBlock: View { var photo: RichTextElement.Photo @Environment(\.richTextStyle) var style: any RichTextStyle + @Environment(\.openURL) private var openURL private var imageURL: URL? { guard let urlString = photo.url, !urlString.isEmpty else { @@ -13,7 +14,7 @@ struct ImageBlock: View { return URL(string: urlString) } - public var body: some View { + @ViewBuilder private var image: some View { Group { if let imageURL { KFAnimatedImage(imageURL) @@ -33,4 +34,17 @@ struct ImageBlock: View { } } } + + public var body: some View { + if let link = photo.link { + Button { + self.openURL(link) + } label: { + self.image + } + .buttonStyle(.plain) + } else { + self.image + } + } } diff --git a/ServerDrivenUI/Sources/ServerDrivenUI/RichTextElement+Normalizing.swift b/ServerDrivenUI/Sources/ServerDrivenUI/RichTextElement+Normalizing.swift index 2d59c65ae7..ca763ffc37 100644 --- a/ServerDrivenUI/Sources/ServerDrivenUI/RichTextElement+Normalizing.swift +++ b/ServerDrivenUI/Sources/ServerDrivenUI/RichTextElement+Normalizing.swift @@ -27,7 +27,7 @@ extension RichTextElement { for child in text.children { if child.isPhoto { elements.append(.text(RichTextElement.Text(text: "", children: currentChildren), header)) - elements.append(child) + elements.append(child.withLink(text.link)) currentChildren = [] } else { currentChildren.append(child) @@ -43,6 +43,29 @@ extension RichTextElement { return false } + /// Sometimes the server returns a Text block with a child + private func withLink(_ link: URL?) -> RichTextElement { + switch self { + case let .text(text, header): + return .text(RichTextElement.Text( + text: text.text, + link: link, + styles: text.styles, + children: text.children + ), header) + case let .photo(photo): + return .photo(RichTextElement.Photo( + altText: photo.altText, + assetID: photo.assetID, + caption: photo.caption, + url: photo.url, + link: link + )) + case .listItemOpen, .listItemClose, .listItem, .audio, .video, .oembed, .unknown: + return self + } + } + private var isEmptyText: Bool { if case let .text(text, _) = self { return text.text.isEmpty && text.children.isEmpty diff --git a/ServerDrivenUI/Sources/ServerDrivenUI/RichTextElement.swift b/ServerDrivenUI/Sources/ServerDrivenUI/RichTextElement.swift index 3623cff402..bc2da87175 100644 --- a/ServerDrivenUI/Sources/ServerDrivenUI/RichTextElement.swift +++ b/ServerDrivenUI/Sources/ServerDrivenUI/RichTextElement.swift @@ -88,6 +88,12 @@ public indirect enum RichTextElement: Sendable, Equatable { let assetID: String? let caption: String? let url: String? + + /// The link this photo should navigate to when tapped. `RichTextPhoto` itself has no + /// `link` field; the server represents a linkable image as a `RichText` node whose `link` + /// wraps a `RichTextPhoto` child, so this is populated during normalization from the + /// enclosing text's link rather than from the photo's own GraphQL fields. + let link: URL? } public struct VideoFormat: Sendable, Equatable { diff --git a/ServerDrivenUI/Sources/ServerDrivenUI/RichTextGQLConversions.swift b/ServerDrivenUI/Sources/ServerDrivenUI/RichTextGQLConversions.swift index c7c60aa4bb..5ca5bcee21 100644 --- a/ServerDrivenUI/Sources/ServerDrivenUI/RichTextGQLConversions.swift +++ b/ServerDrivenUI/Sources/ServerDrivenUI/RichTextGQLConversions.swift @@ -244,7 +244,8 @@ extension RichTextComponentFragment.Item.AsRichTextPhoto { altText: altText, assetID: asset?.id, caption: caption, - url: asset?.url + url: asset?.url, + link: nil )) } } @@ -255,7 +256,8 @@ extension RichTextComponentFragment.Item.AsRichText.Child.AsRichTextPhoto { altText: altText, assetID: asset?.id, caption: caption, - url: asset?.url + url: asset?.url, + link: nil )) } } @@ -266,7 +268,8 @@ extension RichTextComponentFragment.Item.AsRichTextHeader.Child.AsRichTextPhoto altText: altText, assetID: asset?.id, caption: caption, - url: asset?.url + url: asset?.url, + link: nil )) } } @@ -277,7 +280,8 @@ extension RichTextComponentFragment.Item.AsRichTextListItem.Child.AsRichTextPhot altText: altText, assetID: asset?.id, caption: caption, - url: asset?.url + url: asset?.url, + link: nil )) } } diff --git a/ServerDrivenUI/Tests/ServerDrivenUITests/Blocks/ImageBlockTests.swift b/ServerDrivenUI/Tests/ServerDrivenUITests/Blocks/ImageBlockTests.swift index 46338268c2..d3161e18d5 100644 --- a/ServerDrivenUI/Tests/ServerDrivenUITests/Blocks/ImageBlockTests.swift +++ b/ServerDrivenUI/Tests/ServerDrivenUITests/Blocks/ImageBlockTests.swift @@ -151,6 +151,35 @@ final class ImageBlockTests: XCTestCase { ) } + func testImageBlockWithLink_wrapsImageInButton() throws { + let photo = makePhoto( + altText: "Linked image", + url: testImageURL().absoluteString, + link: URL(string: "https://backercrew.com/submit") + ) + + let view = imageBlock(photo: photo, colorScheme: .light) + + XCTAssertNoThrow( + try view.inspect().find(ViewType.Button.self), + "Expected ImageBlock to wrap its image in a Button when photo.link is set." + ) + } + + func testImageBlockWithoutLink_doesNotWrapImageInButton() throws { + let photo = makePhoto( + altText: "Unlinked image", + url: testImageURL().absoluteString + ) + + let view = imageBlock(photo: photo, colorScheme: .light) + + XCTAssertThrowsError( + try view.inspect().find(ViewType.Button.self), + "Expected ImageBlock not to wrap its image in a Button when photo.link is nil." + ) + } + func testImageBlockWithValidURL_respectsContainerFrame() throws { let photo = makePhoto( altText: "Test image", @@ -180,12 +209,13 @@ final class ImageBlockTests: XCTestCase { } } -private func makePhoto(altText: String?, url: String?) -> RichTextElement.Photo { +private func makePhoto(altText: String?, url: String?, link: URL? = nil) -> RichTextElement.Photo { RichTextElement.Photo( altText: altText, assetID: "123", caption: "Test caption", - url: url + url: url, + link: link ) } diff --git a/ServerDrivenUI/Tests/ServerDrivenUITests/RichTextElement+NormalizingTests.swift b/ServerDrivenUI/Tests/ServerDrivenUITests/RichTextElement+NormalizingTests.swift index 158a32f056..4264b9f9e1 100644 --- a/ServerDrivenUI/Tests/ServerDrivenUITests/RichTextElement+NormalizingTests.swift +++ b/ServerDrivenUI/Tests/ServerDrivenUITests/RichTextElement+NormalizingTests.swift @@ -22,14 +22,20 @@ final class RichTextElementNormalizingTests: XCTestCase { /* Non-text elements are left unchanged */ func testLeavesNonTextElementsUnchanged() throws { - let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil) + let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil, link: nil) let element = RichTextElement.photo(photo) XCTAssertEqual(element.withNormalizedNestedElements(), [element]) } /* Text/Photo/Text children split into three sibling elements */ func testSplitsTextPhotoTextIntoThreeElements() throws { - let photo = RichTextElement.Photo(altText: "alt", assetID: "1", caption: "cap", url: "https://img") + let photo = RichTextElement.Photo( + altText: "alt", + assetID: "1", + caption: "cap", + url: "https://img", + link: nil + ) let element = RichTextElement.text( .init(text: "", children: [ .text(.init(text: "Here's an image: "), nil), @@ -54,7 +60,7 @@ final class RichTextElementNormalizingTests: XCTestCase { /* Leading Photo with no preceding text does not produce an empty leading Text */ func testLeadingPhotoProducesNoEmptyLeadingText() throws { - let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil) + let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil, link: nil) let element = RichTextElement.text( .init(text: "", children: [.photo(photo), .text(.init(text: "after"), nil)]), nil @@ -69,7 +75,7 @@ final class RichTextElementNormalizingTests: XCTestCase { /* Trailing Photo with no following text does not produce an empty trailing Text */ func testTrailingPhotoProducesNoEmptyTrailingText() throws { - let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil) + let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil, link: nil) let element = RichTextElement.text( .init(text: "", children: [.text(.init(text: "before"), nil), .photo(photo)]), nil @@ -84,8 +90,8 @@ final class RichTextElementNormalizingTests: XCTestCase { /* Consecutive Photos produce no empty Text between them */ func testConsecutivePhotosProduceNoEmptyTextBetween() throws { - let photo1 = RichTextElement.Photo(altText: "1", assetID: nil, caption: nil, url: nil) - let photo2 = RichTextElement.Photo(altText: "2", assetID: nil, caption: nil, url: nil) + let photo1 = RichTextElement.Photo(altText: "1", assetID: nil, caption: nil, url: nil, link: nil) + let photo2 = RichTextElement.Photo(altText: "2", assetID: nil, caption: nil, url: nil, link: nil) let element = RichTextElement.text(.init(text: "", children: [.photo(photo1), .photo(photo2)]), nil) let result = element.withNormalizedNestedElements() @@ -94,7 +100,7 @@ final class RichTextElementNormalizingTests: XCTestCase { /* The Text element's own text/link/styles are preserved as a leading segment */ func testPreservesOwnTextAsLeadingSegment() throws { - let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil) + let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil, link: nil) let link = URL(string: "https://kickstarter.com") let element = RichTextElement.text( .init(text: "prefix", link: link, styles: [.strong], children: [.photo(photo)]), @@ -108,9 +114,35 @@ final class RichTextElementNormalizingTests: XCTestCase { guard case .photo = result[1] else { return XCTFail("expected .photo second") } } + /* The enclosing text's link is applied to a Photo child, since RichTextPhoto itself has no + * link field — the server represents a linkable image as a RichText node whose link wraps + * a RichTextPhoto child. */ + func testAppliesEnclosingLinkToPhotoChild() throws { + let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: "https://img", link: nil) + let link = URL(string: "https://backercrew.com/submit") + let element = RichTextElement.text(.init(text: "", link: link, children: [.photo(photo)]), nil) + + let result = element.withNormalizedNestedElements() + XCTAssertEqual(result.count, 1) + guard case let .photo(linkedPhoto) = result[0] else { return XCTFail("expected .photo") } + XCTAssertEqual(linkedPhoto.link, link) + XCTAssertEqual(linkedPhoto.url, photo.url) + } + + /* A Photo child is left without a link when the enclosing text has none */ + func testLeavesPhotoWithoutLinkWhenEnclosingTextHasNoLink() throws { + let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: "https://img", link: nil) + let element = RichTextElement.text(.init(text: "", children: [.photo(photo)]), nil) + + let result = element.withNormalizedNestedElements() + XCTAssertEqual(result.count, 1) + guard case let .photo(unlinkedPhoto) = result[0] else { return XCTFail("expected .photo") } + XCTAssertNil(unlinkedPhoto.link) + } + /* Header level is preserved across all split segments */ func testPreservesHeaderLevelAcrossSplits() throws { - let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil) + let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil, link: nil) let element = RichTextElement.text( .init(text: "", children: [.text(.init(text: "a"), nil), .photo(photo), .text(.init(text: "b"), nil)]), .two @@ -126,7 +158,7 @@ final class RichTextElementNormalizingTests: XCTestCase { /* Array-level splitting flattens across multiple top-level elements, preserving surrounding elements */ func testArraySplittingFlattensAcrossElements() throws { - let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil) + let photo = RichTextElement.Photo(altText: nil, assetID: nil, caption: nil, url: nil, link: nil) let textWithNestedPhoto = RichTextElement.text( .init(text: "", children: [.text(.init(text: "a"), nil), .photo(photo), .text(.init(text: "b"), nil)]), nil