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

Fix - Refactor attribute id to data selector to fix duplicate ids #11866

Merged
merged 4 commits into from
Feb 29, 2024

Conversation

Morsey187
Copy link
Contributor

@Morsey187 Morsey187 commented Feb 14, 2024

Description

Link to sample test page: https://foundation.mozilla.org/en/privacynotincluded/
Related PRs/issues: #11605

Swaps use of ID attributes for data attributes on the donate button to allow multiple instances to fix html validation errors of Duplicate IDs. can test localhost:8000/en/privacynotincluded/ with a nu html validator to verify.

Note: This also fixes a bug I noticed where donation button clicks were only being tracked for mobile (i.e. no clicks on the donation button in the desktop navigation bar are currently being tracked) as the querySelector in header-donate-button.js queries for one element. This might be worth forwarding to someone in your SEO team to make sure it doesn’t skew or cause confusion with any active tracking, campaigns etc.

Checklist

Tests

  • Is the code I'm adding covered by tests?

Changes in Models:

  • Did I update or add new fake data?
  • Did I squash my migration?
  • Are my changes backward-compatible. If not, did I schedule a deploy with the rest of the team?

Documentation:

  • Is my code documented?
  • Did I update the READMEs or wagtail documentation?

Merge Method
💡❗Remember to use squash merge when merging non-feature branches into main

┆Issue is synchronized with this Jira Story

@mmmavis
Copy link
Collaborator

mmmavis commented Feb 16, 2024

I'm gonna keep this PR on for now as I need to talk to the team to see if #donate-header-btn is used for our GTM. If we do, then there will be extra step on getting this merged and deployed to prod.

@mmmavis
Copy link
Collaborator

mmmavis commented Feb 28, 2024

Hey @nancyt1 , can I get some help here? I need to find out if the id #donate-header-btn is being used on GTM and wonder what the best way is to do so? I went to Tag Manager's Trigger section and searched for donate to see if anything is using that id. Is this how you normally look things up?

@nancyt1
Copy link
Collaborator

nancyt1 commented Feb 29, 2024

@mmmavis Yes the trigger section is where i would look and I double checked in the tags section too but I didn't find anything explicitly using that ID.

@mmmavis
Copy link
Collaborator

mmmavis commented Feb 29, 2024

@nancyt1 Thank you for checking!

Copy link
Collaborator

@mmmavis mmmavis left a comment

Choose a reason for hiding this comment

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

PR looks good to me. Thank you!

@mmmavis mmmavis merged commit 8d3da0b into main Feb 29, 2024
6 checks passed
@mmmavis mmmavis deleted the fix/11605-duplicate-ids-donate-header-btn-PNI branch February 29, 2024 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants