This diff introduces the loading logic needed for the relationship prompts on both web and native. Initially, I thought about using createLoadingStatusSelector to implement the loading logic; however, the issue with this was that we sometimes show two relationship buttons and using a single createLoadingStatusSelector would cause both button to show loading spinners at the same time even though one button was pressed, which I felt was a bit strange. So I introduced 4 separate loading states (one for each action) and returned those values in this hook to be consumed on both web + native
Details
Details
At the end of the stack confirmed that the relationship prompt buttons were being disabled + showing some sort of visual indication to the user that the action is loading
Depends on D12651
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
We shouldn't set all loading states to false when just one resolves
lib/hooks/relationship-prompt.js | ||
---|---|---|
38 ↗ | (On Diff #41765) | Can we rename loaders to inProgress or loadingState? When I read "loader" I think of a function that will get called to do some loading |
79 ↗ | (On Diff #41765) | Can we rename loaders to inProgress or loadingState? When I read "loader" I think of a function that will get called to do some loading |
102–105 ↗ | (On Diff #41765) | It's a bit weird that we set all of these to false here. Can we try this instead? const updateRelationship = React.useCallback( async (action: TraditionalRelationshipAction, setInProgress: boolean => mixed) => { try { setInProgress(true); invariant(otherUserID, 'Other user info id should be present'); return await callUpdateRelationships({ action, userIDs: [otherUserID], }); } catch (e) { onErrorCallback?.(); throw e; } finally { setInProgress(false); } }, [callUpdateRelationships, onErrorCallback, otherUserID], ); const dispatchActionPromise = useDispatchActionPromise(); const onButtonPress = React.useCallback( (action: TraditionalRelationshipAction, setInProgress: boolean => mixed) => { void dispatchActionPromise( updateRelationshipsActionTypes, updateRelationship(action, setInProgress), ); }, [dispatchActionPromise, updateRelationship], ); const blockUser = React.useCallback( () => onButtonPress(relationshipActions.BLOCK, setIsLoadingBlockUser), [onButtonPress], ); const unblockUser = React.useCallback( () => onButtonPress(relationshipActions.UNBLOCK, setIsLoadingUnblockUser), [onButtonPress], ); const friendUser = React.useCallback( () => onButtonPress(relationshipActions.FRIEND, setIsLoadingFriendUser), [onButtonPress], ); const unfriendUser = React.useCallback( () => onButtonPress(relationshipActions.UNFRIEND, setIsLoadingUnfriendUser), [onButtonPress], ); |
lib/hooks/relationship-prompt.js | ||
---|---|---|
126 ↗ | (On Diff #41862) | Can you move the setInProgress(true); call to the start of updateRelationship, like in my code snippet from the last review? |