[FEATURE] Ajouter un composant PixOverlay (PIX-15734). #802
Conversation
Jeyffrey
added
🚧 Development in progress
|
Une fois l'application déployée, elle sera accessible à cette adresse https://ui-pr802.review.pix.fr |
Jeyffrey
force-pushed
the
pix-15734-new-pix-overlay-component
branch
3 times, most recently
from
c0635a0 to
23c943f
Compare
Jeyffrey
force-pushed
the
pix-15734-new-pix-overlay-component
branch
5 times, most recently
from
efcef7f to
4984d68
Compare
Jeyffrey
force-pushed
the
pix-15734-new-pix-overlay-component
branch
from
4984d68 to
8970ba7
Compare
Jeyffrey
added
👀 Func Review Needed
👀 Tech Review Needed
and removed
🚧 Development in progress
| z-index: 1000; | ||
| inset: 0; | ||
| overflow-y: auto; | ||
| background-color: rgba(var(--pix-primary-700-inline), 0.3); |
There was a problem hiding this comment.
Question : ce background-color primary sera commun aux différentes applications ou y'a des variantes prévues ?
There was a problem hiding this comment.
| import { Meta, Story, ArgTypes } from '@storybook/blocks'; | ||
| import * as ComponentStories from './pix-overlay.stories'; | ||
|
|
||
| <Meta of={ComponentStories} /> |
There was a problem hiding this comment.
Question : Si ce composant à une doc à part entière on pourrait croire qu'il peut s'utiliser seul.
Est-ce qu'on ne préciserait pas qu'il ne peut être utilisé seul mais qu'il existe pour mutualiser l'overlay entre nos différents composants qui en font usage ?
There was a problem hiding this comment.
Est-ce que c'est pas déjà le cas pour le composant PixBlock par exemple ?
| assert.contains("It's a modal!"); | ||
| assert.contains('content'); | ||
| assert.contains('footer'); | ||
| assert.dom('.pix-modal__overlay--hidden').doesNotExist(); |
There was a problem hiding this comment.
question
Est-ce que l'on souhaite tester une variable css ?
suggestion
Utiliser testing library pour tester le texte ?
| @@ -94,7 +94,7 @@ module('Integration | Component | modal', function (hooks) { | |||
| </PixModal>`); | |||
|
|
|||
| // then | |||
| assert.dom('.pix-modal__overlay--hidden').exists(); | |||
| assert.dom('.pix-overlay--hidden').exists(); | |||
There was a problem hiding this comment.
suggestionù
utiliser testing library pour tester la non présence du texte, plutôt que la présence de l'overlay caché ?
There was a problem hiding this comment.
En fait l'overlay n'a rien d'assez spécifique pour vérifier sa présence dans les tests.
Le texte est dans tous les cas toujours présent sur la page.
Donc je vois pas d'autre moyen de tester la présence / absence de l'overlay que par sa classe CSS :/
There was a problem hiding this comment.
l'overlay permet de gérer la visibilité de la modal aussi non avec le cas visible / hidden ?
Du coup tester que le texte n'est pas présent en getByText ( pas accessible suffit à dire que le comportement que l'on souhaite est là ) .
Tester la class dans le component overlay oui pourquoi pas. mais à mon sens le test de la pix-modal n'a pas à avoir cette connaissance 🤔
There was a problem hiding this comment.
Je crois me souvenir pourquoi on a fait ça : le getByText fonctionnerait même lorsque la modale est non visible parce qu'on a toujours la modale dans le DOM.
Elle est juste cachée ou non.
Et qunit ne gère pas ce cas il me semble.
There was a problem hiding this comment.
screen.getByText() si c'est pas accessible (bien que présent dans le dom), il ne devrait pas le lire.
C'est effectivement le assert.dom qui ne fait pas de distinction entre visible ou non dans la page.
🎄 Problème
Les overlay (modal, sidepanel, etc.) ne sont pas uniforme d’une app à l’autre.
🎁 Proposition
Création d’un composant PixOverlay, utilisé dans les composant Modal et Sidebar.
🎅 Pour tester
Tester les différents composants.