Conversation
|
|
||
| export async function scrollGrid(options: ScrollToOptions) { | ||
| page.getGrid().element().scrollTo(options); | ||
| page.getGrid().element().scroll(options); |
There was a problem hiding this comment.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3963 +/- ##
==========================================
- Coverage 97.61% 97.54% -0.08%
==========================================
Files 36 36
Lines 1470 1467 -3
Branches 480 472 -8
==========================================
- Hits 1435 1431 -4
- Misses 35 36 +1
🚀 New features to boost your workflow:
|
|
|
||
| export const row = css` | ||
| @layer rdg.Row { | ||
| display: contents; |
There was a problem hiding this comment.
Rows are now part of the layout.
| await expect.element(headerCells).toHaveLength(0); | ||
| await expect.element(cells).toHaveLength(0); | ||
| await expect.element(rows).toHaveLength(0); | ||
| await expect.element(rows).toHaveLength(34); |
There was a problem hiding this comment.
I guess it ignored rows previously because they were empty and not part of the layout?
|
|
||
| export const row = css` | ||
| @layer rdg.Row { | ||
| display: contents; |
| if (!(target instanceof Element)) return; | ||
|
|
||
| const isCellEvent = target.closest('.rdg-cell') !== null; | ||
| const isRowEvent = isTreeGrid && target.role === 'row'; |
There was a problem hiding this comment.
Wonder if we can/should move the focus logic to the TreeGrid component. I will check
| } | ||
|
|
||
| &:focus { | ||
| outline: none; |
There was a problem hiding this comment.
Can we not set outline on the focused row?
There was a problem hiding this comment.
I set border instead as it's 1 line (no negative offset)
Had to remove the default focus style with outline: none.
The border is always set when the row has the [tabindex='0'] attribute, so this is the same behavior as before: if the position is set on a row/cell, we highlight it, regardless of actual :focus
Would you rather we use outline instead of border?
There was a problem hiding this comment.
I was wondering if pseudo elements are needed to set row focus styles. Can we not used the same strategy as cell and set outline? or border? I am okay with either
There was a problem hiding this comment.
Unfortunately it doesn't work because the cells render above the outline 🤷♂️
I'll add a comment
| & > .${cellFrozen}:first-child::before { | ||
| content: ''; | ||
| display: inline-block; | ||
| position: absolute; | ||
| inset-block: 0; | ||
| inset-inline-start: 0; | ||
| border-inline-start: var(--rdg-selection-width) solid var(--rdg-selection-color); | ||
| } |
There was a problem hiding this comment.
Why do we need this?
There was a problem hiding this comment.
To keep the blue border on the first frozen cell, otherwise when we scroll horizontally the border at the start of the row will disappear.
This was implemented as rowSelectedWithFrozenCell before (same file)
| @layer rdg.Row { | ||
| display: contents; | ||
| display: grid; | ||
| grid-column: 1/-1; |
There was a problem hiding this comment.
by default grid-row is 1?
There was a problem hiding this comment.
We set gridRowStart in JS, so we don't rely on grid-row defaults.
By default, if you don't specify grid-row I believe it just finds the first available grid row.
[fun fact] Also because we set
grid-template: subgrid / subgrid
-> grid-template-rows: subgrid;
-> the row inherits the row config from the parent grid
-> the row only spans 1 grid row, so it lays out its children in this grid row
-> cells have only 1 grid row to render into
When I tried grid-template-columns: subgrid; without grid-template-rows, there were rendering bugs sometimes:
Chrome:
Firefox:
If I try to set the same row size instead of subgrid, again focused rows bug out:
To fix this I'd have to set grid-row on all cells / focus highlight so they don't generate new grid tracks.
subgrid is easier.
|
|
||
| // collpase parent group | ||
| await userEvent.keyboard('{arrowdown}{arrowdown}{arrowleft}'); | ||
| await userEvent.keyboard('{arrowdown}{arrowdown}{arrowleft}{arrowleft}'); |
There was a problem hiding this comment.
why do we need extra {arrowleft}?
There was a problem hiding this comment.
to change selection to the row, as we don't select header rows anymore,
so header cell -> cell -> row
3 participants
Fixes #3957
Only header rows are not focusable, because it gets tricky with column grouping.
But maybe we could enable column grouping in
TreeDataGridnow 🤔