Avatar upload #21
Avatar upload #21
Conversation
|
| App Name | Gravatar Prototype Build | |
| Build Number | 136 | |
| Version | PR #21 | |
| Bundle ID | com.gravatar.GravatarApp.prototype | |
| Commit | 1197107 | |
| Installation URL | 5fdp2mpbo8pko |
| guard let avatar = try await avatarService.upload( | ||
| squareImage, | ||
| selectionBehavior: .selectUploadedImageIfNoneSelected(for: .hashID(profile.hash)), | ||
| accessToken: accessToken | ||
| ) as? AvatarDetails else { | ||
| fatalError() | ||
| } |
There was a problem hiding this comment.
We need to return AvatarDetails from avatarService.upload()
This is a breaking change on the SKD
There was a problem hiding this comment.
IMO, we can start doing these sorts of changes on SDK's trunk branch. We had a list of things for a major version anyway and we have quite some deprecations piled up.
In the meantime if we need to fix anything we can do so in the release branch.
There was a problem hiding this comment.
We can depend point SDK dependency to a alpha/beta version or even to a commit, all better than having a fatalError() call in production.
There was a problem hiding this comment.
all better than having a fatalError() call in production.
Yes I didn't mean to ship this fatal error to production, but I used it while we decide how to approach the SDK change.
As mentioned in the description:
On this PR (so far) we are force-casting it, which is guaranteed to succeed, but ideally we need to get a AvatarDetails type from the upload.
|
Automattic/Gravatar-SDK-iOS#792 solves both problems encountered, and I've updated this PR to implement those SDK API changes |
| Text("Get a new look") | ||
| .font(.headline) | ||
| Text("It’s been 87 days since you updated your avatar.") |
There was a problem hiding this comment.
We don't have localization yet but I think it would still be good to define these in a way that is easy for Glotpress to pick up in the futue, which is by declaring them as NSLocalizedString. WDYT?
There was a problem hiding this comment.
It’s been 87 days since you updated your avatar.
I know this is in the designs but we shouldn't put invalid static text I think. We don't have that info and it is unlikely we'll get it. Let's either replace it with a valid string or just remove this Text altogether until we know what to put here.
pinarol
left a comment
pinarol
left a comment
There was a problem hiding this comment.
Just small comments otherwise looks good 🎉 I don't need to check again I think.
Closes GRA-170
This PR adds the buttons to upload new avatars from 3 different sources:
Most of the types required come directly from the SDK, removing everything related to external custom image editor.
Problems encountered
1. Profile email needed for image upload (on the SDK side).
We need two breaking changes on the SDK. One of them was made here:
Automattic/Gravatar-SDK-iOS#792
The avatar upload request (as it was implemented) required the profile's Email, but what the backend is interested in is only the email's hash. So we should use
PofileIdentifierinstead.As we don't have access to the email on the OAuth app, we must use the hash directly, so this change is required.
2. Erasing
AvatarDetailstype from the avatar upload return.We tried too hard to hide Avatar information.
AvatarDetailsis public, but we erase it on the return of the avatar upload, and we returnAvatarTypeinstead which doesn't provide enough information for the model.On this PR (so far) we are force-casting it, which is guaranteed to succeed, but ideally we need to get aAvatarDetailstype from the upload.Note: Automattic/Gravatar-SDK-iOS#792 has been updated to solve both problems
ScreenRecording_07-01-2025.22-39-02_1.mov
Test