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

[Personalize][Initializers] Move XP tracking API examples to separate add-on initializer #980

Merged
merged 10 commits into from
Apr 18, 2022

Conversation

illiakovalenko
Copy link
Contributor

@illiakovalenko illiakovalenko commented Apr 13, 2022

Description / Motivation

  • Added new nextjs-checkbox control to support error messages (not documented in the inquirer, dave into source code of other libs which implement controls)
  • Tracking items are extracted from the nextjs-styleguide
  • Show warning when nextjs-styleguide-tracking and nextjs-personalize are created together
  • Show warning when nextjs-styleguide-tracking is created without nextjs-styleguide
  • Tracking component added to the /tracking page
  • Added Scripts component in order to do not duplicate Layout component

Testing Details

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@illiakovalenko illiakovalenko requested a review from a team April 13, 2022 14:30
Copy link
Contributor

@ambrauer ambrauer left a comment

Choose a reason for hiding this comment

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

Looking good, see some comments.

Overall, I'm noticing we still have a lot of duplicated code in Layout.tsx accross initializers. We are duplicating to include either <VisitorIdentification/> or <CdpIntegrationScript/> as follows:

  • <VisitorIdentification/> should be added when nextjs-styleguide-tracking is included
  • <CdpIntegrationScript/> should be added when nextjs-personalize is included

Maybe we need a new abstraction here (similar to <Navigation />?) that makes these inclusions independent of Layout.tsx?

Note we should have more flexibility with in this PR.

We will also need to apply same treatment to nextjs-sxa add-on.

@illiakovalenko illiakovalenko requested a review from ambrauer April 18, 2022 09:59
Copy link
Contributor

@ambrauer ambrauer left a comment

Choose a reason for hiding this comment

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

Awesome 👍

@ambrauer ambrauer merged commit 7b1babd into dev Apr 18, 2022
@ambrauer ambrauer deleted the feature/491539 branch April 18, 2022 13:47
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.

2 participants