Add and migrate all tests to use new locators#3954
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3954 +/- ##
==========================================
- Coverage 97.61% 97.61% -0.01%
==========================================
Files 36 36
Lines 1470 1469 -1
Branches 477 480 +3
==========================================
- Hits 1435 1434 -1
Misses 35 35
🚀 New features to boost your workflow:
|
nstepien
force-pushed
the
new-locators
branch
from
fcb5795 to
ca37953
Compare
| const scrollToRowIdx = | ||
| rowIdx !== undefined && isRowIdxWithinViewportBounds(rowIdx) ? rowIdx : undefined; | ||
| rowIdx !== undefined && isRowIdxWithinViewportBounds(rowIdx) | ||
| ? rowIdx + headerAndTopSummaryRowsCount |
There was a problem hiding this comment.
Looks like we forgot to take into account headerAndTopSummaryRowsCount here
|
|
||
| const observer = new IntersectionObserver(removeScrollToCell, { | ||
| root: gridRef.current!, | ||
| threshold: 1.0 |
There was a problem hiding this comment.
This didn't work well when targeting a column/row only, as the target <div> would be taller/wider than the grid, so the intersection ratio would stay below 0.
In such cases, the <div> might not get removed, ScrollToCell would still render, and the useLayoutEffect above would keep running on re-renders.
|
|
||
| useLayoutEffect(() => { | ||
| function removeScrollToCell() { | ||
| if (grid.scrollLeft === scrollLeft && grid.scrollTop === scrollTop) { |
There was a problem hiding this comment.
If scrolling is 'smooth', the scroll position won't immediately change, so this condition will be immediately true.
We can look into it further if it's causing issues.
| await expect.element(cell1).toHaveTextContent('col1'); | ||
| await expect.element(cell2).toHaveTextContent('col3'); | ||
| await expect.element(cell3).toHaveTextContent('col2'); | ||
| await expect.element(cell4).toHaveTextContent('col4'); |
There was a problem hiding this comment.
Better than testing the text content of the whole header row
| function testSelection(rowIdx: number, isSelected: boolean) { | ||
| return expect | ||
| .element(getRowByText(rowIdx)) | ||
| .element(rows.nth(rowIdx)) |
There was a problem hiding this comment.
Not super safe if we start scrolling, as the row index may not match with the rendered rows because of virtualization, but it works in this test suite so 🤷♂️
| '@typescript-eslint/parameter-properties': 1, | ||
| '@typescript-eslint/prefer-as-const': 1, | ||
| '@typescript-eslint/prefer-destructuring': [1, { array: false }], | ||
| '@typescript-eslint/prefer-destructuring': [1, { array: false, object: true }], |
There was a problem hiding this comment.
I don't know why, but it seems like we need to specify the default again if we only configure a subset of the options
| cleanDir: true | ||
| }, | ||
| platform: 'browser', | ||
| platform: 'neutral', |
There was a problem hiding this comment.
To support both SSR and browser rendering.
No change in the build output though, but just to be safe.
| "module": "NodeNext", | ||
| "moduleResolution": "NodeNext", | ||
| "skipLibCheck": true | ||
| "moduleResolution": "NodeNext" |
There was a problem hiding this comment.
Restored alphabetical order
| // TODO: remove when `userEvent.pointer` is supported | ||
| const resizeColumn: BrowserCommand<[name: string, resizeBy: number | readonly number[]]> = async ( | ||
| context, | ||
| { page, iframe }, |
There was a problem hiding this comment.
https://vitest.dev/api/browser/commands.html#custom-playwright-commands
If you need to query an element, you should prefer using
context.iframeinstead because it is more stable and faster.
| await page.mouse.up(); | ||
| }; | ||
|
|
||
| const scrollGrid: BrowserCommand<[{ scrollLeft?: number; scrollTop?: number }]> = async ( |
There was a problem hiding this comment.
Removed since the scrollGrid is enough.
2 participants
Enjoy.
Might have other tweaks here and there.