-
Notifications
You must be signed in to change notification settings - Fork 280
WS-2769 - Update Inline JS: NextJs #14323
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
Open
louisearchibald
wants to merge
21
commits into
latest
Choose a base branch
from
WS-2769-update-inline-JS-nextJS
base: latest
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+63
−3
Open
Changes from 16 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
666c007
Merge branch 'latest' of ssh://github.com/bbc/simorgh into latest
louisearchibald 93cd21f
Merge branch 'latest' of ssh://github.com/bbc/simorgh into latest
louisearchibald 7dfa2f7
extract inline js into own helper files
louisearchibald b8ca483
use in document.page file
louisearchibald 813c5fc
Merge branch 'latest' into WS-2769-update-inline-JS-nextJS
louisearchibald 531785c
Merge branch 'WS-2769-update-inline-JS-nextJS' of ssh://github.com/bb…
louisearchibald 8d18cf4
Merge branch 'latest' into WS-2769-update-inline-JS-nextJS
louisearchibald 89e493d
Merge branch 'WS-2769-update-inline-JS-nextJS' of ssh://github.com/bb…
louisearchibald 793df09
remove arrow functions to maintain compatibility with opera mini
louisearchibald 11c10f1
Merge branch 'latest' into WS-2769-update-inline-JS-nextJS
louisearchibald 7f2b85e
copilot import suggestion
louisearchibald b08f36d
Merge branch 'latest' into WS-2769-update-inline-JS-nextJS
LilyL0u 07e60e6
Merge branch 'latest' into WS-2769-update-inline-JS-nextJS
LilyL0u 59a8aab
adds tests for removeNoJsClass
holchris d9c6257
adds test for getSimorghEnvVars
holchris 082ba49
Merge branch 'latest' into WS-2769-update-inline-JS-nextJS
louisearchibald 30ed11d
update inline JS in canonical renderer nextjs location
louisearchibald ba4b1de
Merge branch 'WS-2769-update-inline-JS-nextJS' of ssh://github.com/bb…
louisearchibald 45abf32
Merge branch 'latest' into WS-2769-update-inline-JS-nextJS
louisearchibald bed60eb
Merge branch 'WS-2769-update-inline-JS-nextJS' of ssh://github.com/bb…
louisearchibald 4dd4e6c
Merge branch 'latest' into WS-2769-update-inline-JS-nextJS
LilyL0u File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import removeNoJsClass from '.'; | ||
|
|
||
| describe('removeNoJsClass', () => { | ||
| afterEach(() => { | ||
| document.documentElement.className = ''; | ||
| }); | ||
|
|
||
| it('removes the no-js class from the document element', () => { | ||
| document.documentElement.classList.add('no-js'); | ||
|
|
||
| removeNoJsClass(); | ||
|
|
||
| expect(document.documentElement.classList.contains('no-js')).toBe(false); | ||
| }); | ||
|
|
||
| it('keeps other classes on the document element', () => { | ||
| document.documentElement.classList.add('no-js', 'other-class'); | ||
|
|
||
| removeNoJsClass(); | ||
|
|
||
| expect(document.documentElement.className).toBe('other-class'); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| function removeNoJsClass() { | ||
| document.documentElement.classList.remove('no-js'); | ||
| } | ||
|
holchris marked this conversation as resolved.
|
||
|
|
||
| export default removeNoJsClass; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| import { getProcessEnvAppVariables } from '../getEnvConfig'; | ||
| import setSimorghEnvVars from '.'; | ||
|
|
||
| describe('setSimorghEnvVars', () => { | ||
| const originalEnvVars = window.SIMORGH_ENV_VARS; | ||
|
|
||
| afterEach(() => { | ||
| window.SIMORGH_ENV_VARS = originalEnvVars; | ||
| }); | ||
|
|
||
| it('sets the environment variables on the window object', () => { | ||
| const envVars = getProcessEnvAppVariables(); | ||
|
|
||
| setSimorghEnvVars(envVars); | ||
|
|
||
| expect(window.SIMORGH_ENV_VARS).toBe(envVars); | ||
| }); | ||
|
|
||
| it('is declared as a classic function for Opera Mini compatibility', () => { | ||
| expect(setSimorghEnvVars.toString()).toMatch(/^function /); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| import type { EnvConfig } from '../getEnvConfig'; | ||
|
|
||
| function setSimorghEnvVars(envVars: EnvConfig) { | ||
| window.SIMORGH_ENV_VARS = envVars; | ||
| } | ||
|
holchris marked this conversation as resolved.
|
||
|
|
||
| export default setSimorghEnvVars; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Does this need to be done here too?
https://github.com/bbc/simorgh/blob/latest/ws-nextjs-app/renderers/CanonicalRenderer.tsx#L147
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've updated this now too, thanks for spotting. 🙏