Docs ab #1947

allan-bonadio · 2018-12-17T06:10:49Z

Overhaul to a lot of ReactWrapper and ShallowWrapper method files, plus others.

Defined 'solo' as a wrapper with one node, and used that terminology throughout. Removed the term 'current node' as applied to a wrapper, and changed each to better wording.
rewrote sections for functions matchesElement, containsMatchingElement, containsAllMatchingElements, containsAnyMatchingElements; each refer to containsMatchIngElement and matchesElement for matching rules. Renamed the matcher node to 'patternNode' to avoid confusion.
compared Shallow and React wrapper methods, corrected a few discrepancies
added some notes for methods in React or Shallow that are not in the other
added .length to both the React and Shallow directories.

Closes #1919.

- Defined 'solo' as a wrapper with one node, and used that terminology throughout. Removed the term 'current node' as applied to a wrapper, and changed each to better wording. - rewrote sections for functions matchesElement, containsMatchingElement, containsAllMatchingElements, containsAnyMatchingElements; each refer to containsMatchIngElement and matchesElement for matching rules. Renamed the matcher node to 'patternNode' to avoid confusion. - compared Shallow and React wrapper methods, corrected a few discrepancies - added some notes for methods in React or Shallow that are not in the other - added .length to both the React and Shallow directories.

ljharb

It'd be very helpful if these could be broken up into separate commits for each kind of change - as it is, this is quite a lot to review.

ljharb · 2018-12-17T06:54:29Z

README.md

@@ -6,8 +6,7 @@ Enzyme
 [![npm Version](https://img.shields.io/npm/v/enzyme.svg)](https://www.npmjs.com/package/enzyme) [![License](https://img.shields.io/npm/l/enzyme.svg)](https://www.npmjs.com/package/enzyme) [![Build Status](https://travis-ci.org/airbnb/enzyme.svg)](https://travis-ci.org/airbnb/enzyme) [![Coverage Status](https://coveralls.io/repos/airbnb/enzyme/badge.svg?branch=master&service=github)](https://coveralls.io/github/airbnb/enzyme?branch=master)


-Enzyme is a JavaScript Testing utility for React that makes it easier to assert, manipulate,
-and traverse your React Components' output.
+Enzyme is a JavaScript Testing utility for React that makes it easier to test your React Components' output.


Why this change? The previous version was correct - i think it's important to mention manipulation and traversal.

Just because something is technically correct, doesn't mean it's informative to someone who's never used Enzyme. When I first read that, I dismissed it as technical mumbo-jumbo. I didn't want to assert, manipulate or traverse, I wanted to test my software. I had to do the tut to figure out what enzyme does. This is the elevator speech; you have to make it as simple as possible. Maybe instead of 'test' it could be 'verify' or 'analyze'. I get it that you can simulate some stuff; maybe 'analyze' can cover that, too.

I definitely like "test" over "assert" (although the two are largely synonymous), but manipulation and traversal are critical means of testing.

what kind of 'manipulation'? certainly you can do stuff with these wrappers, but I don't think you can actually change, say, the nesting of nodes. You can run them through some runtime simulations: simulate, state, props, etc. I can't think of a simple word for this.

I'm trying to think of someway that 'traverse' might be considered part of 'test'. I'm not trying to take the wind out of enzyme's sails, I just want a naive reader to say "That's exactly what I need!"

it is now:
Enzyme is a JavaScript Testing utility for React that makes it easier to test your React Components' output.
You can also manipulate, traverse and in some ways simulate runtime given the output.

You can set props and state.

ljharb · 2018-12-17T06:55:08Z

docs/api/ReactWrapper/closest.md

-current node's ancestors in the tree, starting with itself.
-
-Note: can only be called on a wrapper of a single node.
+wrapped node's ancestors in the tree, starting with itself.  The wrapper must be solo.


I don't think "solo" is a particularly clear term; "a single node" seems clearer to me. Can these changes be reverted?

Well, it's referring to a wrapper, so the whole phrase is "wrapper of a single node". That's 5 words. "Single-node wrapper" is 3 words; I was trying to get it down to two words. There's a lot of methods that only work on single-node wrappers, and a lot of functions that return them. In fact, it's almost like they're a subclass of Wrapper, with their own methods like closest, instance, etc. I count 17 such methods. We don't need a subclass, just a noun. You can get away with defining a term in just a few well-traveled places, say like "must be a solo wrapper (with just 1 child)". They figure it out.

Word count isn't the thing that's going to solely determine how easy it is to digest; people can skim over phrases just as easily as words.

I don't want to invent a new noun in the quest for reducing confusion.

How about we just stick to 'single-node wrapper'?

Sounds good to me!

ljharb · 2018-12-17T06:55:56Z

docs/api/ReactWrapper/containsAllMatchingElements.md

-# `.containsAllMatchingElements(nodes) => Boolean`
+# `.containsAllMatchingElements(patternNodes) => Boolean`
+
+Returns whether or not all of the given react elements in `patternNodes` match an element in the wrapper's render tree.  Every single element of `patternNodes` must be matched one or more times.  Matching follows the rules for `containsMatchingElement`.


containsMatchingElement should probably be a related method here

also, doesn't this matching really follow the rules of matchesElement?

Yeah. lemme clean this up more.

ljharb · 2018-12-17T06:56:00Z

docs/api/ReactWrapper/containsAnyMatchingElements.md


-Returns whether or not one of the given react elements is matching on element in the render tree.
-It will determine if an element in the wrapper __looks like__ one of the expected element by checking if all props of the expected element are present on the wrappers element and equals to each other.
+Returns whether or not at least one of the given react elements in `patternNodes` matches an element in the wrapper's render tree.  One or more elements of `patternNodes` must be matched one or more times.  Matching follows the rules for `containsMatchingElement`.


containsMatchingElement should probably be a related method here

ljharb · 2018-12-17T06:57:58Z

docs/api/ReactWrapper/exists.md

@@ -12,7 +12,7 @@ Returns whether or not the current node exists. Or, if a selector is passed in,

 #### Returns

-`Boolean`: whether or not the current node exists, or the selector had any results.
+`Boolean`: whether or not any nodes are on the list, or the selector had any matches.


"on the list" isn't really a term that makes sense to me; maybe "the wrapper contains any nodes"?

changing it to
...whether or not any nodes exist in the wrapper

ljharb · 2018-12-17T07:01:32Z

docs/api/selector.md


-### 2. Prop Selector


Why remove this section?

it's part of regular CSS so I merged it into the CSS case. Isn't it?

It's a bit different, because you can select on prop values - which can be numbers or booleans as well as strings. CSS selectors only and exclusively operate on string values.

yeah, but the result looks almost the same. '[num=51]' will match num={51}, but not num='51'. And, '[num="51"]' will match num='51' but not num={51}. I think that should get a better description.
Meanwhile, these constructs can mix in with the rest of the CSS selector, so isn't it a part of CSS?

It would be good to have a full accounting of the differences between enzyme selectors and css selectors. There was one phrase in there, "Further, enzyme supports combining any of those supported syntaxes together to uniquely identify a single node." I misinterpreted that to mean you can only select one node if you combine syntaxes. I rewrote the section.

ljharb · 2018-12-17T07:02:21Z

docs/api/selector.md

@@ -79,11 +78,11 @@ const myComponents = wrapper.find(MyComponent);



-### 4. A React Component’s displayName
+### 3. A React Component’s displayName

 enzyme allows you to find components based on a component’s `displayName`. If a component exists
 in a render tree where its `displayName` is set and has its first character as a capital letter,


this isn't technically accurate; for example, things like withSomeHOC(ComponentName) can also be found in many cases (as noted below); perhaps this sentence needs updating as well?

I don't understand what the problem is. Isn't displayName sortof self-service?

ljharb · 2018-12-17T07:03:39Z

docs/api/render.md

@@ -1,6 +1,7 @@
 # Static Rendering API

-enzyme's `render` function is used to render react components to static HTML and analyze the
+Use enzyme's `render` function to generate HTML text from your React tree, 


"HTML text" is a contradiction; something is either HTML, or text. "static HTML" is the proper term here.

and I thought of 'static' as meaning, 'class-based' or 'standing in one spot', neither of which made sense. how about just 'HTML'?

ljharb · 2018-12-17T07:04:07Z

docs/api/ShallowWrapper/type.md

@@ -1,14 +1,15 @@
-# `.type() => String|Function|null`
+# `.type() => String | Function | null`


I believe the jsdoc-based convention here is to not have spaces around the |, even if it's less readable that way.

i'll put them back

docs/api/ShallowWrapper/parent.md

allan-bonadio · 2019-01-21T18:11:59Z

So are we done with this? I think I have in all the changes for now. Am I supposed to complete conversations or something?

ljharb · 2019-01-21T21:17:38Z

I'll rebase it and take another pass :-) thanks for your patience!

ljharb

Thanks, this was a lot of effort and it's much appreciated!

allan-bonadio · 2019-01-23T01:35:13Z

thanks! Allan Bonadio bojnac -at- tactileint.org portfolio.tactileint.org <http://portfolio.tactileint.org/> resume.tactileint.org <http://resume.tactileint.org/>

…

On Jan 21, 2019, at 2:34 PM, Jordan Harband ***@***.***> wrote: Merged #1947 <#1947> into master. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1947 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA91oG3S0HLFuQCfHjLyc3hwBd0JODM5ks5vFkBZgaJpZM4ZVvFc>.

allan-bonadio mentioned this pull request Dec 17, 2018

documentation is too vague #1919

Closed

10 tasks

ljharb requested changes Dec 17, 2018

View reviewed changes

ljharb added the docs label Dec 17, 2018

ljharb force-pushed the docs-ab branch 2 times, most recently from 63193df to 2932d8c Compare January 21, 2019 22:11

ljharb approved these changes Jan 21, 2019

View reviewed changes

ljharb merged commit 2932d8c into enzymejs:master Jan 21, 2019

ljharb added a commit that referenced this pull request Jan 22, 2019

[Docs] add .length (mistakenly omitted from #1947)

2a00a00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Docs ab #1947

Docs ab #1947

allan-bonadio commented Dec 17, 2018 •

edited by ljharb

Loading

ljharb left a comment

ljharb Dec 17, 2018

allan-bonadio Dec 19, 2018

ljharb Dec 19, 2018

allan-bonadio Dec 20, 2018

allan-bonadio Dec 21, 2018

ljharb Dec 24, 2018

ljharb Dec 17, 2018

allan-bonadio Dec 19, 2018

ljharb Dec 19, 2018

allan-bonadio Dec 20, 2018

ljharb Dec 24, 2018

ljharb Dec 17, 2018

ljharb Dec 17, 2018

allan-bonadio Dec 19, 2018

ljharb Dec 17, 2018

allan-bonadio Dec 19, 2018

ljharb Dec 17, 2018

allan-bonadio Dec 19, 2018

ljharb Dec 17, 2018

allan-bonadio Dec 19, 2018

ljharb Dec 19, 2018

allan-bonadio Dec 20, 2018

allan-bonadio Dec 20, 2018

ljharb Dec 17, 2018

allan-bonadio Dec 20, 2018

ljharb Dec 17, 2018

allan-bonadio Dec 19, 2018

ljharb Dec 17, 2018

allan-bonadio Dec 19, 2018

allan-bonadio commented Jan 21, 2019 via email •

edited by ljharb

Loading

ljharb commented Jan 21, 2019

ljharb left a comment

allan-bonadio commented Jan 23, 2019 via email

		@@ -1,14 +1,15 @@
		# `.type() => String\|Function\|null`
		# `.type() => String \| Function \| null`

Docs ab #1947

Docs ab #1947

Conversation

allan-bonadio commented Dec 17, 2018 • edited by ljharb Loading

ljharb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allan-bonadio commented Jan 21, 2019 via email • edited by ljharb Loading

ljharb commented Jan 21, 2019

ljharb left a comment

Choose a reason for hiding this comment

allan-bonadio commented Jan 23, 2019 via email

allan-bonadio commented Dec 17, 2018 •

edited by ljharb

Loading

allan-bonadio commented Jan 21, 2019 via email •

edited by ljharb

Loading