-
-
Notifications
You must be signed in to change notification settings - Fork 61
Fix ForEach Collection change rendered View correctness #245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e9b94a8
116b72c
466c259
76968ca
2dba16b
25c6e66
d582598
743a488
30332e6
1b22d8b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| import DefaultBackend | ||
| import Foundation | ||
| import SwiftCrossUI | ||
|
|
||
| #if canImport(SwiftBundlerRuntime) | ||
| import SwiftBundlerRuntime | ||
| #endif | ||
|
|
||
| @main | ||
| @HotReloadable | ||
| struct ForEachApp: App { | ||
| @State var items = { | ||
| var items = [Item]() | ||
| for i in 0..<20 { | ||
| items.append(.init("\(i)")) | ||
| } | ||
| return items | ||
| }() | ||
| @State var biggestValue = 19 | ||
| @State var insertionPosition = 10 | ||
|
|
||
| var body: some Scene { | ||
| WindowGroup("ForEach") { | ||
| #hotReloadable { | ||
| ScrollView { | ||
| VStack { | ||
| Button("Append") { | ||
| biggestValue += 1 | ||
| items.append(.init("\(biggestValue)")) | ||
| } | ||
|
|
||
| #if !os(tvOS) | ||
| Button( | ||
| "Insert in front of current item at position \(insertionPosition)" | ||
| ) { | ||
| biggestValue += 1 | ||
| items.insert(.init("\(biggestValue)"), at: insertionPosition) | ||
| } | ||
|
|
||
| Slider($insertionPosition, minimum: 0, maximum: items.count - 1) | ||
| .onChange(of: items.count) { | ||
| guard insertionPosition > items.count - 1 else { | ||
| return | ||
| } | ||
| insertionPosition = max(items.count - 1, 0) | ||
|
Comment on lines
+42
to
+45
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It feels odd having a let upperLimit = max(items.count - 1, 0)
insertionPosition = min(insertionPosition, upperLimit)If you want you can add an if statement to only set the insertionPosition if it has changed, but I don't believe that that will be necessary for performance at all once we do dependency analysis stuff. |
||
| } | ||
| #endif | ||
|
|
||
| ForEach(items) { item in | ||
| ItemRow( | ||
| item: item, isFirst: Optional(item.id) == items.first?.id, | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move |
||
| isLast: Optional(item.id) == items.last?.id | ||
| ) { | ||
| items.removeAll(where: { $0.id == item.id }) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment I left on |
||
| } moveUp: { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it'd be best to have the |
||
| guard | ||
| let ownIndex = items.firstIndex(where: { $0.id == item.id }), | ||
| ownIndex != items.startIndex | ||
| else { return } | ||
| items.swapAt(ownIndex, ownIndex - 1) | ||
| } moveDown: { | ||
| guard | ||
| let ownIndex = items.firstIndex(where: { $0.id == item.id }), | ||
| ownIndex != items.endIndex | ||
| else { return } | ||
| items.swapAt(ownIndex, ownIndex + 1) | ||
| } | ||
| } | ||
| } | ||
| .padding(10) | ||
| } | ||
| } | ||
| } | ||
| .defaultSize(width: 400, height: 800) | ||
| } | ||
| } | ||
|
|
||
| struct ItemRow: View { | ||
| @State var item: Item | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Item shouldn't be |
||
| let isFirst: Bool | ||
| let isLast: Bool | ||
| var remove: () -> Void | ||
| var moveUp: () -> Void | ||
| var moveDown: () -> Void | ||
|
|
||
| var body: some View { | ||
| HStack { | ||
| Text(item.value) | ||
| Button("Delete") { remove() } | ||
| Button("⌃") { moveUp() } | ||
| .disabled(isFirst) | ||
| Button("⌄") { moveDown() } | ||
| .disabled(isLast) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| class Item: Identifiable, SwiftCrossUI.ObservableObject { | ||
| let id = UUID() | ||
| @SwiftCrossUI.Published var value: String | ||
|
|
||
| init(_ value: String) { | ||
| self.value = value | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,7 +38,7 @@ | |
| .padding(.top, 20) | ||
|
|
||
| ScrollView { | ||
| ForEach(greetings.reversed()[1...]) { greeting in | ||
| ForEach(items: greetings.reversed()[1...]) { greeting in | ||
|
Check warning on line 41 in Examples/Sources/GreetingGeneratorExample/GreetingGeneratorApp.swift
|
||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix this up to use the new non-deprecated syntax. |
||
| Text(greeting) | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,10 +29,12 @@ struct WebViewApp: App { | |
| } | ||
| .padding() | ||
|
|
||
| WebView($url) | ||
| .onChange(of: url) { | ||
| urlInput = url.absoluteString | ||
| } | ||
| #if !os(tvOS) | ||
| WebView($url) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add an |
||
| .onChange(of: url) { | ||
| urlInput = url.absoluteString | ||
| } | ||
| #endif | ||
| } | ||
| } | ||
| } | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,6 +110,10 @@ let package = Package( | |
| url: "https://github.com/stackotter/swift-winui", | ||
| revision: "1695ee3ea2b7a249f6504c7f1759e7ec7a38eb86" | ||
| ), | ||
| .package( | ||
| url: "https://github.com/apple/swift-collections.git", | ||
| exact: "1.2.1" | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason that this has to be |
||
| ), | ||
| // .package( | ||
| // url: "https://github.com/stackotter/TermKit", | ||
| // revision: "163afa64f1257a0c026cc83ed8bc47a5f8fc9704" | ||
|
|
@@ -129,6 +133,7 @@ let package = Package( | |
| dependencies: [ | ||
| "HotReloadingMacrosPlugin", | ||
| .product(name: "ImageFormats", package: "swift-image-formats"), | ||
| .product(name: "OrderedCollections", package: "swift-collections") | ||
| ], | ||
| exclude: [ | ||
| "Builders/ViewBuilder.swift.gyb", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace with