Page MenuHomePhabricator

[native] add feature flag to avatars render work
ClosedPublic

Authored by ginsu on Mar 20 2023, 7:06 AM.
Tags
None
Referenced Files
F1434065: D7099.id23900.diff
Thu, Mar 28, 8:18 AM
F1434064: D7099.id23944.diff
Thu, Mar 28, 8:18 AM
F1434063: D7099.id23941.diff
Thu, Mar 28, 8:18 AM
F1434062: D7099.id23847.diff
Thu, Mar 28, 8:18 AM
F1434015: D7099.id.diff
Thu, Mar 28, 8:18 AM
F1433986: D7099.diff
Thu, Mar 28, 8:13 AM
Unknown Object (File)
Sat, Mar 16, 7:58 AM
Unknown Object (File)
Sat, Mar 16, 7:45 AM

Details

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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: ashoat, atul, tomek.
ginsu edited the summary of this revision. (Show Details)
ginsu requested review of this revision.Mar 20 2023, 7:21 AM
ashoat added inline comments.
native/components/avatar.react.js
48 ↗(On Diff #23847)

Just wondering... will the Avatar component ever be rendered in this case?

native/utils/avatar-utils.js
7 ↗(On Diff #23847)

Nice, love how easy this is!!

This revision is now accepted and ready to land.Mar 20 2023, 2:43 PM
native/components/avatar.react.js
48 ↗(On Diff #23847)

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?

native/components/avatar.react.js
48 ↗(On Diff #23847)

Do we need this check then?

native/components/avatar.react.js
48 ↗(On Diff #23847)

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

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.

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

Screenshot 2023-03-21 at 11.50.28 AM.png (286×848 px, 61 KB)

ginsu added inline comments.
native/utils/avatar-utils.js
1