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] Redonner toute la hauteur au textarea de la preview de modules (PIX-12468) #8881

Closed
wants to merge 1 commit into from

Conversation

dlahaye
Copy link
Contributor

@dlahaye dlahaye commented May 10, 2024

🦄 Problème

Depuis la version 45.0.0 de Pix UI, la zone de texte de la prévisualisation de modules ne prend plus toute la hauteur disponible.

🤖 Proposition

Cela est dû au fait que dans un des commits de cette version 45.0.0, une div intermédiaire a été ajoutée dans le composant PixTextarea de Pix UI, ce qui casse le style CSS de notre page.
Étant donné que notre feuille de style brisait déjà le 4ème mur de Pix UI en ciblant la div interne .pix-textarea, je me suis permis d'y rajouter la nouvelle div introduite dans le commit cité ci-dessus.

🌈 Remarques

Cette proposition de fix est loin d'être idéale car elle renforce d'autant plus la dépendance entre notre feuille de style et celle du composant venu de Pix UI.
Il faudra surement en re-discuter.

💯 Pour tester

Vérifier sur la review app que la zone de texte a bien repris toute la hauteur.

@dlahaye dlahaye self-assigned this May 10, 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 :

@@ -19,6 +19,10 @@

.pix-textarea {
height: 100%;

div {
Copy link
Member

Choose a reason for hiding this comment

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

Moui comme tu le dis, pas à l'aise avec ça. Mais d'ailleurs même .pix-textarea c'est pas tip top. Tu as réfléchi à quelque chose côté Pix UI pour permettre ça mais de manière propre ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, faudrait creuser pour comprendre l'usage de cette nouvelle div déjà.

Copy link
Member

Choose a reason for hiding this comment

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

On s'est dit avec @reibecca ça pourrait valoir le coup de tester l'attribut rows en "hotfix" avant de voir si on veut gérer cette complexité supplémentaire côté Pix UI

@yannbertrand yannbertrand marked this pull request as draft May 14, 2024 07:53
@yannbertrand
Copy link
Member

Discuté ensemble : on reprend tranquillement pour proposer un patch pérène

@dlahaye dlahaye closed this May 17, 2024
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.

None yet

3 participants