Skip to content
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

var listID: String #10

Open
JCKL opened this issue Jan 12, 2024 · 1 comment
Open

var listID: String #10

JCKL opened this issue Jan 12, 2024 · 1 comment

Comments

@JCKL
Copy link

JCKL commented Jan 12, 2024

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
Copy link

malhal commented Apr 8, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants