Site creation rework: theme loading retry when connection resumes#7247
Conversation
hypest
changed the title
|
I piggybacked:
|
theck13
left a comment
theck13
left a comment
There was a problem hiding this comment.
There are a few optimizations we can make. Other than that, everything looks good!
| || failedState.getStep() == SiteCreationStep.NEW_SITE) { | ||
| createSite(); | ||
| } else { | ||
| retryFromState(failedState, (long) failedState.getPayload()); |
There was a problem hiding this comment.
Both the createSite and retryFromState methods are declared private and they are accessed outside the private scope as well as the mIsPageFinished variable. Making them package-private or protected will avoid create wasteful synthetic accessor methods.
| public boolean isCreationSucceeded() { | ||
| return mCreationSucceeded; | ||
| SiteCreationState state = SiteCreationService.getState(); | ||
| return state != null && SiteCreationService.getState().getStep() == SiteCreationStep.SUCCESS; |
There was a problem hiding this comment.
Since state is created in the line above, can't we simplify SiteCreationService.getState().getStep() to state.getStep() to minimize the SiteCreationService.getState() calls? Otherwise, creating state doesn't reduce the number of SiteCreationService.getState() calls.
There was a problem hiding this comment.
Oh, good find! I was going to reuse the variable but that fell between the cracks! Done with 12a8e0f.
|
Here's a gif with how the no-network toast done in
|
theck13
deleted the
feature/site-creation-theme-retry-when-connectivity
branch
2 participants
Listen for network connectivity and try the themes network fetch again when connectivity is restored.
To test #1:
This PR also includes the fix for #7221 . Shows a "Close" button when the site-creation process finishes but the new site has been created remotely. If it hasn't, a "Back" button is displayed.
To test #2:
To test #3: