Details

Reviewers

ashoat
atul

Commits

rCOMMc8ae0af5dff1: [native] improve factoring out redundant logic for saving an image user avatar

Summary

This diff has two small improvements for factoring out the redundant logic for saving an image avatar. The first improvement makes the callback in useUploadSelectedMedia return a ImageAvatarDBContent instead of a string. The second improvement factors out the update image avatar logic into its own function

Depends on D7622

Test Plan

flow and editing image user avatar still has expected behavior

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Apr 26 2023, 3:25 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptApr 26 2023, 3:25 AM

ginsu edited the summary of this revision. (Show Details)Apr 26 2023, 3:29 AM

ginsu added reviewers: ashoat, atul.

ginsu edited the summary of this revision. (Show Details)

ginsu added a parent revision: D7622: [native] move navigate to emoji user avatar creation function to be within edit user avatar.

ginsu edited the summary of this revision. (Show Details)Apr 26 2023, 3:41 AM

Harbormaster completed remote builds in B18791: Diff 25747.Apr 26 2023, 3:43 AM

ginsu requested review of this revision.Apr 26 2023, 3:43 AM

update

fix feature flag

ginsu edited the summary of this revision. (Show Details)Apr 26 2023, 3:53 AM

Harbormaster completed remote builds in B18794: Diff 25750.Apr 26 2023, 4:09 AM

Harbormaster completed remote builds in B18795: Diff 25751.

ginsu added a child revision: D7624: [native] introduce user avatar camera modal.Apr 26 2023, 4:16 AM

rebase

Harbormaster completed remote builds in B18830: Diff 25798.Apr 26 2023, 9:49 PM

ginsu retitled this revision from [native] improve factoring out redundant logic for saving an image avatar to [native] improve factoring out redundant logic for saving an image user avatar.Apr 26 2023, 10:16 PM

It would be good to explain the reason for the changes here. Are these changes being made because you're going to reuse the factored-out code in a later diff in the stack, so it makes sense to factor out the shared logic here?

native/avatars/edit-user-avatar-provider.react.js
59	Why do we name this `uploadUserAvatarSelectedMedia` here instead of `uploadSelectedMedia`? We use the latter name in `useSelectFromGalleryAndUpdateThreadAvatar`, and it correspond better to the name of the hook.

This revision is now accepted and ready to land.Apr 27 2023, 4:30 AM

ashoat mentioned this in D7645: [native] improve factoring out redundant logic for saving an image thread avatar.Apr 27 2023, 10:27 AM

address comments/rebase before landing

It would be good to explain the reason for the changes here. Are these changes being made because you're going to reuse the factored-out code in a later diff in the stack, so it makes sense to factor out the shared logic here?

Yes sorry I should have done this when I initially put the diff up... this logic is directly shared and used by both the edit image avatar through gallery and camera. So I factored this logic out here in anticipation of the introduction of the edit user avatar camera modal which is introduced in the next diff

Harbormaster completed remote builds in B18897: Diff 25886.Apr 27 2023, 12:59 PM

ginsu removed a child revision: D7624: [native] introduce user avatar camera modal.Apr 27 2023, 1:25 PM

Closed by commit rCOMMc8ae0af5dff1: [native] improve factoring out redundant logic for saving an image user avatar (authored by ginsu). · Explain WhyApr 27 2023, 1:37 PM

This revision was automatically updated to reflect the committed changes.

ginsu added a commit: rCOMMc8ae0af5dff1: [native] improve factoring out redundant logic for saving an image user avatar.