Skip to content

Conversation

KManolov3
Copy link
Contributor

Resolves

https://scratchfoundation.atlassian.net/browse/UEPR-38

Proposed Changes

Implements a debugging modal with text topics

Reason for Changes

Explain why these changes should be made

Test Coverage

Please show how you have added tests to cover your changes

Comment on lines 136 to 144
// Google Tag Manager ID
// Looks like 'GTM-XXXXXXX'
gtm_id: process.env.GTM_ID || '',

// Google Tag Manager env & auth info for alterative GTM environments
// Looks like '&gtm_auth=0123456789abcdefghijklm&gtm_preview=env-00&gtm_cookies_win=x'
// Taken from the middle of: GTM -> Admin -> Environments -> (environment) -> Get Snippet
// Blank for production
gtm_env_auth: process.env.GTM_ENV_AUTH || ''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the sake of consistency, I'd recommend moving this stuff into something like:

const commonHtmlWebpackPluginOptions = {
    // Google Tag Manager ID
    gtm_id: // etc
};

Then you can add something like this here and in all the other pages:

    .addPlugin(new HtmlWebpackPlugin({
        ...commonHtmlWebpackPluginOptions,
        chunks: // etc

That way, all of the generated pages have the same GTM config without needing to manually copy it to each one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also worth checking with Georgi regarding how this might affect GUI stand-alone mode, if you haven't already, but I imagine it's probably fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted the config in a common variable and reused it for the configuration of the rest of the HtmlWebpackPlugin's. Was that what you had in mind?

On your second point - yes, I coordinated this with Georgi.

@KManolov3 KManolov3 merged commit c55b066 into scratchfoundation:develop Nov 1, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants