dom-testing-library icon indicating copy to clipboard operation
dom-testing-library copied to clipboard

Queries for tables

Open lexanth opened this issue 4 years ago • 20 comments

Describe the feature you'd like:

The current queries don't really allow anything matching how a user would interact with a table. There is limited support using the ByRole selectors, to choose all rows, or all cells, or nesting between these, but doesn't always match the principle of "use your component as a user would".

e.g. In a table where each row has a "Delete" icon, choosing the delete icon corresponding to the particular row contents. A user would identify which row they're looking at by the value in one of the cells, then scanning across.

Suggested implementation:

I've implemented some custom queries in https://github.com/lexanth/testing-library-table-queries#readme, but some of it is a bit hacky.

Describe alternatives you've considered:

getAllByRole('row') gets rows, but then filtering by text content is slightly awkward (either index based or filtering based on content).

Teachability, Documentation, Adoption, Migration Strategy:

This would be additional queries, so backwards-compatible.

lexanth avatar May 19 '20 20:05 lexanth

Could you post the markup for your table so that we can identify how testing-library should work?

eps1lon avatar May 19 '20 22:05 eps1lon

As an example:

Name Age Actions
John 23 [Delete]
Jane 34 [Delete]
Joe 45 [Delete]
<table>
  <thead>
    <tr>
      <th>Name</th>
      <th>Age</th>
      <th>Actions</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <td>John</td>
      <td>23</td>
      <td><button>Delete</button></td>
    </tr>
    <tr>
      <td>Jane</td>
      <td>34</td>
      <td><button>Delete</button></td>
    </tr>
    <tr>
      <td>Joe</td>
      <td>45</td>
      <td><button>Delete</button></td>
    </tr>
  </tbody>
</table>

Query examples: e.g. Joe's age should be 45 (might be worth testing as DOB is stored, but age is presented)

expect(getAllByRole('row')[3]).toContainTextContent(/45/)

Picking the row by index doesn't seem to be testing as a user would. And I'm not really testing the Age column, so if I added a phone number and he happened to have 45 in that, my test no longer verifies the age.

Really what I want to do here is "Get row with Joe in the first column, Get cell in "Age" column"

e.g. Click the delete button against Jane

fireEvent.click(within(getAllByRole('row')[2]).getByText('Delete'))

Again, I'm having to pick the row by index, which makes the test more brittle and harder to refactor if something changes.

Another alternative would be adding data-testid attributes all over that table, but again, not exactly how a user would see it.

Really what I want to do here is "Get row with Jane in the first column, click item inside with Delete text"

lexanth avatar May 28 '20 11:05 lexanth

const row = getByRole('cell', {name: 'Jane'}).closest('tr')
fireEvent.click(
  within(row).getByText('Delete')
)

Of course, this doesn't work if you aren't using literal table elements, virtualized tables, etc.

alexkrolick avatar May 28 '20 23:05 alexkrolick

How about

const row = getByText('Jane', { closest: 'row' })
fireEvent.click(
  within(row).getByText('Delete')
)

or

const row = within(getByRole('cell', {name: 'Jane'})).getByClosestRole('row')
fireEvent.click(
  within(row).getByText('Delete')
)

The general idea is to use a role to get the closest element instead of a selector.

filipegiusti avatar May 29 '20 15:05 filipegiusti

@filipegiusti I guess these are further api proposals, rather than actual (workaround) implementation?

jarek-jpa avatar Sep 16 '20 18:09 jarek-jpa

Exactly, those are further api proposals. As this is from May I don't expect it to be ever approved.

filipegiusti avatar Sep 17 '20 14:09 filipegiusti

Instead of within couldn't we nest query function calls?

fireEvent.click(getByText(getBySomething(rowStuff), 'Delete'))

nickmccurdy avatar Nov 09 '20 09:11 nickmccurdy

Are there any examples or documentation of the current best practices for working with tables?

bradleylandis avatar Jul 02 '21 14:07 bradleylandis

I'm also interested in this. I feel the same that testing table's right now does not feel like testing how a user would.

If I were to visually process the rows of data in a table, I would typically reference the column header text as I scan across the row data.

Alternatively, I might look for a particular column, and then scan down the table looking for values of interest.

As a user, you're also typically using context to determine what constitutes uniqueness of data in a table. Perhaps it's a single column value, but maybe it's a composite key of two or more columns like first and last name for instance.

So in that case, say I'm looking for the favorite color of someone named "Steve Rogers." I might scan for first name's of "Steve" and as I find a row that matches, then I'd check whether the value in the last name column matches "Rogers" continuing my scan until the desired row is found or no more rows exist. If found, I could look to the column for favorite color to get the information I'm looking for.

Expressed in RTL, I think the way I'd have to achieve that now would be something like this (untested and just typed here by hand so not sure this is 100% sound).

let favoriteColor = null;

const knownIndexOfColorColumn = getColorColumnIndex();
const table = screen.getByTitle("Super hero stats");

const firstNames = within(table).queryAllByText("Steve");

for (let i = 0; i < firstNames.length; i++) {
    const row = firstNames[i].parent;

    // perhaps this should even limit the search to within the specific cell of the last name column
    const lastName = within(row).queryByText("Rogers");

    if (lastName) {
        favoriteColor = within(row).getAllByRole("cell")[knownIndexOfColorColumn];

        break;
    }
}

expect(within(favoriteColor).getByText("Red")).toBeInTheDocument();

But that just doesn't seem very elegant or to be in the spirit of RTL. I don't have a proposal for how to do it in a more "user testing" manner, but maybe I'm just being rigid in my thinking and someone can offer a better way that the above could be achieved in a more RTL-ish way.

