[lib] Make `displayFailureAlert` optional in `BaseEditUserAvatar`
ClosedPublic
Actions

Authored by atul on Jun 26 2023, 2:45 PM.

Details

Reviewers

ashoat
ginsu
rohan

Commits

rCOMM2dabda2b573f: [lib] Make `displayFailureAlert` optional in `BaseEditUserAvatar`

Summary

displayFailureAlert is native-specific functionality which we won't implement for web.

We previously had an implementation using alert(...), but showing a browser alert was not consistent with the rest of web's error handling. It makes sense to have displayFailureAlert be optional and only implement it for native. Later in the stack we will handle this from native/.../avatar-hooks.js instead.

NOTE: The higher level goal with this (and subsequent) refactor(s) is for [web/native]/.../avatar-hooks.react.js to be platform-specific and entity(user vs. thread)-agnostic AND for edit-*-avatar-provider.react.js to be platform-agnostic and entity-specific. Right now there's some coupling between the two that makes implementation of image avatars on web pretty difficult... will work on cleaning things up in this stack. TLDR: avatar providers will be common interface between client and keyserver and avatar hooks will be platform-specific implementations to get things from platform-specific types (eg MediaLibrarySelection) to platform-agnostic types (eg UpdateUserAvatarRequest) so they can be passed to API provided by providers.

Depends on D8315

Test Plan

Alerts continue to appear as expected on native. Alerts continue NOT to appear on web as expected.

Diff Detail

Repository

rCOMM Comm

Branch

arcpatch-D8316 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Jun 26 2023, 2:45 PM

Herald added a subscriber: tomek. · View Herald TranscriptJun 26 2023, 2:45 PM

Later in the stack we will handle this from native/.../avatar-hooks.js instead.

Specifically we'll just throw an exception from the "platform-agnostic" provider, and surface the error to user in "platform-specific" way via avatar-hooks.react.js

atul published this revision for review.Jun 26 2023, 3:02 PM

Harbormaster completed remote builds in B20511: Diff 28123.Jun 26 2023, 3:04 PM

atul added a child revision: D8318: [lib] Move `selectFromGalleryAndUpdateUserAvatar` from provider to `avatar-hooks`.Jun 26 2023, 3:32 PM

ginsu accepted this revision.Jun 26 2023, 4:10 PM

This revision is now accepted and ready to land.Jun 26 2023, 4:10 PM

rebase and land

This revision was landed with ongoing or failed builds.Jun 27 2023, 8:30 AM

Closed by commit rCOMM2dabda2b573f: [lib] Make `displayFailureAlert` optional in `BaseEditUserAvatar` (authored by atul). · Explain Why

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMM2dabda2b573f: [lib] Make `displayFailureAlert` optional in `BaseEditUserAvatar`.

Harbormaster completed remote builds in B20547: Diff 28179.Jun 27 2023, 8:44 AM