[EuiDataGrid] Add remaining unit tests for focus logic #5698
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
+ remove unnecessary role=gridcell check - after inspecting our DOM, that role attr is set on the topmost cell wrapper but `data-datagrid-cellcontent` is set on a div below that, so there's no way the parent cell should be inside the cell content + swap dataset check for hasAttribute - for some reason Jest/JSDOM was failing to register dataset, but hasAttribute works and should be equivalent
+ change unmount_enzyme helper to run after each file instead of each test, to allow onFocusUpdate tests to retain shared state between each it() block
|
Opening this in draft mode to kick off CI and see if the |
cee-chen
commented
Mar 7, 2022
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5698/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5698/ |
cee-chen
commented
Mar 14, 2022
src/components/datagrid/body/header/data_grid_header_cell_wrapper.test.tsx
Outdated
Show resolved
Hide resolved
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5698/ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
We had a
utils/focus.test.tsfile, but it only had tests forgetParentCellContent. This PR writes unit tests for all the remaining exported utilities in the file.Before
After
Note that the missing lines/functions are the empty arrow function initialized as placeholders in DataGridFocusContext:
I couldn't be bothered to write a suite of tests just to initialize focus context and call noop fns 🤷♀️
unmount_enzyme.js side effect
8b0baa3#diff-fe768e7e63543f0b9a2ef9d1c41b9248f87d76d04d0d9fa0a6dcd341697d6883 / 7bd108e
We had this conversation mostly in Slack, so I'm mentioning it here in the main GitHub issue for completeness. I wanted to avoid automatically unmounting components (i.e. preserving state) between each
it()test, to build upon state between tests. TheafterEach->afterAllchange allows me to do so with minimal impact on memory leakage.Checklist
No changelog; should be dev-only changes