Details
Confirmed that a new role is created when the button is clicked
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
We should make sure that the request is complete before dismissing the modal.
web/roles/create-roles-modal.react.js | ||
---|---|---|
118–130 ↗ | (On Diff #28945) | Should we wait for callModifyCommunityRole to complete successfully before dismissing modal? It looks pretty instantaneous in local dev environment, but might take a second on prod. We can pass an IIFE as the second argument to dispatchActionPromise that wraps the call to callModifyCommunityRole and calls popModal() after awaiting callModifyCommunityRole? Here's an example of passing in IIFE that wraps call* to dispatchActionPromise: (lib/components/base-edit-thread-avatar-provider.react.js) This will also let us catch error which would be helpful for "TODO: Error handling in a later diff"? Here's an example: (native/community-creation/community-configuration.react.js) |
Address feedback (added a eslint suppression since otherwise I got a Unnecessary try/catch wrapper error. This should be addressed once I display an unknown error alert to the client). Also added a loading indicator
keyserver/src/creators/role-creator.js | ||
---|---|---|
117 | Oh forgot to remove this |
web/roles/create-roles-modal.react.js | ||
---|---|---|
145–147 ↗ | (On Diff #29071) | Instead of catching and re-throwing, can we remove the try/catch altogether for now? |
161–169 ↗ | (On Diff #29071) | Was going to suggest something like const saveButtonContent = React.useMemo( () => createRolesLoadingStatus === 'loading' ? ( <LoadingIndicator status={createRolesLoadingStatus} size="medium" /> ) : ( 'Save' ), [createRolesLoadingStatus], ); but it ends up being the same number of lines after being formatted lol (and arguably less readable?) |