Skip to content
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

Turn scroll tracking on for various pages #2324

Merged
merged 1 commit into from
Jan 6, 2022

Conversation

andysellick
Copy link
Contributor

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

What

Turn on scroll tracking for various pages.

NOTE this change depends on this change in the components gem being deployed to static at the same time, otherwise duplicate or no scroll tracking events may occur.

Why

We're retiring the old scroll tracker and migrating all pages that use it to the new one.

Visual changes

None.

Trello card: https://trello.com/c/sdugtMbX/37-migrate-to-new-scroll-tracker

@govuk-ci govuk-ci temporarily deployed to government-f-migrate-to-2kzpuv January 5, 2022 13:24 Inactive
@andysellick andysellick changed the title Turn scroll tracking on for various pages [DO NOT MERGE] Turn scroll tracking on for various pages Jan 5, 2022
Copy link
Member

@kevindew kevindew left a comment

Choose a reason for hiding this comment

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

Nice one! looks a big improvement on the masses of configuration in: https://github.com/alphagov/govuk_publishing_components/pull/2548/files

I think I've caught a small issue with guides, otherwise I've done a quick skim the paths match the source PR but I've not been too thorough.

"/taking-goods-out-uk-temporarily/get-an-ata-carnet",
]

full_url = [@content_item.base_path, @content_item.part_slug].join('/')
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this will operate correctly, given the paths you have listed above.

I assume we're expecting part_slug to sometimes be nil in order to have paths like "/council-housing", however if we do ["/council-housing", nil].join("/") we get /council-housing/. Perhaps you want to do [@content_item.base_path, @content_item.part_slug].compact.join('/') ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great spot, thanks! Have updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The trello card links to a spreadsheet of all these URLs and some more details about them, including what the original tracking was and what changes have been made. Each one falls into one of these categories:

  • not needed and scroll tracking has (probably already) been removed
  • was tracking headings but content has changed so no tracking was occurring, so have removed tracking
  • was tracking headings or percentages so has been migrated to the new scroll tracker

@andysellick andysellick force-pushed the migrate-to-new-scrolltracker branch from c51c656 to e0ca197 Compare January 6, 2022 09:25
@govuk-ci govuk-ci temporarily deployed to government-f-migrate-to-2kzpuv January 6, 2022 09:25 Inactive
@andysellick andysellick changed the title [DO NOT MERGE] Turn scroll tracking on for various pages Turn scroll tracking on for various pages Jan 6, 2022
@andysellick andysellick merged commit 6377ace into main Jan 6, 2022
@andysellick andysellick deleted the migrate-to-new-scrolltracker branch January 6, 2022 12:34
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