Commit `368c59e3f8`

368c59e3f89abac479ed121076e6a42d5ce17d14

parent: 1b7270d767

Unsigned

cmc <hello@cleberg.net> · 2026-07-16 03:02 UTC

fix: collapse patches to stop a recursive layout loop

Opening a patchset wedged the app. UICollectionView reported a row oscillating
between 3674pt and 1647pt and trapped in a recursive layout loop, leaving the
UI unresponsive.

The detail view rendered every patch in the series expanded, so a List held one
enormous self-sizing row per patch, each with a full diff. Self-sizing cells
that large do not settle.

Patches now start collapsed and expand on tap, so at most the ones a reviewer
opens are measured. This is what ThreadDetailView already does — it collapses
every message but the last, and renders the same diffs through the same
DiffView without trouble. Reviewing a series one patch at a time is also closer
to how the reading actually goes.

The rendering of a block list is shared between the cover letter and patches
rather than duplicated.

Layout: unified · split

Hutch/Views/Patchsets/PatchsetDetailView.swift +89 −15

		@@ -7,6 +7,7 @@ struct PatchsetDetailView: View {
7	7	@Environment(AppState.self) private var appState
8	8	@State private var viewModel: PatchsetDetailViewModel?
9	9	@State private var showStatusPicker = false
	10	@State private var expandedPatchIDs: Set<Int> = []
10	11
11	12	var body: some View {
12	13	Group {
		@@ -163,10 +164,31 @@ struct PatchsetDetailView: View {
163	164	}
164	165	}
165	166
	167	/// Patches start collapsed.
	168	///
	169	/// A diff is tall, and a series is many of them. Rendering every patch expanded
	170	/// puts a dozen self-sizing diffs in one List, which drives UICollectionView
	171	/// into a recursive layout loop and wedges the app. The inbox thread view
	172	/// collapses all but the last message for the same reason.
166	173	@ViewBuilder
167	174	private func patchesSection(_ patchset: PatchsetDetail) -> some View {
168		ForEach(patchset.patches) { patch in
169		emailSection(patch, title: patch.seriesLabel.map { "Patch \($0)" } ?? "Patch")
	175	Section("Patches") {
	176	ForEach(patchset.patches) { patch in
	177	PatchRow(
	178	patch: patch,
	179	isExpanded: expandedPatchIDs.contains(patch.id),
	180	onToggle: {
	181	withAnimation(.easeInOut(duration: 0.2)) {
	182	if expandedPatchIDs.contains(patch.id) {
	183	expandedPatchIDs.remove(patch.id)
	184	} else {
	185	expandedPatchIDs.insert(patch.id)
	186	}
	187	}
	188	}
	189	)
	190	.themedRow()
	191	}
170	192	}
171	193	}
172	194
		@@ -178,19 +200,7 @@ struct PatchsetDetailView: View {
178	200	.font(.subheadline.weight(.semibold))
179	201	.textSelection(.enabled)
180	202
181		ForEach(Array(email.contentBlocks.enumerated()), id: \.offset) { _, block in
182		switch block {
183		case .plainText(let text):
184		Text(text)
185		.font(.body)
186		.textSelection(.enabled)
187		.frame(maxWidth: .infinity, alignment: .leading)
188		.fixedSize(horizontal: false, vertical: true)
189		case .diff(let diff):
190		DiffView(diff: diff)
191		.textSelection(.enabled)
192		}
193		}
	203	PatchsetContentBlocks(blocks: email.contentBlocks)
194	204	}
195	205	.padding(.vertical, 4)
196	206	.themedRow()
		@@ -227,6 +237,70 @@ struct PatchsetDetailView: View {
227	237	}
228	238	}
229	239
	240	// MARK: - Patch Row
	241
	242	private struct PatchRow: View {
	243	let patch: PatchsetEmail
	244	let isExpanded: Bool
	245	let onToggle: () -> Void
	246
	247	var body: some View {
	248	VStack(alignment: .leading, spacing: isExpanded ? 10 : 0) {
	249	Button(action: onToggle) {
	250	HStack(alignment: .top, spacing: 12) {
	251	Image(systemName: isExpanded ? "chevron.down" : "chevron.right")
	252	.font(.caption)
	253	.foregroundStyle(.tertiary)
	254	.padding(.top, 3)
	255
	256	VStack(alignment: .leading, spacing: 2) {
	257	Text(patch.subject)
	258	.font(.subheadline.weight(.medium))
	259	.lineLimit(isExpanded ? nil : 2)
	260	.multilineTextAlignment(.leading)
	261	.frame(maxWidth: .infinity, alignment: .leading)
	262
	263	if let seriesLabel = patch.seriesLabel {
	264	Text(seriesLabel)
	265	.font(.caption)
	266	.foregroundStyle(.secondary)
	267	}
	268	}
	269	}
	270	}
	271	.buttonStyle(.plain)
	272	.accessibilityHint(isExpanded ? "Collapses this patch" : "Expands this patch")
	273
	274	if isExpanded {
	275	PatchsetContentBlocks(blocks: patch.contentBlocks)
	276	}
	277	}
	278	.padding(.vertical, 4)
	279	}
	280	}
	281
	282	// MARK: - Content Blocks
	283
	284	private struct PatchsetContentBlocks: View {
	285	let blocks: [InboxMessageContentBlock]
	286
	287	var body: some View {
	288	ForEach(Array(blocks.enumerated()), id: \.offset) { _, block in
	289	switch block {
	290	case .plainText(let text):
	291	Text(text)
	292	.font(.body)
	293	.textSelection(.enabled)
	294	.frame(maxWidth: .infinity, alignment: .leading)
	295	.fixedSize(horizontal: false, vertical: true)
	296	case .diff(let diff):
	297	DiffView(diff: diff)
	298	.textSelection(.enabled)
	299	}
	300	}
	301	}
	302	}
	303
230	304	// MARK: - Status Badge
231	305
232	306	struct PatchsetStatusBadge: View {

Commit 368c59e3f8

Commit `368c59e3f8`