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.

Sign up for GitHub

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

Améliorations posthog #1229

Merged
merged 2 commits into from
Jan 21, 2025
Merged

Améliorations posthog #1229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6383ca3
Améliorations posthog
fabienheureux Jan 20, 2025
eaddb51
Merge branch 'main' into correctifs-posthog-et-script-d-integration
fabienheureux Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion static/to_compile/entrypoints/assistant/script-to-iframe.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import { iframeResize } from "iframe-resizer";
const script = document.currentScript as HTMLScriptElement
const slug = script?.dataset?.objet;
const origin = new URL(script?.getAttribute("src")).origin
const src = `${origin}/${slug || ''}?iframe`;
// TODO: mise en prod, remplacer dechet ci-dessous par une string vide
const src = `${origin}/${slug || 'dechet'}?iframe`;
const iframe = document.createElement("iframe");

const iframeAttributes = {
Expand Down
7 changes: 6 additions & 1 deletion static/to_compile/js/controllers/assistant/analytics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ export default class extends Controller<HTMLElement> {
}

#checkIfWeAreInAnIframe() {
// The iframe URL parameter is not always set :
// - After a navigaation (click on a link)
// - If the iframe integration does not use our script
// In all these cases, we still want to determine
// whether the user browse inside an iframe or not.
Comment on lines +81 to +85
Copy link
Contributor

Choose a reason for hiding this comment

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

  • mettre le commentaire en français
  • et je sais toujours pas pourquoi on fait tout cela 😄 : est-ce que la raison et la suivante?
    • par défaut on a déjà un code analytics qui tourne sur notre site
    • mais quiconque le souhaite peut intégrer notre assistant par iframe dans leur site
    • donc pour éviter les doublons analytics, on n'insère l'analytics dans l'assistant que si il est chargé via iframe

Copy link
Member Author

Choose a reason for hiding this comment

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

c'est un paramètre utilisé côté métier pour les stats.
À mon avis c'est plutôt quelque chose qui devrait vivre dans de la doc tracking (dans la page de doc que je vais écrire après la MEP), voire doc métier

try {
if (window.self !== window.top) {
this.personProperties.iframe = true
Expand All @@ -91,7 +96,7 @@ export default class extends Controller<HTMLElement> {
this.personProperties.iframe = true
}

if (document.referrer) {
if (document.referrer && !document.referrer.includes(document.location.origin)) {
fabienheureux marked this conversation as resolved.
Show resolved Hide resolved
this.personProperties.iframe = true
}
}
Expand Down
Loading