pocket-casts-ios
pocket-casts-ios copied to clipboard
Automattic
Add "Change Avatar" button to account settings
Fixes # gravatarp2 -> 2024/06/10/pt-integrating-gravatar-into-pocket-casts
Adding the ability to change the avatar. Avatar is coming from user's Gravatar account. So the "Change Avatar" button opens the Gravatar web link via in-app Safari.
To test
Profile > Account > "Change Avatar" Observe: In app Safari is presented
- Inside the browser, if you are not logged in you should see the login first, then you'll be redirected to avatar update page.
- The avatar change does not always get reflected to the app immediately even though we force refresh the avatar on the mobile side skipping all client-side caches. This is a known limitation. There's a backend cache too and it's not updated immediately. The plan is to show such a message inside the browser:
If you are not logged in to the app then "Change Avatar" button will not be present for you.
Known issue:
A strange auto-zoom happens when testing in a Simulator but this issue is not reproduced in real device. But let me know if you can reproduce this in a real device.
Checklist
- [x] I have considered if this change warrants user-facing release notes and have added them to
CHANGELOG.mdif necessary. - [x] I have considered adding unit tests for my changes.
- [x] I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.
Wouldn't it make more sence to just tap the icon?
I think this is an experimental feature that's why, it's not a fully native experience to begin with. We just open a browser to change the avatar. But it's a fair question. cc @Luchadores and @david-gonzalez-a8c if you want to add anything.
I think we can try it, not sure if there's any other trigger that Pocket Casts already has on the Avatar itself. If not, we can do it, and see if users tap more directly the Avatar itself.
We did some discussions and for now we'll move fwd with the "Change Avatar" button.
I'll fwd these feedbacks to @wellyshen @aaronfc
- The email field has autocomplete and autocorrect on. I think we might want to disable those as it kinda keeps suggesting changes to email:
I agree, it would be better if those are disabled.
- I felt an "avatar changed" confirmation is missing (either on the webpage or in the app). After cropping the image, there's no success message. I wonder if for any new user, this will confuse them as to whether they changed the avatar or not.
I think the confirmation is there but its position isn't right so it's not easily viewable to user. Seems like a bug.
issues created: https://github.a8c.com/Automattic/gravatar/issues/108240 https://github.a8c.com/Automattic/gravatar/issues/108241
- Do we need to enter the email? We already have this information, in theory, we could be directly redirected to the code phase, without requiring the user to type any info. This will likely reduce friction.
Unfortunately, we need the user to enter their email for now. In Gravatar, users can define multiple avatars for multiple emails under one account. But they can log in to Gravatar only with their primary email. If the user is using one of those secondary emails in Pocket Casts, then pre-filling the field would be misleading because the login attempt would fail. But there's an ongoing project to update the login flow to allow logins by secondary emails. After that the pre-filling can also be an option I think.
Not a big deal, but the icon feels a bit different from the rest to me. The border seems slightly thinner and the inside normally has a more subtle blue.
cc: @Luchadores this is a feedback about the new change avatar icon.
Unfortunately, we need the user to enter their email for now. In Gravatar, users can define multiple avatars for multiple emails under one account. But they can log in to Gravatar only with their primary email. If the user is using one of those secondary emails in Pocket Casts, then pre-filling the field would be misleading because the login attempt would fail. But there's an ongoing project to update the login flow to allow logins by secondary emails. After that the pre-filling can also be an option I think.
Hey, sorry to jump in. But this is not accurate. You can (and you should) pass the ?email= query parameter to the URL. The login process already handles secondary emails (just released) (p8Slzc-2NM-p2), and as soon as Quick Editor project (p8Slzc-3i1-p2) is done it will also redirect to the correct avatars page.
The correct URL, I believe, should be https://gravatar.com/profile?is_quick_editor=true&email=xxxx&scope=avatars (once the Quick Editor project is done). cc/ @wellyshen @jcheringer
Hey, sorry to jump in. But this is not accurate. You can (and you should) pass the ?email= query parameter to the URL. The login process already handles secondary emails (just released) (p8Slzc-2NM-p2), and as soon as Quick Editor project (p8Slzc-3i1-p2) is done it will also redirect to the correct avatars page.
The correct URL, I believe, should be https://gravatar.com/profile?is_quick_editor=true&email=xxxx&scope=avatars (once the Quick Editor project is done)
Okay just to avoid causing our Pocket Casts friends misunderstand the situation. Logging in with a secondary email flow was deployed just yesterday.
And the URL that enables us to pass the email is not completely ready yet, although it seems close to being ready. So I'll wait for it instead of opening a follow up PR which was the original plan. I'll communicate internally about this new URL and update the PR.
I tried triggering a prototype build but it seems like there's an unrelated error there. It shouldn't be a blocker for the merge I think.
@Luchadores I suggest we use this icon in our library, it might suit this new row already.
Thank you ✨
@pinarol, @etoledom - Could we update the icon with the one David provided the link to?
Sure thing! @david-gonzalez-a8c Which one do you mean exactly? I am afraid I can't see it through that link. Can you maybe share its screenshot?
@pinarol -> I believe he is talking about this one:
@david-gonzalez-a8c let us know if you refer to a different one ☝️
I wonder if it would be possible to feature flag these changes?
yeah I think we can do it.
I am applying the new icon. Does it look ok @david-gonzalez-a8c ?
sharing the developer view also, the size differences are related with inner insets of the icons:
Hey Pinar, before David replies, that looks nice, but we don't have the different hues in the icon like the rest. Sync with @etoledom, I believe he solved that last time.
Attached is a screenshot of the previous icon we used, but you can see the difference as it plays on the blue opacity color.
Hey Pinar, before David replies, that looks nice, but we don't have the different hues in the icon like the rest. Sync with @etoledom, I believe he solved that last time.
That's because the icon from the library doesn't have different opacities as other icons have (for example). I've just updated the icon in the Android PR, but let me know if you should use another one.
@hamorillo is right. For that effect to happen the icon should have different opacities in it. @david-gonzalez-a8c could you check the icon and share an updated version if you think it doesn't look right https://github.com/Automattic/pocket-casts-ios/pull/1770#issuecomment-2239034912 ?
Apart from the icon, this is ready for another look. @leandroalonso @danielebogo
👋 I recreated that specific icon with transparency. Attached is an SVG, and PNGs (1x, 2x, 3x). Hope it helps 🤞 @pinarol, @hamorillo. @david-gonzalez-a8c, hope it's the right icon :D
I recreated that specific icon with transparency.
Okay I've used them. This is how it looks now:
Also the avatar changed pretty fast after the vc has been dismissed
Yes that's the case now since we introduced a kind of cache buster parameter with this commit. https://github.com/Automattic/pocket-casts-ios/pull/1770/commits/04d8c08ddab09fa184051a9fc4020267d076f049.
I think we are ready to merge. 🎉 Thank you all for your help and feedback. 🙇