[test] Use describeTreeView for keyboard navigation tests on disabled items
#13184
Conversation
| @@ -32,7 +32,27 @@ export const getPreviousNavigableItem = ( | |||
| return itemMeta.parentId; | |||
| } | |||
|
|
|||
| let currentItemId: string = siblings[itemIndex - 1]; | |||
| // Finds the previous navigable sibling. | |||
There was a problem hiding this comment.
If the previous sibling was disabled it was all broken
There was a problem hiding this comment.
Nice fix. 👍
These are situations where you appreciate the effort put into writing tests. 👏 🎉
flaviendelangle
added
test
core
|
Deploy preview: https://deploy-preview-13184--material-ui-x.netlify.app/ |
| @@ -22,7 +22,7 @@ const getLastNavigableItemInArray = ( | |||
| export const getPreviousNavigableItem = ( | |||
There was a problem hiding this comment.
Side note: Would be awesome if we could strongly battle test those methods to avoid bugs
Because we probably don't want 30 tests on ArrowDown, 30 tests on ArrowUp etc... to check edge-cases of this method.
| @@ -32,7 +32,27 @@ export const getPreviousNavigableItem = ( | |||
| return itemMeta.parentId; | |||
| } | |||
|
|
|||
| let currentItemId: string = siblings[itemIndex - 1]; | |||
| // Finds the previous navigable sibling. | |||
There was a problem hiding this comment.
Nice fix. 👍
These are situations where you appreciate the effort put into writing tests. 👏 🎉
Could you explain what you mean by abstracted here? |
I meant that it seems like a lot of those tests are doing almost the same thing, testing what happens when you click ArrowUp or ArrowDown. |
|
Oh you meant maybe something like we do on the fields with a I propose we stick with the raw tests like today, but instead of adding dozens of new ones to handle edge cases, we test the utility methods, and those tests will be shorter. |
Yes, that's what I had in mind. 👌
Completely agree on this one, especially for now. 👍 |
Part of #12433
I added a few tests and did a bug fix