[native] add feature flag to avatars render work
ClosedPublic
Actions

Authored by ginsu on Mar 20 2023, 7:06 AM.

Details

Reviewers

ashoat
atul
tomek

Commits

rCOMM30bbf4162d48: [native] add feature flag to avatars render work

Summary

added AVATARS_DISPLAY feature flag to all avatars render work that is ready to land. For this diff created a new hook called useShouldRenderAvatars which determines if the current user has the feature flag "on" and returns a boolean to either render or not render the avatar. The criteria for having the AVATARS_DISPLAY feature flag on is that they need to be a staff user. In subsequent diffs I will add the feature flag when introducing things like the thread avatars and user avatars in chat screen.

Depends on D7069

Linear Task: https://linear.app/comm/issue/ENG-3305/avatars-feature-flag-service

Test Plan

Ginsu who is a staff user can see avatars, but Dan who is not a staff avatar can not see the rendered avatars on their screen:

Ginsu's screen:

Dan's screen:

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Mar 20 2023, 7:06 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptMar 20 2023, 7:06 AM

ginsu edited the summary of this revision. (Show Details)Mar 20 2023, 7:09 AM

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

ginsu added reviewers: ashoat, atul, tomek.

ginsu added a parent revision: D7069: [native] render the rest of the user avatars.

ginsu attached a referenced file: F435116: Screen Recording 2023-03-20 at 3.40.31 AM.mov. (Show Details)

ginsu attached a referenced file: F435110: Screen Recording 2023-03-20 at 3.39.45 AM.mov. (Show Details)

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

ginsu added a child revision: D7067: [native] render user avatars in chat screen.Mar 20 2023, 7:17 AM

ginsu mentioned this in D7067: [native] render user avatars in chat screen.Mar 20 2023, 7:19 AM

Harbormaster completed remote builds in B17405: Diff 23847.Mar 20 2023, 7:21 AM

ginsu requested review of this revision.Mar 20 2023, 7:21 AM

ashoat accepted this revision.Mar 20 2023, 2:43 PM

ashoat added inline comments.

native/components/avatar.react.js
48	Just wondering... will the `Avatar` component ever be rendered in this case?
native/utils/avatar-utils.js
7	Nice, love how easy this is!!

This revision is now accepted and ready to land.Mar 20 2023, 2:43 PM

ginsu added inline comments.Mar 20 2023, 3:11 PM

native/components/avatar.react.js
48	It shouldn't. The demo videos in the test plan show that for users who aren't staff users, the `Avatar` component won't render. Does this answer your question?

ashoat added inline comments.Mar 20 2023, 3:11 PM

native/components/avatar.react.js
48	Do we need this check then?

ginsu added inline comments.Mar 20 2023, 3:15 PM

native/components/avatar.react.js
48	Oh sorry I misunderstood, this check is what makes the avatar component not render anything visually. We still render the actual avatar component but they will all just be `null` components

rebase before landing

Harbormaster failed remote builds in B17445: Diff 23900!Mar 20 2023, 3:40 PM

tomek added inline comments.Mar 21 2023, 1:52 AM

native/utils/avatar-utils.js
11 ↗	(On Diff #23900)	We probably should transform it to boolean. `featureFlagConfig` isn't required to have `AVATARS_DISPLAY` property. It's strange that Flow didn't catch that.

ginsu added inline comments.Mar 21 2023, 8:51 AM

native/utils/avatar-utils.js
11 ↗	(On Diff #23900)	Gotcha, flow was saying that I was getting a boolean value from this, but I will return `!!featureFlagConfig['AVATARS_DISPLAY'];` just to be super clear