Page MenuHomePhabricator
Differential D7504

[native] factor out shared code between Emoji[User/Thread]AvatarCreation into EmojiAvatarCreation
ClosedPublic
Actions

Authored by ginsu on Apr 18 2023, 1:24 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Feb 16, 6:47 PM
Unknown Object (File)
Sat, Feb 1, 8:59 PM
Unknown Object (File)
Thu, Jan 30, 4:10 AM
Unknown Object (File)
Sat, Jan 25, 8:29 AM
Unknown Object (File)
Sat, Jan 25, 8:29 AM
Unknown Object (File)
Sat, Jan 25, 8:29 AM
Unknown Object (File)
Sat, Jan 25, 8:29 AM
Unknown Object (File)
Sat, Jan 25, 8:29 AM
View All Files
Subscribers
ashoat
atul
tomek

Details

Reviewers
ashoat
atul
Commits
rCOMMa26ae7f8bbee: [native] factor out shared code between Emoji[User/Thread]AvatarCreation into…
Summary

The overall approach I took with refactoring this is very similar to how we built the Avatar component. We use EmojiAvatarCreation as the base component that handles all the UI/UX and shared logic, and EmojiUserAvatarCreation handles all the user avatar specific logic and EmojiThreadAvatarCreation handles all the thread avatar specific logic

Depends on D7503

Test Plan

Saving emoji avatars still work as expected, no flow errors

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu created this revision.Apr 18 2023, 1:24 PM
Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptApr 18 2023, 1:24 PM
ginsu edited the summary of this revision. (Show Details)Apr 18 2023, 1:27 PM
ginsu added reviewers: ashoat, atul.
ginsu added a parent revision: D7503: [native] naively split EmojiAvatarCreation into EmojiUserAvatarCreation and EmojiThreadAvatarCreation.
ginsu added inline comments.Apr 18 2023, 1:31 PM
native/profile/emoji-user-avatar-creation.react.js
17 ↗(On Diff #25310)

We need this to make flow/eslint happy, saw a similar pattern here

https://github.com/CommE2E/comm/blob/master/native/profile/privacy-preferences.react.js

ginsu updated this revision to Diff 25312.Apr 18 2023, 1:38 PM

rebase

Harbormaster completed remote builds in B18479: Diff 25310.Apr 18 2023, 1:42 PM
ginsu mentioned this in D7503: [native] naively split EmojiAvatarCreation into EmojiUserAvatarCreation and EmojiThreadAvatarCreation.Apr 18 2023, 1:43 PM
Harbormaster completed remote builds in B18481: Diff 25312.Apr 18 2023, 1:56 PM
ginsu requested review of this revision.Apr 18 2023, 1:56 PM
ashoat requested changes to this revision.Apr 19 2023, 5:50 AM

This looks good, but I want to make sure the types are okay before you land, so requesting changes so I can have a final pass-through before you land

native/avatars/emoji-avatar-creation.react.js
24–27 ↗(On Diff #25312)

Is this still used anywhere?

32 ↗(On Diff #25312)
  1. I think you're missing a + here
  2. * is basically any, so we should use a more appropriate type parameter here. I actually don't think we should be using BaseAppState in native at all... can we use the AppState from native/redux/state-types.js?
native/profile/profile-screen.react.js
202–204 ↗(On Diff #25312)

This can be simplified now

This revision now requires changes to proceed.Apr 19 2023, 5:50 AM
ginsu updated this revision to Diff 25377.Apr 19 2023, 10:54 AM

address comments

ashoat accepted this revision.Apr 19 2023, 11:05 AM
This revision is now accepted and ready to land.Apr 19 2023, 11:05 AM
Harbormaster completed remote builds in B18522: Diff 25377.Apr 19 2023, 11:12 AM
ginsu updated this revision to Diff 25391.Apr 19 2023, 12:37 PM

rebase before landing

This revision was landed with ongoing or failed builds.Apr 19 2023, 12:46 PM
Closed by commit rCOMMa26ae7f8bbee: [native] factor out shared code between Emoji[User/Thread]AvatarCreation into… (authored by ginsu). · Explain Why
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMMa26ae7f8bbee: [native] factor out shared code between Emoji[User/Thread]AvatarCreation into….
Harbormaster completed remote builds in B18530: Diff 25391.Apr 19 2023, 12:54 PM

Revision Contents
Changeset List

PathSize
native/
avatars/
73 lines
chat/
settings/
227 lines
navigation/
3 lines
profile/
241 lines
5 lines

Diff 25395

View Options

native/avatars/emoji-avatar-creation.react.js

Loading...
View Options

native/chat/settings/emoji-thread-avatar-creation.react.js

Loading...
View Options

native/navigation/route-names.js

Loading...
View Options

native/profile/emoji-user-avatar-creation.react.js

Loading...
View Options

native/profile/profile-screen.react.js

Loading...