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

[FEATURE] Matomo - Envoyer les events liés à l'utilisation du Expand (toggle) (PIX-15774) #10889

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

dianeCdrPix
Copy link
Contributor

@dianeCdrPix dianeCdrPix commented Dec 23, 2024

🎁 Proposition

Ajouter une fonction dans Passage pour appeler le service metrics et envoyer des informations à Matomo sur le click d'un Expand

🧦 Remarques

  • Pour le futur : refacto du passage et de ses nombreuses méthodes ?
  • Utilisation de l'événement toggle comme mis dans la doc de la balise details
  • Pour les tests, nous avons dû simuler un clic sur la balise summary sinon le contenu ne s'affiche pas.

🎅 Pour tester (en local)

  • Aller sur le didacticiel Modulix
  • Afficher la console navigateur
  • Cliquer sur l'élément Expand en dessous des flashcards
  • Vérifier que l'event Matomo dans la console est bien envoyé 🎉

@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@dianeCdrPix dianeCdrPix force-pushed the pix-15774-add-matomo-event-for-expand branch 2 times, most recently from 9a2ba7a to 4c6e4c6 Compare December 23, 2024 13:44
@er-lim er-lim changed the title [FEATURE] Matomo - Envoyer events liés à l'utilisation du Expand (click) (PIX-14973)(PIX-15774) [FEATURE] Matomo - Envoyer events liés à l'utilisation du Expand (toggle) (PIX-14973)(PIX-15774) Dec 23, 2024
@er-lim er-lim changed the title [FEATURE] Matomo - Envoyer events liés à l'utilisation du Expand (toggle) (PIX-14973)(PIX-15774) [FEATURE] Matomo - Envoyer events liés à l'utilisation du Expand (toggle) (PIX-15774) Dec 23, 2024
@er-lim er-lim force-pushed the pix-15774-add-matomo-event-for-expand branch 2 times, most recently from f2b731a to 10a0407 Compare December 23, 2024 15:34
@er-lim er-lim changed the title [FEATURE] Matomo - Envoyer events liés à l'utilisation du Expand (toggle) (PIX-15774) [FEATURE] Matomo - Envoyer les events liés à l'utilisation du Expand (toggle) (PIX-15774) Dec 23, 2024
@dianeCdrPix dianeCdrPix marked this pull request as draft December 24, 2024 10:40
@dianeCdrPix dianeCdrPix marked this pull request as ready for review December 24, 2024 10:40
@clemlatz clemlatz force-pushed the pix-15774-add-matomo-event-for-expand branch from 10a0407 to cc57b25 Compare January 2, 2025 09:52
Copy link
Member

@clemlatz clemlatz left a comment

Choose a reason for hiding this comment

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

Bravo d'avoir géré les cas "Ouverture" et "Fermeture", je ne pensais pas que c'était possible 👏🏻

export default class ModulixExpand extends Component {
@action
onExpandToggle(event) {
const isOpen = event?.srcElement?.open;
Copy link
Member

Choose a reason for hiding this comment

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

question: est-ce qu'il y a un cas où event ou srcElement peuvent être undefined ?
Si oui, l'évènement envoyé à Matomo sera une fermeture, est-ce vraiment ce qu'on veut ?

suggestion: si isOpen peut être vraiment undefined, cela signifie qu'on on peut ne pas avoir l'information, donc il faudrait gérer dans un troisième cas qui ne soit ni "Ouverture" ni "Fermeture" mais "Clic".

Comment on lines +30 to +31
module('when click on Expand', function () {
test('should call method onExpandToggle', async function (assert) {
Copy link
Member

Choose a reason for hiding this comment

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

suggestion : de formulation orientée utilisateur + il faudrait tester aussi que quand on clique deux fois, ça appelle la méthode avec isOpen: false

Suggested change
module('when click on Expand', function () {
test('should call method onExpandToggle', async function (assert) {
module('when users opens Expand element', function () {
test('should call method onExpandToggle with isOpen set to true', async function (assert) {

Comment on lines +1174 to +1175
module('when user clicks on expand element', function () {
test('should push metrics event', async function (assert) {
Copy link
Member

Choose a reason for hiding this comment

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

suggestion : il faudrait tester aussi le cas où ça appelle la méthode avec "Fermeture"

Suggested change
module('when user clicks on expand element', function () {
test('should push metrics event', async function (assert) {
module('when user clicks expand element', function () {
module('when isOpen argument is true', function () {
test('should push metrics event with "Ouverture" event name', async function (assert) {
//
});
module('when isOpen argument is false', function () {
test('should push metrics event with "Fermeture" event name', async function (assert) {

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.

4 participants