Skip to content

DS-743: Add heap script. #419

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 29, 2024
Merged

DS-743: Add heap script. #419

merged 1 commit into from
May 29, 2024

Conversation

sherakama
Copy link
Member

@sherakama sherakama commented May 28, 2024 •
edited
Loading

READY FOR REVIEW

Summary

  • Adds the same heap script to giving as campaign

Review By (Date)

  • End of the week?

Criticality

  • Normal

Review Tasks

  1. Check out this branch
  2. Fire up npm run dev
  3. Review homepage in the browser and inspect the head for the heap script
  4. Validate no heap script
  5. Set CONTEXT=production in your .env file
  6. Stop your dev instance
  7. Run npm run clean
  8. Start your dev instance npm run dev
  9. Review homepage in the browser and inspect the head for the heap script
  10. Validate heap script loads

sherakama
sherakama commented May 29, 2024
View reviewed changes
gatsby-config.js
Comment on lines -5 to -7
const activeEnv =
process.env.GATSBY_ACTIVE_ENV || process.env.NODE_ENV || 'development';

Copy link
Member Author

Choose a reason for hiding this comment

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

This was never used so I removed it.

sherakama
sherakama commented May 29, 2024
View reviewed changes
src/components/partials/htmlHead.js
@@ -104,6 +106,15 @@ const HtmlHead = () => {
href="https://fonts.googleapis.com/css2?family=Source+Sans+3:ital,wght@0,400;0,600;0,700;1,400;1,600;1,700&family=Source+Serif+4:ital,wght@0,400;0,600;0,700;1,400;1,600;1,700&display=swap"
rel="stylesheet"
/>
{isProduction && (
<script
Copy link
Member Author

@sherakama sherakama May 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The Gatsby <Script /> component wasn't working. No errors, just nothing rendered. I punted and the whole head management has changed in Gatsby and should be upgraded but we also have a rebuild to next in our future.

stfurkan
stfurkan approved these changes May 29, 2024
View reviewed changes
Copy link
Contributor

@stfurkan stfurkan left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @sherakama :)

image

@sherakama sherakama merged commit 64be86b into dev May 29, 2024
5 checks passed
@sherakama sherakama deleted the DS-743 branch May 29, 2024 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stfurkan stfurkan stfurkan approved these changes

@rebeccahongsf rebeccahongsf Awaiting requested review from rebeccahongsf

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sherakama @stfurkan