[#2130] Changed the keyboard navigation for the Accordion component t… #2198
Conversation
…onent to enter key
|
Thank you for opening this pull request with us! Be sure to follow our Pull Request Practices. Let us know if you have any questions on Slack. |
| @@ -43,14 +43,19 @@ export const Accordion = ({ | |||
| setOpen(!open); | |||
| }; | |||
|
|
|||
| const openToggle = (event: any) => { | |||
There was a problem hiding this comment.
Let's specify the type here, it should be SyntheticKeyboardEvent<HTMLDivElement>.
There was a problem hiding this comment.
@julianguyen Thanks for mentioning it.
I have updated the file.
| const inputClassNames = () => `${dark ? css.dark : ''} ${large ? css.large : ''}`; | ||
|
|
||
| return ( | ||
| <div id={`${id}_accordion`} className={inputClassNames()}> | ||
| <div | ||
| className={`accordion ${globalCss.gridRowSpaceBetween} ${css.accordion}`} | ||
| onClick={toggleOpen} | ||
| onKeyDown={toggleOpen} | ||
| onKeyDown={openToggle} |
There was a problem hiding this comment.
Let's also write a test for this in client/app/components/Accordion/__tests__/Accordion.spec.jsx! If you need help on writing this, here's a similar test for the Modal component.
There was a problem hiding this comment.
@julianguyen
I haven't written any test cases yet.
Can you please explain how to write them..
There was a problem hiding this comment.
Hi @julianguyen
I have added the test case. Please have a look now, and let me know if anything needs to be changed.
|
|
||
| const accordionContent = getByRole('region'); | ||
| const accordionBtn = getByRole('button'); | ||
| fireEvent.keyDown(accordionContent, { |
There was a problem hiding this comment.
You want to trigger the Enter key on the accordionBtn not accordionContent. After doing that, expect(accordionContent).toHaveClass('accordionContent'); should pass. expect(accordionContent).toHaveClass('accordionClose'); should only pass once the accordion is closed.
There was a problem hiding this comment.
Sorry, @julianguyen I haven't written any test cases before that's why it is taking some time. Hopefully, it's completed now.
|
Thank you for merging this pull request with us! If you haven't already, in another pull request, please add yourself to our About page. |
Description
More Details
I have changed the keyboard navigation of Accordion from TAB to ENTER
Corresponding Issue
#2130
Screenshots
Reviewing this pull request? Check out our Code Review Practices guide if you haven't already!