Skip to content

add within API #111

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

Closed
wants to merge 2 commits into from
Closed

add within API #111

wants to merge 2 commits into from

Conversation

Dajust
Copy link

@Dajust Dajust commented Jun 12, 2018

What: Add a within API

Why: Kindly see issue #53 of dom-testing-library

How: Exported getQueriesForElement as within from the index.js file

Checklist:

  • Documentation
  • Tests
  • Ready to be merged
  • Added myself to contributors table

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much Justice! Could you open this up in dom-testing-library instead?

src/index.js Outdated
@@ -44,4 +44,10 @@ syntheticEvents.forEach(eventName => {

// just re-export everything from dom-testing-library
export * from 'dom-testing-library'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was actually hoping this PR would be made to dom-testing-library. Then we'll get it automatically in react-testing-library because of this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kentcdodds, I noticed that dom-testing-library already exported getQueriesForElement as something else:

export {getQueriesForElement as bindElementToQueries}

Is it okay to still export it as yet another thing? like:

export {getQueriesForElement as bindElementToQueries}
export {getQueriesForElement as within}

I've not done such export before 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that works just fine 👍 You can even do it in a single line:

export {getQueriesForElement as bindElementToQueries, getQueriesForElement as within}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks man!

How about the docs, should I also move it to dom-testing-library?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to have it in both places actually. I think we really need a documentation website to consolidate this, but I don't have time to build it right now 😅

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have updated this to add only docs.

@kentcdodds
Copy link
Member

Could you resolve merge conflicts please? Thanks!

@@ -620,6 +621,22 @@ expect(submitButtons).toHaveLength(3) // expect 3 elements
expect(submitButtons[0]).toBeInTheDOM()
```

## `within` API

Sometimes, there is no garauntee that the text, placeholder, or label you want to query is unique on the page. So you might want to explicity tell react-render-dom to get an element **only within** a particular section of the page. `within` is a helper function for this case.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small typo in here, garauntee should be guarantee 🎉

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's react-render-dom?

@kentcdodds
Copy link
Member

🎉 This issue has been resolved in version 4.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

4 participants