apple / sample-cloudkit-sync-engine

MIT License
172 stars 13 forks source link

var listID: String #10

Open JCKL opened 8 months ago

JCKL commented 8 months ago

In ContactsList.swift extension Contact

// In order for the list to update properly when fetch changes from the cloud, we need to use something other than the contact ID for the list item ID. var listID: String { "\(self.id)\(Self.listIDSeparator)\(self.name)" }

The problem with this approach is when you start adding more fields to Contact. Your approach doesn't guarantee updates when changes are fetched.

I propose changing to the following: var listID: String { "\(self.id)\(Self.listIDSeparator)\(self.userModificationDate)" }

In my testing, this solves all update issues regardless of adding fields to Contact.

Thoughts?

malhal commented 5 months ago

Copied from my comment on your pull request:

Rather than hack the List a better solution would be to fix the mistake in ContactView where @State var contact: Contact should be @Binding var contact: Contact. It's a little bit more complicated to pass a binding to a sorted item but can easily be done the way we did it before List got binding support. I believe it's still used in Fruta (or Food Truck).

Turns out it was Food Truck:

    public func donutBinding(id: Donut.ID) -> Binding<Donut> {
        Binding<Donut> {
            self.donuts[id]
        } set: { newValue in
            self.donuts[id] = newValue
        }
    }

However in looking closer at the code it seems to me the problem they were trying to solve with the non-standard use of @State was to get an initial value to use for the text field with not affecting the actual contact, there is a better way to solve that problem.