Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IDs for testing #514

Closed
Andrii-A opened this issue Feb 21, 2020 · 11 comments
Closed

IDs for testing #514

Andrii-A opened this issue Feb 21, 2020 · 11 comments

Comments

@Andrii-A
Copy link

Andrii-A commented Feb 21, 2020

Expected Behavior

Each table row has it's own data-id for testing purpose.
Each cell in the row has it's own data-id for testing purpose.

Current Behavior

The table produces pretty complicated layout and it's hard to test it without specific hooks.
Currently I need to do something like cy.get('[data-attr-id="my-table"] > div > div > div:nth-child(3) > div') to access table rows with Cypress. To access the cell in the table to check it's value, the path will be twice longer.

Possible Solution

To add to each table row something like data-id="fixed-data-table-row-${idx}".
To add to each row cell something like data-id="fixed-data-row-cell-${idx}".

@Andrii-A Andrii-A changed the title Rows id for testing IDs for testing Feb 22, 2020
@wcjordan
Copy link
Member

I'd be fine w/ a PR for this. Any concerns from your end @pradeepnschrodinger ?

@pradeepnschrodinger
Copy link
Collaborator

No concerns, this sounds good.

FWIW, we set aria attributes for the rows at <FixedDataTableRowImpl/>. I recommend showing actual row index at the same DOM level.
And for cells, it might be helpful to store both column id and row id, probably around here.

@vcardins
Copy link

Hey guys, any progress on this issue?

@wcjordan
Copy link
Member

@Andrii-A is this something you're working on or is it dormant?

@Andrii-A
Copy link
Author

Andrii-A commented Aug 6, 2020

Well, it was more like a feature request... Sorry for not making it clear.
Unfortunately now I don't really have capacity to contribute myself.

@wcjordan
Copy link
Member

wcjordan commented Aug 6, 2020

Cool, no worries. Sounds like this is dormant, @vcardins

@vcardins
Copy link

vcardins commented Apr 30, 2021

Hey guys, by investigating the source code I found an undocumented prop that does the job:

<Table rowAttributesGetter={getRowAttributes} />

then

function getRowAttributes(rowIndex: number) {
	const row = <your_row_object>[rowIndex];
	return { id: <your_method_to_build_row_id>.(row) };
}

I hope it helps. @wcjordan Could you update the documentation and maybe close this ticket?
Thanks

@pradeepnschrodinger
Copy link
Collaborator

@vcardins, nice catch.

For more info, rowAttributesGetter was added along with gridAttributesGetter through #491, but only the latter prop was documented.
We'll just need to add it in FixedDataTable's prop types to have it included in the docs.

@vcardins
Copy link

vcardins commented May 3, 2021

@pradeepnschrodinger Thanks for the prompt response. How soon should we expect an update?

@pradeepnschrodinger
Copy link
Collaborator

@vcardins , I put a PR #580 for adding the docs, which is in turn blocked by #581.

I'll give it a couple more days to have it merged.

@pradeepnschrodinger
Copy link
Collaborator

Added docs for rowAttributesGetter through #580, and released it with v1.1.3.

The new docs should appear as part of the Table API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants