-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add a .childAt(index) method #187
Add a .childAt(index) method #187
Conversation
Just an FYI, you can do this in userland. Just do something like this before your tests, ShallowWrapper.prototype.childrenAt = function(filter, selector) {
return this.children(selector).at(filter);
}
tests(wrapper.childrenAt(0)).toBeDefined(); |
@blainekasten totally... But I believe the idea is to provide as many useful methods as possible internally in the library... I don't think extending the library on the fly would be the best approach. Also adding this code or importing it in all my test files wouldn't scale very well... But thanks for the snippet. |
My only concern is that we are creating meaning for an integer in the same argument slot that a selector already is. This doesn't have any issues right now, but if we ever wanted to have a number mean something as a selector, we would not be able to do this. Using a number as a selector may seem like a strange idea, but it's actually something I've considered for better utility with React Native. React Native produces numbers for the output of I'm not in disagreement about the utility of this pattern, though. An alternative might be to have a new method, called Thoughts? |
|
@mauriciosoares would you like to update your PR to use |
cool, I think that I'll update this PR asap. |
We could also work on implementing the |
hey @lelandrichardson and @ljharb I've updated my PR... Would you mind to take a look? most importantly in the docs? :) |
@blainekasten before we go implementing pseudo-selectors, I'd like to investigate how difficult it would be to bring in https://github.com/jquense/bill in a non-breaking fashion. It would be nice to have the CSS selection bits of enzyme outside of the project. Additionally having a |
|
||
```jsx | ||
const wrapper = shallow(<ToDoList items={items} />); | ||
expect(wrapper.find('ul').childAt(0).type).to.equal('li'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/type/type()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to double check, by s/type/type()
you mean that I should write: wrapper.find('ul').childAt(0).type()
correct?
I forgot that type is a method...
@mauriciosoares made a couple of comments. It also looks like you are missing the |
@@ -0,0 +1,28 @@ | |||
# `.childAt(index) => ReactWrapper` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: you will want to include both of these pages in the /docs/README.md
page, as well as including links to them in the /docs/api/shallow.md
and /docs/api/mount.md
pages.
Thanks!
You're correct. missed that hahaha. I'll update the code and docs and let you know. tks |
@lelandrichardson I have updated this PR, would you care to take a look? :) |
@mauriciosoares looks great. Can you rebase and squash this into one commit for me? That'd be perfect! |
82713f1
to
9ae5bea
Compare
@lelandrichardson should I squash the "master merge commit" into mine as well? |
@mauriciosoares we prefer the rebase workflow.... if you could do an interactive rebase and remove the merge commit, then rebase from upstream, we should end up with a single commit on top of current upstream/master. Make sense? |
Now you can filter the childrens by a number, which would be equivalent to use the `at()` method. `.children(0)` === `children().at(0)`; update the children docs remove comment update the api This commit undo the changes I've done to the `children` method, and created a new method named `childAt`. update the docs update docs and test description update api and tests update docs
9ae5bea
to
da803c4
Compare
@lelandrichardson sorry for the mess... I believe that now this is what you expected. (I've never used interactive rebase before 😄) |
@mauriciosoares perfect! Thanks for doing that. This all looks good - merging. |
Add a .childAt(index) method
@lelandrichardson feel free to ping me on the https://github.com/jquense/bill repo with any question you might have. We built it for teaspoon, our enzyme-esque testing lib, so I think there's probably a lot of overlap of needs, and more then happy to collaborate to have it cover more use cases. I had also mentioned this briefly with @ljharb on IRC a while back. |
Hello Guys, this PR improves the
.children
API a little bit.Now the
children
API accepts a number, and this number will filter the index of the children.Example:
The 2 above are equivalent.
The thing is, I noticed that I've been using the
children().at(x)
pattern too much, and this can be simplified by passing the number directly to thechildren
method.As the
selector
is a type of filter for thechildren
method, I believe thenumber
can be used as well.One thing I'd like to ask is: In case you accept this PR, take a look at the docs and check if the examples are clear enough (I'm not the best doc writter in the world hahahaha).
Cheers