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(MenuButton): remove auto-focus from first menu item (ABF-6488) #492

Merged
merged 3 commits into from
Jun 29, 2023

Conversation

samuelguebo
Copy link
Contributor

@samuelguebo samuelguebo commented May 3, 2023

Remove auto-focus from first menu item at the DS-level, as requested on ABF-6488.

Bug fix checklist

  • Units tests have been adjusted to account for bug.
  • The fix has been tested in multiple Storybook stories.
  • All GitHub checks are successful.

New component checklist

  • The new component and its tests are in the same component folder.
  • The component is unit tested and/or snapshot tested.
  • All of the relevant Storybook stories have been added to the storybook package.
  • There are no linting errors or warnings in the modified/new code.
  • All GitHub checks are successful.

@samuelguebo samuelguebo requested a review from a team as a code owner May 3, 2023 15:20
Copy link
Contributor

@JsGarneau JsGarneau left a comment

Choose a reason for hiding this comment

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

Garder le focus sur le bouton c'est bien, par contre on voudrait pouvoir ensuite naviguer le menu à l'aide de "Tabs" par la suite.

@meriouma
Copy link
Contributor

meriouma commented May 4, 2023

Et dans la doc il y a cette mention, il faudrait probablement garder ce comportement :

Users can expand the menu by either pressing the enter or the space key when focussed or by clicking on the button. The focus will then move to the first option (if the users uses the keyboard).

Copy link
Contributor

@JsGarneau JsGarneau left a comment

Choose a reason for hiding this comment

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

Tu peux regarder le comportement du Bento et/ou du User Profile, ces components on les bons comportements

Copy link
Contributor

@JsGarneau JsGarneau left a comment

Choose a reason for hiding this comment

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

Lorsqu'on ouvre le menu avec le clavier, si j'utilise Tab pour changer d'élément, je me retrouve sur le prochain bouton, je crois que c'est OK, par contre le menu ne se ferme pas.

@LarryMatte Je remarque que si j'ouvre le menu avec un Click et que je Tab ensuite, le focus ne se dirige pas dans le menu, mais sur le bouton à coté. Est-ce que c'est le comportement souhaité? Je me serais attendu à rentrer dans le menu et pouvoir ensuite naviguer avec les flèches.

@LarryMatte
Copy link
Contributor

@JsGarneau

@LarryMatte Je remarque que si j'ouvre le menu avec un Click et que je Tab ensuite, le focus ne se dirige pas dans le menu, mais sur le bouton à coté. Est-ce que c'est le comportement souhaité? Je me serais attendu à rentrer dans le menu et pouvoir ensuite naviguer avec les flèches.

On va changer ça, si le focus est sur le bouton et que l'utilisateur press enter ou spacebar OU s'il clique sur le bouton pour expand le listbox, le visual focus va tomber sur la 1ere option dans le listbox. Si l'utilisateur fait tab, il passe au prochain élément focusable dans la page et le menu se collapse.

@samuelguebo samuelguebo force-pushed the dev/ABF-6488 branch 2 times, most recently from 8dc0d8a to 5158493 Compare June 7, 2023 18:45
@samuelguebo
Copy link
Contributor Author

@JsGarneau

On va changer ça, si le focus est sur le bouton et que l'utilisateur press enter ou spacebar OU s'il clique sur le bouton pour > expand le listbox, le visual focus va tomber sur la 1ere option dans le listbox. Si l'utilisateur fait tab, il passe au prochain > élément focusable dans la page et le menu se collapse.

C'est mis a jour avec les comportements suivants:

  • Si l'utilisateur fait tab, il passe au prochain élément focusable dans la page et le menu collapse
  • Si l'utilisateur press enter ou spacebar le visual focus va tomber sur la 1ere option dans le listbox.
  • Si l'utilisateur clique sur le bouton pour expand le listbox, le visual focus va tomber sur la 1ere option dans le listbox.

@samuelguebo samuelguebo changed the title fix(MenuButton): Remove auto-focus from first menu item (ABF-6488) fix(MenuButton): remove auto-focus from first menu item (ABF-6488) Jun 8, 2023
JsGarneau
JsGarneau previously approved these changes Jun 8, 2023
Copy link
Contributor

@JsGarneau JsGarneau left a comment

Choose a reason for hiding this comment

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

Bon, tout fonctionne selon les commentaires de @LarryMatte , par contre, c'est contre les critères d'acceptation de la carte, on voulait justement retirer le focus visuel sur le premier élément lorsqu'on click sur le boutton.

Il faudrait potentiellement parler avec Kevin pour que ca soit clair que le comportement actuel est ce qu'on désire.

@samuelguebo
Copy link
Contributor Author

samuelguebo commented Jun 8, 2023

Il faudrait potentiellement parler avec Kevin pour que ca soit clair que le comportement actuel est ce qu'on désire.

Hello @kdoucet-kronos, on a besoin de toi pour trancher. Sur la base de ce que Larry a recommandé voici les comportements qu'on a en ce moment:

  • Si l'utilisateur press enter ou spacebar le focus passe sur le 1er element de la liste.
  • Si l'utilisateur clique sur le bouton, le focus passe sur le 1er element de la liste.

Est-ce qu'on garde le focus sur le premier element lorsqu'on clique?

@kdoucet-kronos
Copy link

@samuelguebo
J'aimerais que l'on conserve le comportement que nous avons avec le top menu fait en DS et approuvé.

C'est à dire que
--> si la navigation se fait par clavier, le 1er élément de la liste a le visual focus.
--> si la navigation se fait par clique de souris, le 1er élément n'a pas l'encadré du visual focus.

Exemple : Bento box dans le top menu.

Je penche vers ce choix sur la base que nous avons déjà un comportement modifié pour le top menu que ce que @LarryMatte propose, que c'est visuellement plus plaisant et évite possiblement une incompréhension de l'utilisation lorsqu'il y a 2 choix dans le menu mais le premier est encadré (pourrait insinuer que nous recommandons ce choix ou qu'il doit maintenant cliquer là).

@samuelguebo
Copy link
Contributor Author

Hey @JsGarneau et @kdoucet-kronos, c'est mis a jour avec les comportements suivants:

  • Si l'utilisateur clique sur le bouton pour expand le listbox, le 1er élément du menu n'a pas l'encadré du visual focus.
  • Si l'utilisateur press enter ou spacebar le visual focus va tomber sur la 1ere option dans le listbox.
  • Si l'utilisateur fait tab, il passe au prochain élément focusable dans la page et le menu collapse

@JsGarneau JsGarneau merged commit ebffdc7 into master Jun 29, 2023
20 checks passed
@JsGarneau JsGarneau deleted the dev/ABF-6488 branch June 29, 2023 12:34
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.

None yet

5 participants