Skip to content

Commit f444d56

Browse files
cee-chencee-chen
committed
[PR feedback] Separate cell row/column data into individual data-attrs
+ update query selectors and Cypress tests to use new dat attrs + deprecate `data-gridcell-id`
1 parent d218634 commit f444d56

11 files changed

Lines changed: 292 additions & 42 deletions

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
- Updated the outline color in `euiCustomControlFocused` mixin to use `$euiFocusRingColor` instead of `currentColor` ([#5479](https://github.com/elastic/eui/pull/5479))
66
- Fixed keyboard navigation in `EuiDataGrid` not fully scrolling cells into view ([#5515](https://github.com/elastic/eui/pull/5515))
77

8+
**Deprecations**
9+
10+
- Deprecated `data-gridcell-id` from `EuiDataGrid` in favor of 4 new and more flexible props - `data-gridcell-column-id`, `data-gridcell-column-index`, `data-gridcell-row-index`, and `data-gridcell-visible-row-index` ([#5515](https://github.com/elastic/eui/pull/5515))
11+
812
## [`45.0.0`](https://github.com/elastic/eui/tree/v45.0.0)
913

1014
- Added virtulized rendering option to `EuiSelectableList` with `isVirtualized` ([#5521](https://github.com/elastic/eui/pull/5521))

‎src/components/datagrid/__snapshots__/data_grid.test.tsx.snap‎

Lines changed: 160 additions & 10 deletions
Large diffs are not rendered by default.

‎src/components/datagrid/body/__snapshots__/data_grid_body.test.tsx.snap‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@ exports[`EuiDataGridBody renders 1`] = `
1515
>
1616
<div
1717
class="euiDataGridHeaderCell euiDataGridHeaderCell--boolean"
18-
data-gridcell-id="0,-1"
18+
data-gridcell-column-id="columnA"
19+
data-gridcell-column-index="0"
20+
data-gridcell-row-index="-1"
21+
data-gridcell-visible-row-index="-1"
1922
data-test-subj="dataGridHeaderCell-columnA"
2023
role="columnheader"
2124
style="width:100px"
@@ -54,7 +57,10 @@ exports[`EuiDataGridBody renders 1`] = `
5457
</div>
5558
<div
5659
class="euiDataGridHeaderCell euiDataGridHeaderCell--string"
57-
data-gridcell-id="1,-1"
60+
data-gridcell-column-id="columnB"
61+
data-gridcell-column-index="1"
62+
data-gridcell-row-index="-1"
63+
data-gridcell-visible-row-index="-1"
5864
data-test-subj="dataGridHeaderCell-columnB"
5965
role="columnheader"
6066
style="width:100px"
@@ -94,7 +100,11 @@ exports[`EuiDataGridBody renders 1`] = `
94100
</div>
95101
<div
96102
class="euiDataGridRowCell euiDataGridRowCell--boolean euiDataGridRowCell--firstColumn"
103+
data-gridcell-column-id="columnA"
104+
data-gridcell-column-index="0"
97105
data-gridcell-id="0,0"
106+
data-gridcell-row-index="0"
107+
data-gridcell-visible-row-index="0"
98108
data-test-subj="dataGridRowCell"
99109
role="gridcell"
100110
style="position:absolute;left:0;top:0px;height:34px;width:100px"
@@ -124,7 +134,11 @@ exports[`EuiDataGridBody renders 1`] = `
124134
</div>
125135
<div
126136
class="euiDataGridRowCell euiDataGridRowCell--string euiDataGridRowCell--lastColumn"
137+
data-gridcell-column-id="columnB"
138+
data-gridcell-column-index="1"
127139
data-gridcell-id="1,0"
140+
data-gridcell-row-index="0"
141+
data-gridcell-visible-row-index="0"
128142
data-test-subj="dataGridRowCell"
129143
role="gridcell"
130144
style="position:absolute;left:100px;top:0px;height:34px;width:100px"

‎src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,11 @@ exports[`EuiDataGridCell renders 1`] = `
4343
>
4444
<div
4545
className="euiDataGridRowCell"
46+
data-gridcell-column-id="someColumn"
47+
data-gridcell-column-index={0}
4648
data-gridcell-id="0,0"
49+
data-gridcell-row-index={0}
50+
data-gridcell-visible-row-index={0}
4751
data-test-subj="dataGridRowCell"
4852
onBlur={[Function]}
4953
onFocus={[Function]}

‎src/components/datagrid/body/data_grid_cell.tsx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,12 @@ export class EuiDataGridCell extends Component<
616616
ref={this.cellRef}
617617
{...cellProps}
618618
data-test-subj="dataGridRowCell"
619-
data-gridcell-id={`${this.props.colIndex},${this.props.visibleRowIndex}`}
619+
// Data attributes to help target specific cells by either data or current cell location
620+
data-gridcell-column-id={this.props.columnId} // Static column ID name, not affected by column order
621+
data-gridcell-column-index={this.props.colIndex} // Affected by column reordering
622+
data-gridcell-row-index={this.props.rowIndex} // Index from data, not affected by sorting or pagination
623+
data-gridcell-visible-row-index={this.props.visibleRowIndex} // Affected by sorting & pagination
624+
data-gridcell-id={`${this.props.colIndex},${this.props.rowIndex}`} // TODO: Deprecate in favor of the above 4 data attrs
620625
onKeyDown={handleCellKeyDown}
621626
onFocus={this.onFocus}
622627
onMouseEnter={() => {

‎src/components/datagrid/body/header/data_grid_header_cell_wrapper.test.tsx‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,10 @@ describe('EuiDataGridHeaderCellWrapper', () => {
4949
>
5050
<div
5151
className="euiDataGridHeaderCell"
52-
data-gridcell-id="0,-1"
52+
data-gridcell-column-id="someColumn"
53+
data-gridcell-column-index={0}
54+
data-gridcell-row-index="-1"
55+
data-gridcell-visible-row-index="-1"
5356
data-test-subj="dataGridHeaderCell-someColumn"
5457
role="columnheader"
5558
style={Object {}}
@@ -72,7 +75,10 @@ describe('EuiDataGridHeaderCellWrapper', () => {
7275
<div
7376
aria-label="test"
7477
className="euiDataGridHeaderCell euiDataGridHeaderCell--test"
75-
data-gridcell-id="0,-1"
78+
data-gridcell-column-id="someColumn"
79+
data-gridcell-column-index={0}
80+
data-gridcell-row-index="-1"
81+
data-gridcell-visible-row-index="-1"
7682
data-test-subj="dataGridHeaderCell-someColumn"
7783
role="columnheader"
7884
style={

‎src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,10 @@ export const EuiDataGridHeaderCellWrapper: FunctionComponent<EuiDataGridHeaderCe
134134
tabIndex={isFocused && !isCellEntered ? 0 : -1}
135135
className={classes}
136136
data-test-subj={`dataGridHeaderCell-${id}`}
137-
data-gridcell-id={`${index},-1`}
137+
data-gridcell-column-id={id}
138+
data-gridcell-column-index={index}
139+
data-gridcell-row-index="-1"
140+
data-gridcell-visible-row-index="-1"
138141
style={width != null ? { width: `${width}px` } : {}}
139142
{...rest}
140143
>

‎src/components/datagrid/data_grid.spec.tsx‎

Lines changed: 61 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,12 @@ describe('EuiDataGrid', () => {
236236
// starts with body in focus
237237
cy.focused().should('not.exist');
238238

239-
cy.get('[data-gridcell-id="1,1"]').click();
240-
cy.focused().should('have.attr', 'data-gridcell-id', '1,1');
239+
cy.get(
240+
'[data-gridcell-column-index="1"][data-gridcell-visible-row-index="1"]'
241+
).click();
242+
cy.focused()
243+
.should('have.attr', 'data-gridcell-column-index', '1')
244+
.should('have.attr', 'data-gridcell-row-index', '1');
241245
});
242246

243247
describe('cell keyboard interactions', () => {
@@ -269,7 +273,9 @@ describe('EuiDataGrid', () => {
269273

270274
// tab into the grid, should focus first cell after a short delay
271275
cy.focused().tab();
272-
cy.focused().should('have.attr', 'data-gridcell-id', '0,0');
276+
cy.focused()
277+
.should('have.attr', 'data-gridcell-column-index', '0')
278+
.should('have.attr', 'data-gridcell-row-index', '0');
273279

274280
cy.focused().tab().should('have.id', 'final-tabbable');
275281
});
@@ -282,27 +288,37 @@ describe('EuiDataGrid', () => {
282288
cy.get('[data-test-subj=euiDataGridBody]').focus();
283289

284290
// first cell is non-interactive and non-expandable = focus cell
285-
cy.focused().should('have.attr', 'data-gridcell-id', '0,0');
291+
cy.focused()
292+
.should('have.attr', 'data-gridcell-column-index', '0')
293+
.should('have.attr', 'data-gridcell-row-index', '0');
286294

287295
// already left-most, should be no-op
288296
cy.focused().type('{leftarrow}');
289-
cy.focused().should('have.attr', 'data-gridcell-id', '0,0');
297+
cy.focused()
298+
.should('have.attr', 'data-gridcell-column-index', '0')
299+
.should('have.attr', 'data-gridcell-row-index', '0');
290300

291301
// arrow right, expandable cell with no interactive = focus cell
292302
cy.focused().type('{rightarrow}');
293-
cy.focused().should('have.attr', 'data-gridcell-id', '1,0');
303+
cy.focused()
304+
.should('have.attr', 'data-gridcell-column-index', '1')
305+
.should('have.attr', 'data-gridcell-row-index', '0');
294306

295307
// arrow right, non-expandable cell with one interactive = focus interactive
296308
cy.focused().type('{rightarrow}');
297309
cy.focused().should('have.attr', 'data-test-subj', 'focusOnMe');
298310

299311
// arrow right, non-expandable cell with two interactives = focus cell
300312
cy.focused().type('{rightarrow}');
301-
cy.focused().should('have.attr', 'data-gridcell-id', '3,0');
313+
cy.focused()
314+
.should('have.attr', 'data-gridcell-column-index', '3')
315+
.should('have.attr', 'data-gridcell-row-index', '0');
302316

303317
// arrow right, expandable cell with two interactives = focus cell
304318
cy.focused().type('{rightarrow}');
305-
cy.focused().should('have.attr', 'data-gridcell-id', '4,0');
319+
cy.focused()
320+
.should('have.attr', 'data-gridcell-column-index', '4')
321+
.should('have.attr', 'data-gridcell-row-index', '0');
306322
});
307323

308324
it('cell expansion/interaction', () => {
@@ -314,34 +330,50 @@ describe('EuiDataGrid', () => {
314330

315331
// first cell is non-interactive and non-expandable, enter should have no effect
316332
cy.focused().type('{enter}');
317-
cy.focused().should('have.attr', 'data-gridcell-id', '0,0');
333+
cy.focused()
334+
.should('have.attr', 'data-gridcell-column-index', '0')
335+
.should('have.attr', 'data-gridcell-row-index', '0');
318336

319337
// second cell is expandable
320-
cy.get('[data-gridcell-id="1,0"]').click();
338+
cy.get(
339+
'[data-gridcell-column-index="1"][data-gridcell-row-index="0"]'
340+
).click();
321341
cy.focused().type('{enter}');
322342
cy.focused().should(
323343
'have.attr',
324344
'data-test-subj',
325345
'euiDataGridExpansionPopover'
326346
);
327347
cy.focused().type('{esc}');
328-
cy.focused().should('have.attr', 'data-gridcell-id', '1,0');
348+
cy.focused()
349+
.should('have.attr', 'data-gridcell-column-index', '1')
350+
.should('have.attr', 'data-gridcell-row-index', '0');
329351

330352
// third cell is non-expandable & interactive, click should focus on the link
331-
cy.get('[data-gridcell-id="2,0"]').click();
353+
cy.get(
354+
'[data-gridcell-column-index="2"][data-gridcell-row-index="0"]'
355+
).click();
332356
cy.focused().type('{enter}');
333357
cy.focused().should('have.attr', 'data-test-subj', 'focusOnMe');
334358

335359
// fourth cell is non-expandable with multiple interactives, click should focus on the cell
336-
cy.get('[data-gridcell-id="3,0"]').click();
360+
cy.get(
361+
'[data-gridcell-column-index="3"][data-gridcell-row-index="0"]'
362+
).click();
337363
cy.focused().type('{enter}');
338364
cy.focused().should('have.attr', 'data-test-subj', 'focusOnMe'); // focus trap focuses the link
339365
cy.focused().type('{esc}');
340-
cy.focused().should('have.attr', 'data-gridcell-id', '3,0');
366+
cy.focused()
367+
.should('have.attr', 'data-gridcell-column-index', '3')
368+
.should('have.attr', 'data-gridcell-row-index', '0');
341369

342370
// fifth cell is non-expandable & no-actions with multiple interactives, click should focus cell
343-
cy.get('[data-gridcell-id="4,0"]').click('topLeft'); // top left to avoid clicking a button
344-
cy.focused().should('have.attr', 'data-gridcell-id', '4,0');
371+
cy.get(
372+
'[data-gridcell-column-index="4"][data-gridcell-row-index="0"]'
373+
).click('topLeft'); // top left to avoid clicking a button
374+
cy.focused()
375+
.should('have.attr', 'data-gridcell-column-index', '4')
376+
.should('have.attr', 'data-gridcell-row-index', '0');
345377
// enable interactives & focus trap
346378
cy.focused().type('{enter}');
347379
cy.focused().should('have.attr', 'data-test-subj', 'btn-yes');
@@ -350,11 +382,17 @@ describe('EuiDataGrid', () => {
350382
cy.focused().tab();
351383
cy.focused().should('have.attr', 'data-test-subj', 'btn-yes');
352384
cy.focused().type('{esc}');
353-
cy.focused().should('have.attr', 'data-gridcell-id', '4,0');
385+
cy.focused()
386+
.should('have.attr', 'data-gridcell-column-index', '4')
387+
.should('have.attr', 'data-gridcell-row-index', '0');
354388

355389
// sixth cell is expandable cell with two interactives, click should focus on the cell
356-
cy.get('[data-gridcell-id="5,0"]').click('topLeft', { force: true }); // top left to avoid clicking a button
357-
cy.focused().should('have.attr', 'data-gridcell-id', '5,0');
390+
cy.get(
391+
'[data-gridcell-column-index="5"][data-gridcell-row-index="0"]'
392+
).click('topLeft', { force: true }); // top left to avoid clicking a button
393+
cy.focused()
394+
.should('have.attr', 'data-gridcell-column-index', '5')
395+
.should('have.attr', 'data-gridcell-row-index', '0');
358396
cy.focused().type('{enter}'); // trigger expansion popover
359397
cy.focused().should('have.attr', 'data-test-subj', 'btn-yes'); // focus trap should move focus to the first button
360398
cy.focused().parentsUntil(
@@ -369,15 +407,17 @@ describe('EuiDataGrid', () => {
369407
'euiDataGridExpansionPopover'
370408
);
371409
cy.focused().type('{esc}');
372-
cy.focused().should('have.attr', 'data-gridcell-id', '5,0');
410+
cy.focused()
411+
.should('have.attr', 'data-gridcell-column-index', '5')
412+
.should('have.attr', 'data-gridcell-row-index', '0');
373413
});
374414
});
375415
});
376416
});
377417

378418
function getGridData() {
379419
// wait for the virtualized cells to render
380-
cy.get('[data-gridcell-id="0,1"]');
420+
cy.get('[data-gridcell-column-index="0"][data-gridcell-row-index="1"]');
381421
const rows = cy.get('[role=row]');
382422
return rows.then((rows) => {
383423
const headers: string[] = [];

‎src/components/datagrid/data_grid.test.tsx‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,11 @@ describe('EuiDataGrid', () => {
529529
Array [
530530
Object {
531531
"className": "euiDataGridRowCell euiDataGridRowCell--firstColumn customClass",
532+
"data-gridcell-column-id": "A",
533+
"data-gridcell-column-index": 0,
532534
"data-gridcell-id": "0,0",
535+
"data-gridcell-row-index": 0,
536+
"data-gridcell-visible-row-index": 0,
533537
"data-test-subj": "dataGridRowCell",
534538
"onBlur": [Function],
535539
"onFocus": [Function],
@@ -550,7 +554,11 @@ describe('EuiDataGrid', () => {
550554
},
551555
Object {
552556
"className": "euiDataGridRowCell euiDataGridRowCell--lastColumn customClass",
557+
"data-gridcell-column-id": "B",
558+
"data-gridcell-column-index": 1,
553559
"data-gridcell-id": "1,0",
560+
"data-gridcell-row-index": 0,
561+
"data-gridcell-visible-row-index": 0,
554562
"data-test-subj": "dataGridRowCell",
555563
"onBlur": [Function],
556564
"onFocus": [Function],
@@ -571,7 +579,11 @@ describe('EuiDataGrid', () => {
571579
},
572580
Object {
573581
"className": "euiDataGridRowCell euiDataGridRowCell--stripe euiDataGridRowCell--firstColumn customClass",
582+
"data-gridcell-column-id": "A",
583+
"data-gridcell-column-index": 0,
574584
"data-gridcell-id": "0,1",
585+
"data-gridcell-row-index": 1,
586+
"data-gridcell-visible-row-index": 1,
575587
"data-test-subj": "dataGridRowCell",
576588
"onBlur": [Function],
577589
"onFocus": [Function],
@@ -592,7 +604,11 @@ describe('EuiDataGrid', () => {
592604
},
593605
Object {
594606
"className": "euiDataGridRowCell euiDataGridRowCell--stripe euiDataGridRowCell--lastColumn customClass",
607+
"data-gridcell-column-id": "B",
608+
"data-gridcell-column-index": 1,
595609
"data-gridcell-id": "1,1",
610+
"data-gridcell-row-index": 1,
611+
"data-gridcell-visible-row-index": 1,
596612
"data-test-subj": "dataGridRowCell",
597613
"onBlur": [Function],
598614
"onFocus": [Function],

‎src/components/datagrid/utils/scrolling.spec.tsx‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,24 @@ describe('useScroll', () => {
3838
cy.realPress('ArrowDown');
3939

4040
cy.realPress('End');
41-
cy.focused().should('have.attr', 'data-gridcell-id', '5,0');
41+
cy.focused()
42+
.should('have.attr', 'data-gridcell-column-index', '5')
43+
.should('have.attr', 'data-gridcell-visible-row-index', '0');
4244

4345
cy.realPress(['Control', 'End']);
44-
cy.focused().should('have.attr', 'data-gridcell-id', '5,14');
46+
cy.focused()
47+
.should('have.attr', 'data-gridcell-column-index', '5')
48+
.should('have.attr', 'data-gridcell-visible-row-index', '14');
4549

4650
cy.realPress('Home');
47-
cy.focused().should('have.attr', 'data-gridcell-id', '0,14');
51+
cy.focused()
52+
.should('have.attr', 'data-gridcell-column-index', '0')
53+
.should('have.attr', 'data-gridcell-visible-row-index', '14');
4854

4955
cy.realPress(['Control', 'Home']);
50-
cy.focused().should('have.attr', 'data-gridcell-id', '0,0');
56+
cy.focused()
57+
.should('have.attr', 'data-gridcell-column-index', '0')
58+
.should('have.attr', 'data-gridcell-visible-row-index', '0');
5159
});
5260
});
5361
});

0 commit comments

Comments
 (0)