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

Fix 725 Enter key now submits passwordless login form. #726

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

millette
Copy link
Contributor

@millette millette commented Oct 3, 2023

No description provided.

Copy link
Collaborator

@tahini tahini left a comment

Choose a reason for hiding this comment

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

Est-ce que yarn format a été roulé dans ce package? J'ai l'impression que le return devrait être sur l'autre ligne.

@kaligrafy peux-tu reviewer l'événement? Je ne suis pas experte en la matière, mais pourquoi keyDown ici alors que sur le bouton c'est keyUp et l'autre LoginForm, c'est keyPressed? On s'en fout ou c'est important?

@kaligrafy
Copy link
Collaborator

keyDown, tu ne peux pas annuler une fois la touche appuyée. KeyUp, tu peux annuler, car tant que la touche n'Est pas lâchée, en principe tu peux trouver le moyen d'annuler avant le keyup.

@kaligrafy
Copy link
Collaborator

donc si possible remplacer par onKeyUp

@kaligrafy
Copy link
Collaborator

@millette
Copy link
Contributor Author

millette commented Oct 5, 2023

À propos de keyUp/Down, je ne comprends pas le key event sur le Button:

https://github.com/millette/transition/blob/main/packages/chaire-lib-frontend/src/components/forms/auth/passwordless/PwdLessLoginForm.tsx#L110-L114

@tahini J'ai reformatté, j'oublie souvent les «;» (voir https://standardjs.com/rules.html#semicolons, pas que je recommande de laisser tomber les «;». Pour le return, j'aime bien le principe de return early au lieu d'avoir du code de plus en plus indenté.

@kaligrafy
Copy link
Collaborator

À propos de keyUp/Down, je ne comprends pas le key event sur le Button:

https://github.com/millette/transition/blob/main/packages/chaire-lib-frontend/src/components/forms/auth/passwordless/PwdLessLoginForm.tsx#L110-L114

Ça "clique" sur le boutton si quelqu'un utilise un lecteur d'écran ou navigue avec un clavier (accessibilité). C'est standard d'activer un bouton avec enter ou l'espace

@millette
Copy link
Contributor Author

millette commented Oct 6, 2023

@kaligrafy Oui, d'accord. J'ai toujours vu ça comme un feature du browser, jamais avec du JS. Alors j'ai laissé tel quel, mais j'étais vraiment curieux.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants