feat(pwa): track online status [LIBS-315] #718
Conversation
There was a problem hiding this comment.
I've left some comments relating to code style. You have also provided me with in depth explanation about various parts of this PR on Slack and during meetings. After receiving these, I think the changes you've implemented / the approach you took here make a lot of sense.
There are still quite a few todo comments in the code and the PR is still draft, so I'll just leave this review as a "comment". I can have another look once you take this PR out of draft mode.
| export function handleRecordedRequest({ request, event }) { | ||
| const recordingState = self.clientRecordingStates[event.clientId] | ||
| export class RecordingMode extends Strategy { | ||
| _handle(request, handler) { | ||
| const { event } = handler | ||
| const recordingState = self.clientRecordingStates[event.clientId] |
There was a problem hiding this comment.
This handler is refactored to a Workbox route handling strategy to take advantage of the 'plugin' functionality
| window.localStorage.DHIS2_BASE_URL, | ||
| url: process.env.REACT_APP_DHIS2_BASE_URL, |
There was a problem hiding this comment.
localStorage is now checked in ServerVersionProvider.js
| /** | ||
| * A plugin to hook into lifecycle events in workbox strategies | ||
| * https://developer.chrome.com/docs/workbox/using-plugins/ | ||
| */ | ||
| export const dhis2ConnectionStatusPlugin = { | ||
| // todo: remove console logs | ||
| fetchDidFail: async ({ request, error }) => { | ||
| console.log('fetch did FAIL', { request, error }) | ||
|
|
||
| if (await isRequestToDhis2Server(request)) { | ||
| broadcastDhis2ConnectionStatus(false) | ||
| } | ||
| }, | ||
| fetchDidSucceed: async ({ request, response }) => { | ||
| console.log('fetch did SUCCEED', { request, response }) | ||
|
|
||
| if (await isRequestToDhis2Server(request)) { | ||
| broadcastDhis2ConnectionStatus(true) | ||
| } | ||
| return response | ||
| }, | ||
| } |
There was a problem hiding this comment.
This plugin is used in set-up-service-worker.js
|
I had a quick look at this PR again, and while the code is looking good to me, there are still a lot of todo comments in the newly added code. Since it's not obvious to me if these are just ideas for potential future improvements, known issues that will be addressed later, or things that should actually get fixed before the PR is merged, I stopped reviewing and didn't functionally test either. |
|
Todos have been clarified |
|
Just to note: I decided to approve this PR and its counterpart dhis2/app-runtime#1203 because the code is looking good. But before merging, please take care to either remove the |
# [10.3.0](v10.2.3...v10.3.0) (2023-03-03) ### Features * **pwa:** track online status [LIBS-315] ([#718](#718)) ([1dfd1e6](1dfd1e6))
|
🎉 This PR is included in version 10.3.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Adds the service worker, offline interface, and app adapter foundation for tracking the DHIS2 connection status alongside incidental requests.
Part of LIBS-315
The spec
Notes for this PR
To test locally:
update: some of these tools are not available any more, since much of the testing has passed
yarn build:pwa && yarn build:adapteryarn temp:startto copy the built app-runtime libs into the app-platform node_modules and start the PWA appTodos: