Details

Reviewers

ashoat
atul

Commits

rCOMMcaf80b74e363: [native] use savedEmojiAvatarSelectorForCurrentUser in EmojiAvatarCreation

Summary

Based on feedback I got in D7359, when we "reset" the emoji avatar during the emoji avatar creation flow , we want to reset to the current state in redux which we are considering the source of truth. This is the diff that implements the selector I introduced in the previous diff

Depends on D7402

Test Plan

The reset functionality behaves as expected:

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ginsu created this revision.Apr 12 2023, 7:17 PM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptApr 12 2023, 7:17 PM

ginsu edited the summary of this revision. (Show Details)Apr 12 2023, 7:24 PM

ginsu edited the test plan for this revision. (Show Details)

ginsu added reviewers: ashoat, atul.

ginsu added a parent revision: D7402: [lib] introduce savedEmojiAvatarSelectorForCurrentUser selector.

ginsu attached a referenced file: F481856: Screen Recording 2023-04-12 at 3.00.00 PM.mov. (Show Details)

ginsu attached a referenced file: F481857: Screen Recording 2023-04-12 at 2.59.18 PM.mov. (Show Details)

ginsu added a child revision: D7359: [native] implement onPressSetAvatar function.Apr 12 2023, 7:26 PM

Harbormaster completed remote builds in B18281: Diff 25054.Apr 12 2023, 7:34 PM

ginsu requested review of this revision.Apr 12 2023, 7:34 PM

ashoat accepted this revision.Apr 13 2023, 12:55 AM

ashoat mentioned this in D7402: [lib] introduce savedEmojiAvatarSelectorForCurrentUser selector.

ashoat added inline comments.

native/profile/emoji-avatar-creation.react.js
17 ↗	(On Diff #25054)	See my note about making this selector return a function in D7402

This revision is now accepted and ready to land.Apr 13 2023, 12:56 AM

address comments

Harbormaster completed remote builds in B18294: Diff 25071.Apr 13 2023, 6:09 AM

ashoat added inline comments.Apr 13 2023, 12:40 PM

native/profile/emoji-avatar-creation.react.js
22–27 ↗	(On Diff #25071)	We could pass initialization functions to `useState` to avoid needing to call `savedEmojiAvatarFunc()` when we don't need to, but it's not a huge deal
42 ↗	(On Diff #25071)	We could call `savedEmojiAvatarFunc()` here to avoid calling it unless we need to

address feedback

Harbormaster completed remote builds in B18329: Diff 25113.Apr 13 2023, 3:37 PM

ashoat added inline comments.Apr 13 2023, 4:00 PM

native/profile/emoji-avatar-creation.react.js
25 ↗	(On Diff #25113)	Shouldn't this be `.color`?

ginsu added inline comments.Apr 13 2023, 4:07 PM

native/profile/emoji-avatar-creation.react.js
25 ↗	(On Diff #25113)	thanks for catching that

fix

Harbormaster completed remote builds in B18333: Diff 25120.Apr 13 2023, 4:28 PM

Closed by commit rCOMMcaf80b74e363: [native] use savedEmojiAvatarSelectorForCurrentUser in EmojiAvatarCreation. · Explain WhyApr 13 2023, 4:30 PM

This revision was automatically updated to reflect the committed changes.

ginsu added a commit: rCOMMcaf80b74e363: [native] use savedEmojiAvatarSelectorForCurrentUser in EmojiAvatarCreation.

[native] use savedEmojiAvatarSelectorForCurrentUser in EmojiAvatarCreation
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 25124

native/navigation/route-names.js

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

[native] use savedEmojiAvatarSelectorForCurrentUser in EmojiAvatarCreationClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 25124

native/navigation/route-names.js

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

[native] use savedEmojiAvatarSelectorForCurrentUser in EmojiAvatarCreation
ClosedPublic
Actions

Revision Contents
Changeset List