[native] Support displaying encrypted avatars
ClosedPublic
Actions

Authored by bartek on Nov 1 2023, 1:04 AM.

Details

Reviewers

atul
ginsu

Commits

rCOMM6f544446b17c: [native] Support displaying encrypted avatars

Summary

Added possibility to display encrypted image avatars on native.

Depends on D9170

Test Plan

Encrypted avatars that I set on the web (previous diffs) are now displaying on native.
Confirmed that other avatars (both image and emoji) are still displaying correctly.
Tested in user tab, in chat and on chat list (both thread and user avatars).

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

bartek created this revision.Nov 1 2023, 1:04 AM

bartek held this revision as a draft.

Herald added subscribers: will, tomek, ashoat. · View Herald TranscriptNov 1 2023, 1:04 AM

bartek added a child revision: D9669: [native] Support avatar uploads to blob service.Nov 1 2023, 1:07 AM

bartek published this revision for review.Nov 1 2023, 1:23 AM

Harbormaster completed remote builds in B23640: Diff 32588.Nov 1 2023, 1:26 AM

atul accepted this revision.Nov 1 2023, 7:25 PM

atul added inline comments.

native/avatars/avatar.react.js
88 ↗	(On Diff #32588)	Hm, using `invariant` with `false` seems a bit strange. Should we instead have an invariant "above" that's something like invariant(avatarInfo.type === 'encrypted-image' \|\| avatarInfo.type === 'image', 'avatar must be of type encrypted-image or image'); or something like that?

atul added inline comments.Nov 1 2023, 7:27 PM

native/avatars/avatar.react.js
88 ↗	(On Diff #32588)	On the other hand, it does look like we do (or did) have a pattern of using `invariant(false, "blah")` for conditions that should be unreachable: so maybe it's fine? CC @ashoat, @tomek for thoughts

atul attached a referenced file: F848934: 605f64.png. (Show Details)Nov 1 2023, 7:27 PM

tomek added inline comments.Nov 2 2023, 1:56 AM

native/avatars/avatar.react.js
88 ↗	(On Diff #32588)	I would rather return `null` instead of having an avatar here. This issue isn't that big that we should kill the app. And if we decide in the future that some other `type`s are needed, it would be better to not display them instead of killing the app. Overall, `invariant`s should be used only if there's no other way, e.g. to express something our type system can't express.