added Delete Account UI #80 #1518
Conversation
bryanmontz
requested review from
mplorentz,
martindsq and
joshuatbrown
as code owners
| message: { TextState(String(localized: .localizable.deleteAccountDescription)) } | ||
| ) | ||
| } | ||
| .clipShape(Capsule()) |
There was a problem hiding this comment.
I think that this could be added into ActionButton directly instead of having its more manual corner rounding method, but I wasn't sure.
There was a problem hiding this comment.
Is the more manual method you are referring to the .cornerRadius() modifier? I think just a plain Capsule() is something we'd need to run past Sebastian, because I'm not sure how pick he is about using the exact radius he has in Figma on the corners. It seems like Capsule would scale more elegantly to larger text sizes so I'm in favor of that.
There was a problem hiding this comment.
Yes, that's what I meant. I can ask Sebastian about it.
There was a problem hiding this comment.
Hey Bryan, this looks good and publishes the NIP-62 event as described in the ticket. However as I'm playing with it I think a few more changes are necessary.
- Until we have implemented this on the server side it doesn't make sense to deploy it to users. Can you lock the button behind a feature flag for now?
- If you log back in to the account you just deleted it still shows all your data. Obviously when some/most relays implement this NIP that will help, but I think we should do a couple more mitigations:
- Let's drop all tables on sqlite on logout. That's something we should have been doing anyway. Ideally we could use
PersistenceController.setUp(erasing: true)but I'm not sure how our Dependencies library will handle a new instance of PersistenceController. We might need to NSBatchDelete all the models individually. - I think we should overwrite the user's kind 0 and kind 3. Maybe we should check with Linda on this one, but I think publishing an empty profile that says "Account deleted" in the name fields or something would go a long way towards signaling the user's intentions as much as possible, especially for relays and clients that don't support NIP-62. We could also publish an empty kind 3 (just make sure that it gets published to the user's relays. I could see it publishing to an empty list of relays because we are deleting their relay list with this action).
- In design review we talked about adding more friction before the account deletion takes place to prevent accidental taps. I left a comment on the ticket to follow up on that.
What are your thoughts? I'm fine with all this being in separate PRs, except for the feature flag.
| message: { TextState(String(localized: .localizable.deleteAccountDescription)) } | ||
| ) | ||
| } | ||
| .clipShape(Capsule()) |
There was a problem hiding this comment.
Is the more manual method you are referring to the .cornerRadius() modifier? I think just a plain Capsule() is something we'd need to run past Sebastian, because I'm not sure how pick he is about using the exact radius he has in Figma on the corners. It seems like Capsule would scale more elegantly to larger text sizes so I'm in favor of that.
|
@mplorentz Yeah, I think that makes sense. I added a feature flag to this so we can get it merged, then I can tackle the other items you mentioned in one or more other PRs. Thanks for being thoughtful about user data like this! |
|
@mplorentz Ticket for deleting all content on sign out: #1528 |
Issues covered
#80
Description
Adds the UI associated with deleting a user's account.
How to test
While signed in to an account that you don't mind being deleted...
At this point, a NIP-62 Request to Vanish event will be published to all the user's relays and the account will be signed out locally.
Screenshots/Video