joshua-phillips avatar Jul 28 '21 14:07 joshua-phillips

This got my head in the right space to solve a similar issue. https://polvara.me/posts/five-things-you-didnt-know-about-testing-library Specifically, const row = screen.getByText(id).closest("tr");

ThomasGHenry avatar Sep 23 '21 22:09 ThomasGHenry

A couple of solutions here suggest using 'closest' but the testing-library linter dislikes this: https://github.com/testing-library/eslint-plugin-testing-library/blob/main/docs/rules/no-node-access.md

cbookg avatar Nov 25 '21 19:11 cbookg

After thinking about this alot, I found a way that seems to work without using closest and would most "resemble the way your software is used".

getByRole('row', { name: /Joe/ })

testing-playground

You can then use within to query for other things in the row.

All rows get an aria accessible name that is all the text content within the row separated by spaces. The example table originally on this thread has the aria name Joe 45 Delete.

By using regex for the name option you are able to isolate the row you need both ByRole and indirectly "ByText".

kealjones-wk avatar Dec 13 '21 23:12 kealjones-wk

I've been digging into this for a project I've been working on with the intention of building queries based on table headers, which is something I've found challenging in the past. I tried to be pretty comprehensive to account for semantic and aria-* tables alike with various rowSpans and colSpans.

I'm sure that I've made some incorrect assumptions but figured I'd share what I came up with and propose a path forward if this is of interest.

Code sample

Here's the gist of my solution 🔗

So far I've tested this for the following table:

Table of Items Sold August 2016 with nested row headers and column headers which is tested in the provided Github Gist
┌─────────┬───────────────────┬─────────────┬────────────┬───────────┬───────────┬───────────────┬───────────────┐
│ (index) │         0         │      1      │     2      │     3     │     4     │       5       │       6       │
├─────────┼───────────────────┼─────────────┼────────────┼───────────┼───────────┼───────────────┼───────────────┤
│    0    │        ''         │     ''      │ 'Clothes'  │ 'Clothes' │ 'Clothes' │ 'Accessories' │ 'Accessories' │
│    1    │        ''         │     ''      │ 'Trousers' │ 'Skirts'  │ 'Dresses' │  'Bracelets'  │    'Rings'    │
│    2    │     'Belgium'     │  'Antwerp'  │    '56'    │   '22'    │   '43'    │     '72'      │     '23'      │
│    3    │     'Belgium'     │   'Gent'    │    '46'    │   '18'    │   '50'    │     '61'      │     '15'      │
│    4    │     'Belgium'     │ 'Brussels'  │    '51'    │   '27'    │   '38'    │     '69'      │     '28'      │
│    5    │ 'The Netherlands' │ 'Amsterdam' │    '89'    │   '34'    │   '69'    │     '85'      │     '38'      │
│    6    │ 'The Netherlands' │  'Utrecht'  │    '80'    │   '12'    │   '43'    │     '36'      │     '19'      │
└─────────┴───────────────────┴─────────────┴────────────┴───────────┴───────────┴───────────────┴───────────────┘

API

My proposal is to add a query, byTableHeaderText, and a corresponding matcher, toHaveTableHeaderText to enable the following:

const gentRow = within(table).getByRole('row', /Gent/);

const gentTrousers = within(gentRow).getByTableHeaderText('Trousers');
expect(gentTrousers).toHaveTableHeaderText('Gent');

A few more examples:

// Note: the second argument with scope is optional
const belgiumCells = screen.getAllByTableHeaderText('Belgium', { scope: 'rowgroup' });
const gentCells = screen.getAllByTableHeaderText('Gent', { scope: 'row' });

const gentHeader = within(table).getByRole('rowheader', { name: 'Gent' });
expect(gentHeader).toHaveTableHeaderText('Belgium', { scope: 'rowgroup' });

const clothesCells = screen.getAllByTableHeaderText('Clothes', { scope: 'colgroup' });
const trousersCells = screen.getAllByTableHeaderText('Trousers', { scope: 'col' });

const trousersHeader = within(table).getByRole('columnheader', { name: 'Trousers' });
expect(trousersHeader).toHaveTableHeaderText('Clothes', { scope: 'colgroup' });

// Filter to get the intersection
const gentTrousersCells = _.intersection(gentCells, trousersCells);

// Alternatively without lodash this intersection would be:
// gentCells.filter(cell => trousersCells.includes(cell));

If this is of interest I'd be happy to put up a PR!

enagy27 avatar Dec 19 '21 08:12 enagy27

This is how I am doing it right now:

const row = screen.getAllByRole('row').find((row) => within(row).queryByText(text) !== null);

This means it will return undefined if no result is found and the HTMLElement if found, which still doesn't match up to exactly what get* means but it's better than nothing...

jason-yang avatar Mar 21 '22 18:03 jason-yang

I think we have a winner idea already, the most liked comment above.

anilanar avatar Nov 10 '22 22:11 anilanar

Was this ever integrated into testing-library?

cjsilva-umich avatar Dec 08 '22 21:12 cjsilva-umich

It is often that you would like to assert the content of a tabular information To me a nice API would look like:

expect(screen.getByRole('cell', {row: 1, column: 1, name: 'foo'})).toBeInDocument();
expect(screen.getByRole('cell', {columnheader: 'foo', rowheader: 'bar', name: '1'})).toBeInDocument();

Off the topic sorry, but why the role is cell and not gridcell as shown int he inspector ? @mathieu-suen-sonarsource

Jhonatangiraldo avatar Feb 22 '23 17:02 Jhonatangiraldo

cell is the more generic role of a gridcell according to w3c

This let me think that we can also encourage the following techniques using this API: expect(screen.getByRole('cell', {headers: ['a', 'b']})).toBeInDocument();