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

Would like to have some data-testid attributes on ui components of the datatable #618

Closed
jdetle opened this issue May 13, 2019 · 5 comments

Comments

@jdetle
Copy link
Contributor

jdetle commented May 13, 2019

Hi, I have a mui-datatable that has some custom search functionality that I'd like to test. I've gotten really into the react-testing-library approach of actually testing the component, and am running into issues with being able to select different elements of the datatable. If you'd be okay with it, I'd like to submit a PR to add data-testid props to key elements of the component.

@jdetle jdetle changed the title Would like to have some data-testid attributes so that I can test some of my customSearch work Would like to have some data-testid attributes on ui components of the datatable May 13, 2019
@gabrielliwerant
Copy link
Collaborator

gabrielliwerant commented May 14, 2019

I think it's a good idea to add ids for testing, but maybe we should come up with a standard naming approach first for consistency? I like data-test-id as the attr, and something like component-name-element-name?

Do you have any thoughts on this? @gregnb

@jdetle
Copy link
Contributor Author

jdetle commented May 14, 2019

I think it's a good idea to add ids for testing, but maybe we should come up with a standard naming approach first for consistency? I like data-test-id as the attr, and something like component-name-element-name?

I am onboard with whatever approach you all agree on. The standard syntax is data-testid, which is useful to have for helper functions in react-testing-library, so thats the only hill I would die on. Thanks for your quick engagement.

@gabrielliwerant
Copy link
Collaborator

You're right, it looks like react native set the precedent for data-testid. Looking at that hurts my brain, but convention is convention! 😛

Ok, I say go ahead with this, and lets see what you come up with as far as naming conventions for the actual ids.

@jdetle
Copy link
Contributor Author

jdetle commented May 25, 2019

I got some work done on this, will be pushing it up on wednesday after I'm done giving a talk.

@gabrielliwerant
Copy link
Collaborator

Test ids added in 2.7.0.

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

Successfully merging a pull request may close this issue.

2 participants