[lib] introduce savedEmojiAvatarSelectorForCurrentUser selector
ClosedPublic
Actions

Authored by ginsu on Apr 12 2023, 11:55 AM.

Details

Reviewers

atul
ashoat

Commits

rCOMM842d73e6de51: [lib] introduce savedEmojiAvatarSelectorForCurrentUser selector

Summary

Based on some 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 selector that would be called in EmojiAvatarCreation to get the current emoji avatar from redux for the current user

Depends on D7401

Test Plan

The reset functionality behaves as expected as seen in the demo below

An example where we are getting a random default avatar:

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ginsu created this revision.Apr 12 2023, 11:55 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptApr 12 2023, 11:55 AM

ginsu edited the test plan for this revision. (Show Details)Apr 12 2023, 12:00 PM

ginsu added reviewers: atul, ashoat.

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

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

ginsu added a child revision: D7359: [native] implement onPressSetAvatar function.

Harbormaster completed remote builds in B18279: Diff 25051.Apr 12 2023, 12:13 PM

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

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

ginsu added a parent revision: D7401: [lib] introduce getRandomDefaultEmojiAvatar function.

ginsu added inline comments.

lib/selectors/user-selectors.js
220 ↗	(On Diff #25051)	This is for the case if the user has an ens/image avatar already set as their current avatar and they want to switch to an emoji avatar
221 ↗	(On Diff #25051)	I thought making the new default emoji avatar when you are creating a new emoji avatar would make the experience more dynamic; however, if we just want to use the default emoji avatar based off the hash function I created earlier I can switch this

ginsu edited the test plan for this revision. (Show Details)Apr 12 2023, 12:23 PM

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

ginsu added a child revision: D7403: [native] use savedEmojiAvatarSelectorForCurrentUser in EmojiAvatarCreation.

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

ashoat accepted this revision.Apr 12 2023, 5:48 PM

ashoat mentioned this in D7401: [lib] introduce getRandomDefaultEmojiAvatar function.

ashoat added inline comments.

lib/selectors/user-selectors.js
221 ↗	(On Diff #25051)	I like this idea

This revision is now accepted and ready to land.Apr 12 2023, 5:48 PM

ashoat added inline comments.Apr 12 2023, 5:55 PM

lib/selectors/user-selectors.js
221 ↗	(On Diff #25051)	That said, one thing that's weird here is that the random selection changes based on the inputs to the selector. If in the future we changed the selector to have a new param, then we might see the output of the selector randomly change. In our case we actually aren't worried about this because we only do "spot checks" of this value (see D7403), once when `EmojiAvatarCreation` first renders and then once when `onPressReset` is called. To better reflect that this is only used for spot checks, and that its value can change randomly, I think we should return a function (that takes no parameters) here. We do that elsewhere in the codebase in cases where the value can change randomly or over time.