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

592 assistant add keyboard navigation control #622

Merged
merged 5 commits into from
May 31, 2024
Merged

592 assistant add keyboard navigation control #622

merged 5 commits into from
May 31, 2024

Conversation

stanlee974
Copy link
Member

No description provided.

@stanlee974 stanlee974 linked an issue May 13, 2024 that may be closed by this pull request
[assistant] add keyboard navigation control #592
Closed
@luifr10 luifr10 force-pushed the 592-assistant-add-keyboard-navigation-control branch from c7c01da to 5efabb8 Compare May 21, 2024 14:47
@stanlee974 stanlee974 force-pushed the 592-assistant-add-keyboard-navigation-control branch 4 times, most recently from a5e740d to 9f5dee1 Compare May 26, 2024 04:15
@luifr10 luifr10 force-pushed the 592-assistant-add-keyboard-navigation-control branch 6 times, most recently from 7da75a5 to 8309fe7 Compare May 30, 2024 15:52
@luifr10 luifr10 force-pushed the 592-assistant-add-keyboard-navigation-control branch from 8309fe7 to a1bb8ab Compare May 30, 2024 22:40
@luifr10 luifr10 force-pushed the 592-assistant-add-keyboard-navigation-control branch 3 times, most recently from b8c03b5 to ff0b848 Compare May 31, 2024 05:34
stanlee974
stanlee974 commented May 31, 2024
View reviewed changes
Copy link
Member Author

@stanlee974 stanlee974 left a comment

Choose a reason for hiding this comment

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

Top comme modifs, ça en faisait pas mal. Je suis pas fan des méthodes qui retournent pleins d'objets différents. tu as dû faire des extract d'IDE pour plus de lisibilité, mais les méthodes sont pas oufs au niveau des paramètres passés et des retours, lol si on veut clarifier, il faudra adapter l'algo. On le fera plus tard.

packages/runner-cypress/e2e/en-keyboard-navigation.feature Show resolved Hide resolved
packages/runner-playwright/e2e/en-keyboard-navigation.feature Outdated Show resolved Hide resolved
packages/docs/docs/03-recommendations/02-improving-accessibility.md Outdated Show resolved Hide resolved
@luifr10
Copy link
Member

luifr10 commented May 31, 2024

Top comme modifs, ça en faisait pas mal. Je suis pas fan des méthodes qui retournent pleins d'objets différents. tu as dû faire des extract d'IDE pour plus de lisibilité, mais les méthodes sont pas oufs au niveau des paramètres passés et des retours, lol si on veut clarifier, il faudra adapter l'algo. On le fera plus tard.

En effet, là je suis allé au plus simple en faisant un extract avec l'ide mais je pense que certains champs retournés ne sont même pas utiles, je vais vois comment refacto simplement ca

@luifr10 luifr10 force-pushed the 592-assistant-add-keyboard-navigation-control branch from ff0b848 to 0f3a3ab Compare May 31, 2024 14:57
@luifr10 luifr10 merged commit 91c2665 into main May 31, 2024
24 checks passed
@luifr10 luifr10 deleted the 592-assistant-add-keyboard-navigation-control branch May 31, 2024 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luifr10 luifr10 luifr10 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[assistant] add keyboard navigation control
2 participants
@stanlee974 @luifr10