[web] Wrap modals with `WebEditUserAvatarProvider`
ClosedPublic
Actions

Authored by atul on Jun 20 2023, 6:02 PM.

Details

Reviewers

ashoat
ginsu
michal
kamil
inka
rohan

Commits

rCOMM78b1d9a11c0c: [web] Wrap modals with `WebEditUserAvatarProvider`

Summary

We want to be able to access eg setUserAvatar from EmojiAvatarSelectionModal, however, we can't access the context since the modal is being rendered outside of "mainContent".

We're only wrapping modals with WebEditUserAvatarProvider when the user is logged in.

Depends on D8268

Test Plan

Able to access EditUserAvatarContext from EmojiAvatarSelectionModal as expected.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.Jun 20 2023, 6:02 PM

Herald added a subscriber: tomek. · View Herald TranscriptJun 20 2023, 6:02 PM

atul added inline comments.Jun 20 2023, 6:06 PM

web/app.react.js
176–179 ↗	(On Diff #27916)	Not sure if we ever have modals when logged out, but want to keep this a "noop refactor" and leave things as is.

Harbormaster completed remote builds in B20371: Diff 27916.Jun 20 2023, 6:20 PM

atul requested review of this revision.Jun 20 2023, 6:20 PM

atul added reviewers: michal, kamil, inka, rohan.Jun 21 2023, 9:47 AM

atul added a child revision: D8278: [web] Add "Save Avatar" button to `EmojiAvatarSelectionModal`.Jun 21 2023, 12:27 PM

Adding @ginsu since he introduced the equivalent provider on native.

This makes sense, but one question inline

web/app.react.js
186 ↗	(On Diff #27916)	Why didn't we wrap the `WebEditUserAvatarProvider` right under here? I know in the description it says: We're only wrapping modals with WebEditUserAvatarProvider when the user is logged in. I feel like wrapping `WebEditUserAvatarProvider` under here would be more simple, but are there other advantages to your current implementation that I might be overlooking?

This revision is now accepted and ready to land.Jun 22 2023, 3:58 PM

atul added inline comments.Jun 23 2023, 1:44 PM

web/app.react.js
186 ↗	(On Diff #27916)	Jotted down my reasoning here: https://phab.comm.dev/D8249?id=27866#inline-53090 If we were to move it under `MenuProvider`, isn't it possible that eg `userAvatarSaveInProgress` could be incorrect if the user logged out and back in? Thought it made sense to have a new instance of `WebEditUserAvatarProvider` for each user session? CC @ashoat to double check my assumptions/reasoning here