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

Jump to bottom

[BUGFIX] Permet le clique entre les input radio ou checkbox et leur label #607

Merged
merged 1 commit into from
May 6, 2024
Merged

[BUGFIX] Permet le clique entre les input radio ou checkbox et leur label #607

merged 1 commit into from
May 6, 2024

Conversation

yannbertrand
Copy link
Member

@yannbertrand yannbertrand commented Apr 12, 2024 •
edited
Loading

🎄 Problème

Depuis la v45, on ne peut plus cliquer entre un input "radio" ou "checkbox" et son label associé.

image(23)

🎁 Proposition

Replacer l'input à l'intérieur du label et utiliser flexbox comme avant.

🌟 Remarques

R AS

🎅 Pour tester

Vérifier qu'on peut correctement interagir avec les composants PixRadioButton et PixCheckbox.

@yannbertrand yannbertrand self-assigned this Apr 12, 2024
@pix-bot-github
Copy link

Une fois l'application déployée, elle sera accessible à cette adresse https://ui-pr607.review.pix.fr
Les variables d'environnement seront accessibles sur scalingo https://dashboard.scalingo.com/apps/osc-fr1/pix-ui-review-pr607/environment

@yannbertrand yannbertrand force-pushed the fix-label-input-click branch from aabc071 to 97ef7f9 Compare April 12, 2024 13:38
@xav-car
Copy link
Contributor

xav-car commented Apr 15, 2024

Est ce que l'ajout d'un yield input dans PixLabel serait plus "clean" . ( if has-block input ) etc... ? et gérer ça côté PixLabel ?

@yannbertrand
Copy link
Member Author

Est ce que l'ajout d'un yield input dans PixLabel serait plus "clean" . ( if has-block input ) etc... ? et gérer ça côté PixLabel ?

J'aime bien la simplicité de l'interface actuelle, j'ai peur qu'ajouter un yield ajoute une variante d'usage qu'on devra maintenir derrière.

@dlahaye
Copy link

dlahaye commented Apr 15, 2024

J'aime bien la simplicité de l'interface actuelle, j'ai peur qu'ajouter un yield ajoute une variante d'usage qu'on devra maintenir derrière.

Je comprends les deux points de vue. De mon côté même si j'ai du mal à voir les impacts (négatifs) possibles de cette PR, ça me fait un peu peur de voir le composant radio-button intégrer des usages dans le composant label sans que ce dernier n'ait été pensé pour et testé en conséquence

@yannbertrand
Copy link
Member Author

De mon côté même si j'ai du mal à voir les impacts (négatifs) possibles de cette PR

Pour moi l'impact principal c'est le lien fort entre le composant PixRadioButton et la classe pix-label qui est interne à PixLabel. Si un jour, le nom pix-label évolue, les styles risquent de casser. Mais :

  1. je ne vois pas pourquoi ce nom évoluerait,
  2. j'espère qu'on s'en rendrait compte 😬

@xav-car
Copy link
Contributor

xav-car commented Apr 16, 2024

ajouter un attribut au pixLabel @wrappedElement true|false ? qui ajoute un modifier à la classe pixLabel ?

@xav-car xav-car force-pushed the fix-label-input-click branch 2 times, most recently from 571448b to 454ff65 Compare May 2, 2024 13:15
@xav-car xav-car marked this pull request as ready for review May 2, 2024 15:01
@yannbertrand yannbertrand force-pushed the fix-label-input-click branch from 454ff65 to dc9a4b0 Compare May 3, 2024 13:33
matthiasferraina
matthiasferraina approved these changes May 3, 2024
View reviewed changes
Copy link
Contributor

@matthiasferraina matthiasferraina left a comment

Choose a reason for hiding this comment

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

Ca fonctionne bien sur storybook 👌🏻 . On verra ce que ça donnera lors de l'intégration dans pix app après la montée de version. Il faudra bien vérifier qu'il n'y a pas de regression sur les QCU et QCM.

@yannbertrand yannbertrand force-pushed the fix-label-input-click branch 2 times, most recently from 39b44fa to aa3484d Compare May 6, 2024 08:04
@yannbertrand yannbertrand force-pushed the fix-label-input-click branch from aa3484d to 82ab5ae Compare May 6, 2024 08:17
@pix-service-auto-merge pix-service-auto-merge merged commit 9bc8519 into dev May 6, 2024
9 of 11 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the fix-label-input-click branch May 6, 2024 08:20
pix-service-auto-merge pushed a commit that referenced this pull request May 6, 2024
@semantic-release-bot
chore(release): 45.4.1 [skip ci]
65c1094 
## [45.4.1](v45.4.0...v45.4.1) (2024-05-06)

### 🐛 Correction

- [#607](#607) Permet le clique entre les input `radio` ou `checkbox` et leur label
@pix-service-auto-merge
Copy link
Contributor

🎉 This PR is included in version 45.4.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xav-car xav-car xav-car left review comments

@dlahaye dlahaye dlahaye approved these changes

@matthiasferraina matthiasferraina matthiasferraina approved these changes

Assignees

@yannbertrand yannbertrand

Labels
cross-team Toutes les équipes de dev Func Review OK released 🚀 Ready to Merge Tech review OK
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@yannbertrand @pix-bot-github @xav-car @dlahaye @pix-service-auto-merge @matthiasferraina