Sjors / nthkey-ios

Your iOs device in a Bitcoin multi-sig
https://nthkey.com
MIT License
17 stars 7 forks source link

Feature mark as used #57

Closed w-i-n-s closed 3 years ago

w-i-n-s commented 3 years ago

Resolve #29

Sjors commented 3 years ago

(needs rebase)

Can you change the text here to "Mark used"? Ideally also change the color to something non-dangerous.

Schermafbeelding 2021-06-14 om 13 13 47

The strike-through looks fine, thanks:

Schermafbeelding 2021-06-14 om 13 15 19

When the user does another swipe on it, maybe call it "Mark unused" (or "Undo") and undo the action.

As for updating the UI, can't you redraw the cell after markAsUsed(indexSet: IndexSet) in AddressesViewModel.swift?

w-i-n-s commented 3 years ago

(needs rebase)

sure

Can you change the text here to "Mark used"? Ideally also change the color to something non-dangerous.

For the moment it doesn't have a straightway to solve it. Of course, I can bring UIKIt's controller here to make text and color as we need. But I recommend not to invest time here. UI side can wait before iOS15, I hope. (details)[https://github.com/Sjors/nthkey-ios/issues/29#issuecomment-860251724]

When the user does another swipe on it, maybe call it "Mark unused" (or "Undo") and undo the action.

Yes, we can toggle it.

As for updating the UI, can't you redraw the cell after markAsUsed(indexSet: IndexSet) in AddressesViewModel.swift?

No, it doesn't work like this. The SwiftUI 'List' observes the array and if the array will change - then the list will redraw. View of every row also observe changes in the AddressEntity item. But the item is the object - it means objectWillChange will not send when we mark the address as used. I'll try to map the CoreData object into a structure (which looks weird I think) - because a change in structure should send the objectWillChange signal.

Sjors commented 3 years ago

Ok, waiting for iOs 15 is fine for improving the text and color. And otherwise we could add a checkbox instead, rather than the swipe gesture.

w-i-n-s commented 3 years ago

add a checkbox instead, rather than the swipe gesture.

fixed in 647d0d8