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

[BUGFIX] Corriger la mention "Terminé" sur la dernière flashcard (PIX-15735) #10824

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

er-lim
Copy link
Contributor

@er-lim er-lim commented Dec 16, 2024

🎄 Problème

Lors de l’affichage de la dernière flashcard, la mention “terminé” apparaît sur fond blanc 😅

Après investigation, c'est causé par l'ajout d'une propriété css color: var(--pix-neutral-0)

🎁 Proposition

Enlever cette ligne.

🧦 Remarques

Régression due à une montée en version pix-ui.
A voir pourquoi cette ligne a été ajouté précédemment 🤔

🎅 Pour tester

  • Aller sur le didacticiel modulix
  • Faire l'exercice des flashcards jusqu'à la fin 🐱
  • Vérifier que la mention Terminé n'apparaît plus sur fond blanc.

@er-lim er-lim requested a review from a team as a code owner December 16, 2024 08:05
@er-lim er-lim changed the title fix(mon-pix) correct alert color of outro card [BUGFIX] Corriger la mention "Terminé" sur la dernière flashcard (PIX-15735) Dec 16, 2024
@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 :

@yannbertrand
Copy link
Member

Régression due à une montée en version pix-ui.
A voir pourquoi cette ligne a été ajouté précédemment 🤔

Possible d'en apprendre plus ? J'ai pas trouvé en creusant côté Pix UI :(

@er-lim
Copy link
Contributor Author

er-lim commented Dec 16, 2024

Possible d'en apprendre plus ? J'ai pas trouvé en creusant côté Pix UI :(

@yannbertrand Oui pour les détails :

  • La PR pix-ui qui fait la montée en version en v.51.0.0 change la manière d'écrire le style .pix-notification-alert--success sur cette modification
  • Ceci faisait que même si tu surcharges le style en BEM comme on le faisait sur les flashcards, ça ne faisait rien (voir capture d'écran)
Capture d’écran 2024-12-16 à 09 24 45
  • Avec le changement en v51.0.0, color est surchargé avec ce qu'on fait sur les flashcards, ce qui cause cette régression 😜

Reste à comprendre pourquoi on avait mis cette propriété 🤔

@yannbertrand
Copy link
Member

Possible d'en apprendre plus ? J'ai pas trouvé en creusant côté Pix UI :(

@yannbertrand Oui pour les détails :

* La PR pix-ui qui fait la montée en version en v.51.0.0 change la manière d'écrire le style `.pix-notification-alert--success` sur cette [modification](https://github.com/1024pix/pix-ui/pull/786/files#diff-8a29f724b94fc5eb568d0cb3f04ae758d6b919570ec61c420e42cf8af1d85ef0R28)

* Ceci faisait que même si tu surcharges le style en BEM comme on le faisait sur les flashcards, ça ne faisait rien (voir capture d'écran)
Capture d’écran 2024-12-16 à 09 24 45
* Avec le changement en v51.0.0, `color` est surchargé avec ce qu'on fait sur les flashcards, ce qui cause cette régression 😜

Reste à comprendre pourquoi on avait mis cette propriété 🤔

🤯 Est-ce que pour éviter les soucis dans le futur on ne ferait pas carrément notre propre composant ? J'ai l'impression qu'on est pas vraiment sur le type de notification de Pix UI : https://ui.pix.fr/?path=/docs/feedback-notification-alert--docs

@er-lim er-lim self-assigned this Dec 16, 2024
@er-lim
Copy link
Contributor Author

er-lim commented Dec 16, 2024

🤯 Est-ce que pour éviter les soucis dans le futur on ne ferait pas carrément notre propre composant ? J'ai l'impression qu'on est pas vraiment sur le type de notification de Pix UI : https://ui.pix.fr/?path=/docs/feedback-notification-alert--docs

En effet on est peut être plus proche d'un tag dessus. A creuser pour voir s'il n'y a pas le même besoin dans les autres équipes 👍

@pix-service-auto-merge pix-service-auto-merge force-pushed the pix-15735-fix-outro-card-alert-color branch from 0e25780 to c702339 Compare December 17, 2024 08:06
@pix-service-auto-merge pix-service-auto-merge merged commit 6c9a5f0 into dev Dec 17, 2024
7 of 8 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the pix-15735-fix-outro-card-alert-color branch December 17, 2024 08:11
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.

5 participants