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

Pass aria properties through - add RTL test example #438

Merged
merged 8 commits into from
May 19, 2022

Conversation

dwjohnston
Copy link
Contributor

@dwjohnston dwjohnston commented Feb 22, 2021

This PR allows and demonstrates the use of React Testing Library with Draftail.

It passes through aria properties so RTL style queries can be used.

facebookarchive/draft-js#2833

  • Stay on point and keep it small so it can be easily reviewed. For example, try to apply any general refactoring separately outside of the PR.
  • Consider adding unit tests, especially for bug fixes. If you don't, tell us why.
  • All new and existing tests pass, with 100% test coverage (npm run test:coverage)
  • Linting passes (npm run lint)
  • Consider updating documentation. If you don't, tell us why.
  • List the environments / platforms in which you tested your changes.

Thanks for contributing to Draftail!

@thibaudcolas
Copy link
Collaborator

Hey @dwjohnston, thank you for proposing this. It feels a bit odd to me to change the editor’s API to accommodate any particular testing tool, but I’d definitely want testing with React Testing Library to be possible, so I’ll have a look.

@dwjohnston
Copy link
Contributor Author

@thibaudcolas I've since removed the onMount stuff - so basically this PR is passing through the aria labels, and adds RTL and a demo test.

@dwjohnston dwjohnston mentioned this pull request Feb 24, 2021
6 tasks
@dwjohnston dwjohnston changed the title Onmount fork Pass aria properties through - add RTL test example Feb 24, 2021
@dwjohnston
Copy link
Contributor Author

dwjohnston commented Feb 24, 2021

@thibaudcolas I'm unable to make sense of the errors in the CI. Are you able to walk this through for me? We are otherwise maintaining a fork just to get those aria properties
Note I have removed the pre-publish script, it was erroring locally for me building storybook, you probably want to put that back in.

@thibaudcolas thibaudcolas merged commit f1fbc27 into springload:main May 19, 2022
@thibaudcolas
Copy link
Collaborator

Thank you @dwjohnston! I’m currently going through a lot of changes to the library, will pick up the remaining test failures later.

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

Successfully merging this pull request may close these issues.

2 participants