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

feat: multiple & configureSecond #31

Closed

Conversation

reslear
Copy link

@reslear reslear commented Nov 7, 2023

@johannschopplich you need to do a review and apply eslint formatting since github.dev doesn't support it :)

πŸ”— Linked issue

closes #30

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme or JSDoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

sources:

Copy link

netlify bot commented Nov 7, 2023

βœ… Deploy Preview for nuxt-gtag ready!

Name Link
πŸ”¨ Latest commit 713a055
πŸ” Latest deploy log https://app.netlify.com/sites/nuxt-gtag/deploys/654aaa32b57e610008bf517f
😎 Deploy Preview https://deploy-preview-31--nuxt-gtag.netlify.app
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@johannschopplich
Copy link
Owner

I will probably refactor the API a bit, but thanks for the effort nonetheless!

@johannschopplich
Copy link
Owner

@reslear Thanks a lot for the PR, it was inspiring. I implemented it a bit different tho in aedc6e4.

@reslear
Copy link
Author

reslear commented Feb 9, 2024

@johannschopplich
Honestly, I'm disappointed. I'm sorry:

  • you've over-complicated the code :))))
  • and forgot to dynamically add grantConsent to the ids array

@johannschopplich
Copy link
Owner

johannschopplich commented Feb 9, 2024

Feel free to iterate once the next version is released and open a PR with simplifications and fixes. πŸ™Œ

@johannschopplich
Copy link
Owner

@reslear I've released v1.2.0. Thanks for the hint on the init for multiple IDs. Would you mind doing a review on the latest code changes? Thanks in advance. πŸ™‚

@reslear
Copy link
Author

reslear commented Feb 9, 2024

@johannschopplich Well I looked it up) looks good, but maybe I missed something? how implement analogue https://github.com/johannschopplich/nuxt-gtag/pull/31/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R253 ?

case:
We have a page displaying both my and customer IDs from pageData. The ID for "my" is set in the Nuxt config, while the customer ID requires setting an additional customer gtag ID on the /event/:id page after retrieving it with asyncData.

@johannschopplich
Copy link
Owner

johannschopplich commented Feb 9, 2024

@reslear Does the new module options beforehand meet your requirements? This is possible now and doesn't require a call like configureSecond at runtime:

// `nuxt.config.ts`
export default defineNuxtConfig({
  modules: ['nuxt-gtag'],

  gtag: {
    tags: [
      {
        id: 'GT-XXXXXXXXXX',
        config: {
          page_title: 'GA4'
        }
      },
      {
        id: 'DC-ZZZZZZZZZZ',
        config: {
          page_title: 'Floodlight'
        }
      }
    ]
  }
})

@reslear
Copy link
Author

reslear commented Feb 12, 2024

@johannschopplich
Copy link
Owner

Yep, I've built Nuxt API Party based on it.

Your function configureSecond uses gtag under the hood. Can't you create a wrapper around it in your project? I don't get what's missing in my implementation.

If you think there's something missing, please open an issue and gladly open a new PR with your ideas. Thanks!

@reslear
Copy link
Author

reslear commented Feb 12, 2024

yeah, no problem, I was just suggesting using Sugar's syntax. Thanks!

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.

Multiple tracking IDs
2 participants