diff options
| author | Christian Cleberg <[email protected]> | 2026-07-15 22:02:55 -0500 |
|---|---|---|
| committer | Christian Cleberg <[email protected]> | 2026-07-15 22:02:55 -0500 |
| commit | 2913d950a5c82d80226050e05bd64be7e30ff87d (patch) | |
| tree | a5e178a4cb241c8ad362f864fd6ce082e0bbeefa /Hutch/Views/Patchsets/PatchsetDetailView.swift | |
| parent | 575e62f6dab44b0c9836623fe8b7d17a219f8e0f (diff) | |
| download | hutch-2913d950a5c82d80226050e05bd64be7e30ff87d.tar.gz hutch-2913d950a5c82d80226050e05bd64be7e30ff87d.tar.bz2 hutch-2913d950a5c82d80226050e05bd64be7e30ff87d.zip | |
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.
Diffstat (limited to 'Hutch/Views/Patchsets/PatchsetDetailView.swift')
| -rw-r--r-- | Hutch/Views/Patchsets/PatchsetDetailView.swift | 104 |
1 files changed, 89 insertions, 15 deletions
diff --git a/Hutch/Views/Patchsets/PatchsetDetailView.swift b/Hutch/Views/Patchsets/PatchsetDetailView.swift index 7bc5630..f904424 100644 --- a/Hutch/Views/Patchsets/PatchsetDetailView.swift +++ b/Hutch/Views/Patchsets/PatchsetDetailView.swift @@ -7,6 +7,7 @@ struct PatchsetDetailView: View { @Environment(AppState.self) private var appState @State private var viewModel: PatchsetDetailViewModel? @State private var showStatusPicker = false + @State private var expandedPatchIDs: Set<Int> = [] var body: some View { Group { @@ -163,10 +164,31 @@ struct PatchsetDetailView: View { } } + /// Patches start collapsed. + /// + /// A diff is tall, and a series is many of them. Rendering every patch expanded + /// puts a dozen self-sizing diffs in one List, which drives UICollectionView + /// into a recursive layout loop and wedges the app. The inbox thread view + /// collapses all but the last message for the same reason. @ViewBuilder private func patchesSection(_ patchset: PatchsetDetail) -> some View { - ForEach(patchset.patches) { patch in - emailSection(patch, title: patch.seriesLabel.map { "Patch \($0)" } ?? "Patch") + Section("Patches") { + ForEach(patchset.patches) { patch in + PatchRow( + patch: patch, + isExpanded: expandedPatchIDs.contains(patch.id), + onToggle: { + withAnimation(.easeInOut(duration: 0.2)) { + if expandedPatchIDs.contains(patch.id) { + expandedPatchIDs.remove(patch.id) + } else { + expandedPatchIDs.insert(patch.id) + } + } + } + ) + .themedRow() + } } } @@ -178,19 +200,7 @@ struct PatchsetDetailView: View { .font(.subheadline.weight(.semibold)) .textSelection(.enabled) - ForEach(Array(email.contentBlocks.enumerated()), id: \.offset) { _, block in - switch block { - case .plainText(let text): - Text(text) - .font(.body) - .textSelection(.enabled) - .frame(maxWidth: .infinity, alignment: .leading) - .fixedSize(horizontal: false, vertical: true) - case .diff(let diff): - DiffView(diff: diff) - .textSelection(.enabled) - } - } + PatchsetContentBlocks(blocks: email.contentBlocks) } .padding(.vertical, 4) .themedRow() @@ -227,6 +237,70 @@ struct PatchsetDetailView: View { } } +// MARK: - Patch Row + +private struct PatchRow: View { + let patch: PatchsetEmail + let isExpanded: Bool + let onToggle: () -> Void + + var body: some View { + VStack(alignment: .leading, spacing: isExpanded ? 10 : 0) { + Button(action: onToggle) { + HStack(alignment: .top, spacing: 12) { + Image(systemName: isExpanded ? "chevron.down" : "chevron.right") + .font(.caption) + .foregroundStyle(.tertiary) + .padding(.top, 3) + + VStack(alignment: .leading, spacing: 2) { + Text(patch.subject) + .font(.subheadline.weight(.medium)) + .lineLimit(isExpanded ? nil : 2) + .multilineTextAlignment(.leading) + .frame(maxWidth: .infinity, alignment: .leading) + + if let seriesLabel = patch.seriesLabel { + Text(seriesLabel) + .font(.caption) + .foregroundStyle(.secondary) + } + } + } + } + .buttonStyle(.plain) + .accessibilityHint(isExpanded ? "Collapses this patch" : "Expands this patch") + + if isExpanded { + PatchsetContentBlocks(blocks: patch.contentBlocks) + } + } + .padding(.vertical, 4) + } +} + +// MARK: - Content Blocks + +private struct PatchsetContentBlocks: View { + let blocks: [InboxMessageContentBlock] + + var body: some View { + ForEach(Array(blocks.enumerated()), id: \.offset) { _, block in + switch block { + case .plainText(let text): + Text(text) + .font(.body) + .textSelection(.enabled) + .frame(maxWidth: .infinity, alignment: .leading) + .fixedSize(horizontal: false, vertical: true) + case .diff(let diff): + DiffView(diff: diff) + .textSelection(.enabled) + } + } + } +} + // MARK: - Status Badge struct PatchsetStatusBadge: View { |
