Page MenuHomePhabricator

Move registrationMode-related logic from `updateImageUserAvatar` to `nativeUpdateUserImageAvatar`
ClosedPublic

Authored by atul on Jun 27 2023, 5:59 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 18, 5:01 PM
Unknown Object (File)
Sat, Nov 30, 11:37 PM
Unknown Object (File)
Sat, Nov 23, 2:24 PM
Unknown Object (File)
Sat, Nov 23, 2:24 PM
Unknown Object (File)
Sat, Nov 23, 1:19 PM
Unknown Object (File)
Nov 14 2024, 6:22 AM
Unknown Object (File)
Nov 12 2024, 10:33 AM
Unknown Object (File)
Nov 1 2024, 5:36 PM
Subscribers

Details

Summary

Effectively D8343, but for updateImageUserAvatar instead of setUserAvatar.

The registrationMode-related code is native-specific and was pretty interwoven with the logic in setUserAvatar/updateImageUserAvatar.

This diff decouples native-specific from updateImageUserAvatar and moves it to nativeUpdateImageUserAvatar.

As of this diff, updateImageUserAvatar is NOT yet platform-agnostic. We still need to pull out uploadSelectedMedia and want the function to take ImageAvatarDBContent instead of the native-specific NativeMediaSelection. The next diff will address this.


Depends on D8344

Test Plan

Continue to be able to set image avatars on native as expected.

Diff Detail

Repository
rCOMM Comm
Branch
arcpatch-D8345 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul published this revision for review.Jun 27 2023, 6:00 PM
This revision is now accepted and ready to land.Jun 28 2023, 10:10 AM
ashoat requested changes to this revision.Jun 28 2023, 6:20 PM

Same feedback as in D8343

This revision now requires changes to proceed.Jun 28 2023, 6:20 PM
atul requested review of this revision.EditedJun 28 2023, 6:41 PM

updateImageUserAvatar previously took (selection: NativeMediaSelection) as an argument to satisfy the mediaSelection: NativeMediaSelection field of the object passed to registrationModeRef.current.successCallback when needsUpload is true.

As of D8346 the argument is changed to (imageAvatarUpdateRequest: ImageAvatarDBContent) so updateUserImageAvatar can be used by both web and native. It becomes the responsibility of platform-specific avatar-hooks to provide platform-agnostic ImageAvatarDBContent to *EditUserAvatarProvider.

Because I was moving the registrationMode stuff out of updateImageUserAvatar, I decided to do the same for setUserAvatar for consistency/symmetry. I thought it'd be best if the registrationMode stuff for image and non-image avatars were handled in the same file in nativeUpdateUserImageAvatar(...) and nativeSetUserAvatar(...) instead of in nativeUpdateUserImageAvatar(...) in avatar-hooks and setUserAvatar(...) in *EditUserAvatarProvider.

registrationMode is only relevant on native because there's no registration flow on web (and based on my understanding there won't be in the foreseeable future). We are leaving setRegistrationMode, getRegistrationModeEnabled, etc here because it doesn't affect anything on web and I wanted to limit the amount of refactor work.

This revision is now accepted and ready to land.Jun 28 2023, 6:46 PM
This revision was landed with ongoing or failed builds.Jul 28 2023, 1:55 PM
This revision was automatically updated to reflect the committed changes.