Skip to content

Commit

Permalink
[web] Consolidate errorMessage and updateSuccessful state in `Emo…
Browse files Browse the repository at this point in the history
…jiAvatarSelectionModal`

Summary:
Since we're only displaying one error message, it doesn't really make sense to store the `errorMessage` in component state as `string`... what we really need is a `boolean`.

That would leave us with two boolean component states (`updateFailed` and `updateSuccessful`). Since those states are mutually exclusive, it makes sense to consolidate them into a single state of type `?('success' | 'failure')`.

NOTE: Still think it makes sense to leave D8280 and D8281 up since they cover some other changes (styling and whatnot) and squashing this diff with those may lead to a large changeset. They also may better show how things build up? However, if it's easier for the reviewer to have them squashed I'm happy to update this stack to do so.

---

Depends on D8281

Test Plan:
Things continue to work as expected:

{F600823}

Reviewers: ashoat, ginsu, rohan

Reviewed By: rohan

Subscribers: tomek

Differential Revision: https://phab.comm.dev/D8283
  • Loading branch information
atulsmadhugiri committed Jun 23, 2023
1 parent b904988 commit 72b8c7f
Showing 1 changed file with 9 additions and 12 deletions.
21 changes: 9 additions & 12 deletions web/avatars/emoji-avatar-selection-modal.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,8 @@ function EmojiAvatarSelectionModal(): React.Node {

const { setUserAvatar, userAvatarSaveInProgress } = editUserAvatarContext;

const [errorMessage, setErrorMessage] = React.useState<?string>();
const [updateSuccessful, setUpdateSuccessful] =
React.useState<boolean>(false);
const [updateAvatarStatus, setUpdateAvatarStatus] =
React.useState<?('success' | 'failure')>();

const currentUserInfo = useSelector(state => state.currentUserInfo);
const currentUserAvatar: ClientAvatar = getAvatarForUser(currentUserInfo);
Expand Down Expand Up @@ -67,23 +66,21 @@ function EmojiAvatarSelectionModal(): React.Node {
);

const onEmojiSelect = React.useCallback(selection => {
setErrorMessage();
setUpdateSuccessful(false);
setUpdateAvatarStatus();
setPendingAvatarEmoji(selection.native);
}, []);

const onColorSelection = React.useCallback((hex: string) => {
setErrorMessage();
setUpdateSuccessful(false);
setUpdateAvatarStatus();
setPendingAvatarColor(hex);
}, []);

const onSaveAvatar = React.useCallback(async () => {
try {
await setUserAvatar(pendingEmojiAvatar);
setUpdateSuccessful(true);
setUpdateAvatarStatus('success');
} catch {
setErrorMessage('Avatar update failed. Please try again.');
setUpdateAvatarStatus('failure');
}
}, [pendingEmojiAvatar, setUserAvatar]);

Expand All @@ -92,20 +89,20 @@ function EmojiAvatarSelectionModal(): React.Node {
if (userAvatarSaveInProgress) {
buttonColor = buttonThemes.standard;
saveButtonContent = <LoadingIndicator status="loading" size="medium" />;
} else if (updateSuccessful) {
} else if (updateAvatarStatus === 'success') {
buttonColor = buttonThemes.success;
saveButtonContent = (
<>
<SWMansionIcon icon="check-circle" size={24} />
{'Avatar update succeeded.'}
</>
);
} else if (errorMessage) {
} else if (updateAvatarStatus === 'failure') {
buttonColor = buttonThemes.danger;
saveButtonContent = (
<>
<SWMansionIcon icon="warning-circle" size={24} />
{errorMessage}
{'Avatar update failed. Please try again.'}
</>
);
} else {
Expand Down

0 comments on commit 72b8c7f

Please sign in to comment.