WS-1400: Add client side redirect #13407
Conversation
|
Thanks for addressing the comments! |
| | '2g' | ||
| | '3g' | ||
| | '4g' | ||
| | '5g' |
There was a problem hiding this comment.
Still not convinced we'll get 5g through as per the docs - https://developer.mozilla.org/en-US/docs/Glossary/Effective_connection_type (though I know that was there before)
There was a problem hiding this comment.
Hello, I put this one here because it partially clashed with the hook that Neon are working on. I don't want to make any changes in case it affects their work.
|
|
||
| const allowList = ['/pidgin/articles/czrzwn80zjmo']; | ||
| if ( | ||
| window?.navigator?.connection?.effectiveType && |
There was a problem hiding this comment.
Older browsers like Opera Mini don’t support optional chaining I don't think, so they might hit a SyntaxError before the redirect runs (with the redirectScript.toString() injected in the script tag on lines 30-34
There was a problem hiding this comment.
The script should be transpiled down by the time its rendered in the browser. Worth checking locally that that is the case.
| return ( | ||
| <script> | ||
| {` | ||
| window.addEventListener('load', () => { |
There was a problem hiding this comment.
shouldn't we use https://developer.mozilla.org/en-US/docs/Web/API/Document/DOMContentLoaded_event instead? The window load will happen after all HTML/JSON/CSS/images load so perhaps too long?
Resolves JIRA: https://bbc.atlassian.net/browse/WS-1400
Summary
This is part of a spike, the following changes automatically redirect users on low data speeds. This approach uses Helmet and vanilla javascript. Alternative approaches including using the Next router, and Belfrage/GTM/Fastly were considered, but we felt that this was the best approach going forward.
Code changes
Developer Checklist
Testing
Ready-For-Test, Local)Ready-For-Test, Test)Ready-For-Test, Preview)Ready-For-Test, Live)Additional Testing Steps
Useful Links