-
Notifications
You must be signed in to change notification settings - Fork 262
WS-1838: Add PWA Offline page tracking #13545
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: ws-1837-service-worker-changes
Are you sure you want to change the base?
WS-1838: Add PWA Offline page tracking #13545
Conversation
|
Can we add some tests around the new |
|
|
||
| const title = 'You are offline'; | ||
| const message = | ||
| "It seems you don't have an internet connection at the moment. Please check your connection and reload the page."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a semantic suggestion - the word “connection” seems a bit repetitive, what if we simplified & removed the bullet points? E.g.:
You’re Offline
Looks like you’re not online right now. Please check your network and reconnect. Once you’re back, just refresh the page to continue.
| statsDestination, | ||
| ].every(Boolean); | ||
|
|
||
| console.log('Custom Event Tracker', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can remove the console.logs at this point.
| }, [isPWA]); | ||
| }; | ||
|
|
||
| export default useOfflinePageFlag; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: export { useOfflinePageFlag, OFFLINE_VISIT_FLAG };
| it('should set offline flag when in PWA mode', () => { | ||
| mockUseIsPWA.mockReturnValue(true); | ||
|
|
||
| renderHook(() => useOfflinePageFlag()); | ||
|
|
||
| expect(localStorage.setItem).toHaveBeenCalledWith( | ||
| OFFLINE_VISIT_FLAG, | ||
| 'true', | ||
| ); | ||
| }); | ||
|
|
||
| it('should not set flag when in browser mode', () => { | ||
| mockUseIsPWA.mockReturnValue(false); | ||
|
|
||
| renderHook(() => useOfflinePageFlag()); | ||
|
|
||
| expect(localStorage.setItem).not.toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is all this logic redundant as we only show the offline page in the PWA anyway? So the flag would only be set on offline pages that would only be visited in the PWA? Sorry I may be missing something here.
| expect(mockTrackOfflinePageViewEvent).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should fire event when in PWA mode, online, and flag is set', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the flag is set it must have been set while the PWA was offline, I'm not sure it matters if the event is sent from browser mode? I think this could potentially decrease the number of metrics we send as multiple offline visits will be treated as one if the event is not dispatched and the flag cleared.
So current logic (PWA state considered):
PWA == true && Online == false -> set flag
PWA == false && Online == true -> Do nothing as PWA is false
PWA == true && Online == false -> No need to set flag as already set
PWA == true && Online == true -> Dispatch tracking event
In this logic only one event is sent even though the user completed to offline browsing sessions that could have been tracked.
With alternative logic (PWA state ignored)
PWA == true && Online == false -> set flag
PWA == false && Online == true -> Dispatch tracking event as we are online and a previous offline session was recorded while using the PWA
PWA == true && Online == false -> set flag again as we have another offline session since the last one
PWA == true && Online == true -> Dispatch tracking event again as we've had another offline session since the first
This revised logic tracks both sessions seperately giving us more accurate analytics
| '4g', | ||
| '5g', | ||
| 'unknown', | ||
| ] as const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn be good to use jests each functionality here: https://jestjs.io/docs/api#testeachtablename-fn-timeout
| eventName: OFFLINE_PAGE_VIEW_EVENT_NAME, | ||
| }); | ||
|
|
||
| console.log('usePWAOfflineTracking invoked', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be removed
Co-authored-by: Andrew Bennett <andrew.bennett07@bbc.co.uk>
Resolves JIRA: https://bbc.atlassian.net/browse/WS-1838
Summary
Adding flag for offline tracking
New Hooks:
useOfflinePageFlag - Sets flag when offline page visited in PWA
usePWAOfflineTracking - Fires pwa-offline-page-view event on offline→online transition
useSendPWAStatus - Sends PWA status to service worker
useServiceWorkerRegistration - Handles SW registration:
Added tests for all 4 hooks following codebase patterns
Fixed merge conflict in LatestMediaItem styles
Updated Storybook 10.0.8 → 10.1.10
Testing Instructions
How to test the offline flag and tracking
Setup:
Stop your dev server
Close Chrome completely and uninstall any existing PWA
Start the dev server: yarn dev
Go to http://localhost:7081/mundo/live/c7dkx155e626t?renderer_env=local
Install it as a PWA (click the install icon in the address bar)
Test the flag:
Once installed, refresh the page to make sure the offline page gets cached
Open DevTools, go to Network tab and set it to Offline
Refresh the page - you should see the custom offline page
Check Local Storage (in DevTools → Application → Local Storage)
You should see offline_page_visit with value "true"
Test the tracking event:
While still on the offline page with the flag set
Open the Console tab in DevTools
Switch Network back to Online
You should see a tracking beacon fire with event name pwa-offline-page-view and network type
Check Local Storage again - the offline_page_visit flag should be gone
Test it fires again:
Go offline again, refresh to see the offline page
The flag gets set again
Go back online
The event fires again and the flag is removed
That's it. The event should fire every time you go from offline → online after visiting the offline page.
Developer Checklist
Testing
Ready-For-Test, Local)Ready-For-Test, Test)Ready-For-Test, Preview)Ready-For-Test, Live)Additional Testing Steps
Useful Links