Page MenuHomePhabricator

[web] Consolidate `errorMessage` and `updateSuccessful` state in `EmojiAvatarSelectionModal`
ClosedPublic

Authored by atul on Jun 21 2023, 2:58 PM.
Tags
None
Referenced Files
F2104853: D8283.id.diff
Tue, Jun 25, 5:52 AM
Unknown Object (File)
Sat, Jun 22, 2:35 AM
Unknown Object (File)
Thu, Jun 13, 12:16 PM
Unknown Object (File)
Thu, Jun 13, 10:29 AM
Unknown Object (File)
Sun, Jun 9, 1:16 AM
Unknown Object (File)
Sun, Jun 9, 1:15 AM
Unknown Object (File)
Wed, Jun 5, 10:35 PM
Unknown Object (File)
Wed, Jun 5, 10:34 PM
Subscribers

Details

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:

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul requested review of this revision.Jun 21 2023, 3:19 PM
rohan added inline comments.
web/avatars/emoji-avatar-selection-modal.react.js
38 ↗(On Diff #27982)

I think it'd read better if these variants were pulled outside of this useState, but it should be fine if there are only going to be 2 possible states

This revision is now accepted and ready to land.Jun 22 2023, 6:25 AM
This revision was landed with ongoing or failed builds.Jun 23 2023, 2:07 PM
This revision was automatically updated to reflect the committed changes.