Depends on D13996
when the explore communities button is pressed, we should navigate to the modal. if for some reason we are unable to fetch the community info from the keyserver, we will display an alert asking the user to try again
Details
Details
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| native/navigation/community-drawer-content.react.js | ||
|---|---|---|
| 196 | I'm not sure if this is the right experience. we could instead just disable the explore communities button if !fetchedCommunitiesWithNames, but that feels worse | |
Comment Actions
Seems like we're adding two new keyserver calls on app start here, but I think we can do this without adding any
| native/navigation/community-drawer-content.react.js | ||
|---|---|---|
| 64–66 | This is not a promise, but rather a function that returns a promise | |
| 77–84 | I don't love that we're doing both of these calls separately. Doesn't fetchAllCommunityInfosWithNamesActionTypes have all the data that fetchCommunityInfosActionTypes needs? Can you combine them? | |
| 86 | You're creating the promise twice, which I think results in two calls to the keyserver. You should only invoke fetchPromise once, and then use the returned Promise here, as well as passing it to dispatchActionPromise | |
| 196 | Seems like this could happen if the user presses the button while the first request is ongoing, which might be a likely thing to happen and a negative experience I think you should assign the Promise to a ref when you create it in the useEffect above, and then await it here if fetchedCommunitiesWithNames isn't set yet After you do that, then this should be less likely to occur and probably an Alert isn't the worse thing to do | |
| lib/reducers/community-reducer.js | ||
|---|---|---|
| 50 ↗ | (On Diff #46389) | Thought a bit about whether we should also have a KNOW_OF check here, but I don't think it's possible (or will likely ever be possible) to be a member without KNOW_OF |
| native/navigation/community-drawer-content.react.js | ||
| 219–221 ↗ | (On Diff #46389) | I think it's okay for this one to be "cancelable" (dismissable by pressing the backdrop) since it's just something we want the user to acknowledge. I think a good rule of thumb (we probably don't consistently follow it though) is that if there's just one button and it's "OK", it probably should be cancelable |