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

Add GA4 section parameter #3333

Merged
merged 1 commit into from
Jul 20, 2023
Merged

Add GA4 section parameter #3333

merged 1 commit into from
Jul 20, 2023

Conversation

JamesCGDS
Copy link
Contributor

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

Follow these steps if you are doing a Rails upgrade.

What

This PR adds the section parameter to links on second level browse pages.

Why

Requested by PA's.

Trello card

Visual changes

N/A

@govuk-ci govuk-ci temporarily deployed to collections-pr-3333 July 19, 2023 21:37 Inactive
@JamesCGDS JamesCGDS requested a review from AshGDS July 20, 2023 08:17
Copy link
Contributor

@AshGDS AshGDS left a comment

Choose a reason for hiding this comment

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

Looks good 👍 I think it requires a small change though

<% if page.related_topics.any? %>
<%
if page.related_topics.any?
heading_text = t("second_level_browse.more_on_this_topic")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you'll need to use a version of the heading_text that has it forced to use the English locale, e.g.

<% 
  if page.related_topics.any? 
  heading_text = t("second_level_browse.more_on_this_topic")
  ga4_heading_text = t("second_level_browse.more_on_this_topic", locale: :en)
%>

Copy link
Contributor Author

@JamesCGDS JamesCGDS Jul 20, 2023

Choose a reason for hiding this comment

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

Good spot, I guess there's no point in declaring a variable in that case and I could just pass them directly

@govuk-ci govuk-ci temporarily deployed to collections-pr-3333 July 20, 2023 12:44 Inactive
Copy link
Contributor

@AshGDS AshGDS left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@JamesCGDS JamesCGDS merged commit 3550d15 into main Jul 20, 2023
9 checks passed
@JamesCGDS JamesCGDS deleted the ga4_bug_fixes branch July 20, 2023 12:50
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