Skip to content

Commit 1737a28

Browse files
authored
Merge pull request #1242 from Esri/df/FFVLeakPatch
`FeatureFormView` - Various fixes
2 parents b8523f8 + d3412a0 commit 1737a28

File tree

4 files changed

+66
-65
lines changed

4 files changed

+66
-65
lines changed

Sources/ArcGISToolkit/Common/AttachmentPreview.swift

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@ import SwiftUI
1717

1818
/// A view displaying a list of attachments in a "carousel", with a thumbnail and title.
1919
struct AttachmentPreview: View {
20-
/// An action which scrolls the Carousel to the front.
21-
@Binding var scrollToNewAttachmentAction: (() -> Void)?
22-
2320
/// The name for the existing attachment being edited.
2421
@State private var currentAttachmentName = ""
2522

@@ -47,6 +44,9 @@ struct AttachmentPreview: View {
4744
/// A Boolean value which determines if the attachment editing controls should be disabled.
4845
private let editControlsDisabled: Bool
4946

47+
/// The last locally added attachment.
48+
private let lastAttachmentAdded: AttachmentModel?
49+
5050
/// The action to perform when the attachment is deleted.
5151
private let onDelete: (@MainActor (AttachmentModel) -> Void)?
5252

@@ -59,30 +59,26 @@ struct AttachmentPreview: View {
5959
init(
6060
attachmentModels: [AttachmentModel],
6161
editControlsDisabled: Bool = true,
62+
lastAttachmentAdded: AttachmentModel? = nil,
6263
onRename: (@MainActor (AttachmentModel, String) -> Void)? = nil,
6364
onDelete: (@MainActor (AttachmentModel) -> Void)? = nil,
64-
proposedCellSize: CGSize,
65-
scrollToNewAttachmentAction: Binding<(() -> Void)?>
65+
proposedCellSize: CGSize
6666
) {
6767
self.attachmentModels = attachmentModels
6868
self.proposedCellSize = proposedCellSize
6969
self.editControlsDisabled = editControlsDisabled
70+
self.lastAttachmentAdded = lastAttachmentAdded
7071
self.onRename = onRename
7172
self.onDelete = onDelete
72-
_scrollToNewAttachmentAction = scrollToNewAttachmentAction
7373
}
7474

7575
var body: some View {
76-
Carousel { computedCellSize, scrollToLeftAction in
77-
Group {
78-
makeCarouselContent(for: computedCellSize)
79-
.transition(.asymmetric(insertion: .slide, removal: .scale))
80-
}
81-
.onAppear {
82-
scrollToNewAttachmentAction = scrollToLeftAction
83-
}
76+
Carousel { computedCellSize in
77+
makeCarouselContent(for: computedCellSize)
78+
.transition(.asymmetric(insertion: .slide, removal: .scale))
8479
}
8580
.cellBaseWidth(proposedCellSize.width)
81+
.leftScrollTrigger(lastAttachmentAdded?.id)
8682
}
8783

8884
/// - Note: Contextual actions are disabled for empty attachments as deletion and rename

Sources/ArcGISToolkit/Common/AttachmentsFeatureElementView.swift

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,8 @@ struct AttachmentsFeatureElementView: View {
3434
/// A Boolean value indicating whether the input is editable.
3535
@State private var isEditable = false
3636

37-
/// Scrolls an ``AttachmentPreview`` to the front.
38-
///
39-
/// Call this action when a new attachment is added to make it visible to the user.
40-
@State private var scrollToNewAttachmentAction: (() -> Void)?
37+
/// The last locally added attachment.
38+
@State private var lastAttachmentAdded: AttachmentModel?
4139

4240
/// A Boolean value denoting if the view should be shown as regular width.
4341
var isRegularWidth: Bool {
@@ -126,21 +124,21 @@ struct AttachmentsFeatureElementView: View {
126124
AttachmentPreview(
127125
attachmentModels: attachmentModels,
128126
editControlsDisabled: !isEditable,
127+
lastAttachmentAdded: lastAttachmentAdded,
129128
onRename: onRename,
130129
onDelete: onDelete,
131-
proposedCellSize: thumbnailSize,
132-
scrollToNewAttachmentAction: $scrollToNewAttachmentAction
130+
proposedCellSize: thumbnailSize
133131
)
134132
case .auto:
135133
Group {
136134
if isRegularWidth {
137135
AttachmentPreview(
138136
attachmentModels: attachmentModels,
139137
editControlsDisabled: !isEditable,
138+
lastAttachmentAdded: lastAttachmentAdded,
140139
onRename: onRename,
141140
onDelete: onDelete,
142-
proposedCellSize: thumbnailSize,
143-
scrollToNewAttachmentAction: $scrollToNewAttachmentAction
141+
proposedCellSize: thumbnailSize
144142
)
145143
} else {
146144
AttachmentList(attachmentModels: attachmentModels)
@@ -176,7 +174,7 @@ struct AttachmentsFeatureElementView: View {
176174
models.insert(newModel, at: 0)
177175
withAnimation { attachmentModelsState = .initialized(models) }
178176
embeddedFeatureFormViewModel?.evaluateExpressions()
179-
scrollToNewAttachmentAction?()
177+
lastAttachmentAdded = newModel
180178
}
181179

182180
/// Renames the attachment associated with the given model.

Sources/ArcGISToolkit/Components/FeatureFormView/InternalFeatureFormView.Model.swift renamed to Sources/ArcGISToolkit/Components/FeatureFormView/EmbeddedFeatureFormViewModel.swift

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,14 @@ class EmbeddedFeatureFormViewModel {
3939
var visibleElements = [FormElement]()
4040

4141
/// The expression evaluation task.
42+
@ObservationIgnored
4243
private var evaluateTask: Task<Void, Never>?
4344

4445
/// The feature form.
4546
private(set) var featureForm: FeatureForm
4647

4748
/// The group of visibility tasks.
49+
@ObservationIgnored
4850
private var isVisibleTasks = [Task<Void, Never>]()
4951

5052
/// Initializes a form view model.
@@ -64,10 +66,13 @@ class EmbeddedFeatureFormViewModel {
6466
/// Kick off tasks to monitor `isVisible` for each element.
6567
@MainActor
6668
private func initializeIsVisibleTasks() {
69+
isVisibleTasks.forEach { $0.cancel() }
70+
isVisibleTasks.removeAll()
6771
for element in featureForm.elements {
68-
let newTask = Task {
72+
let newTask = Task { [weak self] in
6973
for await _ in element.$isVisible {
70-
updateVisibleElements()
74+
guard !Task.isCancelled else { return }
75+
self?.updateVisibleElements()
7176
}
7277
}
7378
isVisibleTasks.append(newTask)

Sources/ArcGISToolkit/Utility/Carousel.swift

Lines changed: 42 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ struct Carousel<Content: View>: View {
2222
@State private var cellSize = CGSize.zero
2323

2424
/// The identifier for the Carousel content.
25-
@State private var contentIdentifier = UUID()
25+
let contentIdentifier = UUID()
2626

2727
/// The content shown in the Carousel.
28-
let content: (_: CGSize, _: (() -> Void)?) -> Content
28+
let content: (_: CGSize) -> Content
2929

3030
/// The amount to offset the scroll indicator.
3131
let scrollIndicatorOffset = 10.0
@@ -39,9 +39,12 @@ struct Carousel<Content: View>: View {
3939
/// The fractional width of the partially visible cell.
4040
var cellVisiblePortion = 0.25
4141

42+
/// The trigger used to scroll the scroll view to content's leading anchor.
43+
var leftScrollTrigger: (any Equatable & Hashable)?
44+
4245
/// A horizontally scrolling container to display a set of content.
4346
/// - Parameter content: A view builder that creates the content of this Carousel.
44-
init(@ViewBuilder content: @escaping (_: CGSize, _: (() -> Void)?) -> Content) {
47+
init(@ViewBuilder content: @escaping (_: CGSize) -> Content) {
4548
self.content = content
4649
}
4750

@@ -87,17 +90,18 @@ struct Carousel<Content: View>: View {
8790

8891
func makeCommonScrollViewContent(_ scrollViewProxy: ScrollViewProxy) -> some View {
8992
HStack(spacing: cellSpacing) {
90-
content(cellSize) {
91-
withAnimation {
92-
scrollViewProxy.scrollTo(contentIdentifier, anchor: .leading)
93-
}
94-
}
95-
.id(contentIdentifier)
96-
.frame(width: cellSize.width, height: cellSize.height)
97-
.clipped()
9893
// Pad the content such that the scroll indicator appears beneath it
99-
// so that the content is not covered.
100-
.padding(.bottom, scrollIndicatorOffset)
94+
// so that the content is not covered.
95+
content(cellSize)
96+
.id(contentIdentifier)
97+
.frame(width: cellSize.width, height: cellSize.height)
98+
.clipped()
99+
.padding(.bottom, scrollIndicatorOffset)
100+
.onChange(of: leftScrollTrigger?.hashValue) {
101+
withAnimation {
102+
scrollViewProxy.scrollTo(contentIdentifier, anchor: .leading)
103+
}
104+
}
101105
}
102106
}
103107
}
@@ -131,24 +135,30 @@ extension Carousel {
131135
copy.cellVisiblePortion = visiblePortion
132136
return copy
133137
}
138+
139+
func leftScrollTrigger(_ trigger: (any Equatable & Hashable)?) -> Self {
140+
var copy = self
141+
copy.leftScrollTrigger = trigger
142+
return copy
143+
}
134144
}
135145

136146
#Preview("cellBaseWidth(_:)") {
137-
Carousel { _, _ in
147+
Carousel { _ in
138148
PreviewContent()
139149
}
140150
.cellBaseWidth(75)
141151
}
142152

143153
#Preview("cellSpacing(_:)") {
144-
Carousel { _, _ in
154+
Carousel { _ in
145155
PreviewContent()
146156
}
147157
.cellSpacing(2)
148158
}
149159

150160
#Preview("cellVisiblePortion(_:)") {
151-
Carousel { _, _ in
161+
Carousel { _ in
152162
PreviewContent()
153163
}
154164
.cellVisiblePortion(0.5)
@@ -157,38 +167,30 @@ extension Carousel {
157167
#Preview("In a List") {
158168
List {
159169
Text(verbatim: "Hello")
160-
Carousel { _, _ in
170+
Carousel { _ in
161171
PreviewContent()
162172
}
163173
Text(verbatim: "World!")
164174
}
165175
}
166176

167-
#Preview("Scroll to left action") {
168-
struct ScrollDemo: View {
169-
@State var scrollToLeftAction: (() -> Void)?
170-
171-
var body: some View {
172-
Carousel { _, scrollToLeftAction in
173-
ForEach(1..<11) {
174-
Text($0.description)
175-
.id($0)
176-
}
177-
.onAppear {
178-
self.scrollToLeftAction = scrollToLeftAction
179-
}
180-
}
181-
Button {
182-
withAnimation {
183-
scrollToLeftAction?()
184-
}
185-
} label: {
186-
Text(verbatim: "Scroll to left")
187-
}
177+
#Preview("Programmatically scroll to left") {
178+
@Previewable @State var items = [1]
179+
180+
Carousel { _ in
181+
ForEach(items, id: \.self) {
182+
Text($0.description)
183+
.id($0)
188184
}
189185
}
190-
191-
return ScrollDemo()
186+
.leftScrollTrigger(items)
187+
Button {
188+
items.insert(items.count + 1, at: 0)
189+
} label: {
190+
Text(verbatim: "Insert Item")
191+
}
192+
Text(verbatim: "The Carousel will scroll to the left to make the item visible, if needed.")
193+
.foregroundStyle(.secondary)
192194
}
193195

194196
private struct PreviewContent: View {

0 commit comments

Comments
 (0)