Use "ancestor" instead of "parent" #20
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The term "parent" means that the node is the direct ancestor of the descendant component (child). This is not necessarily the case in this code, so I am changing the terminology to use the more general and accurate "ancestor".
ESLint pointed out a number of small improvements, all of which I make in this commit. These changes slightly improve the readability of the code.
This ES6 syntax is slightly nicer than the old way of doing this, and more consistent with the Brigade codebase.
Our tests were failing in Travis CI with the following error: > Definition for rule 'computed-property-spacing' was not found. I think that bumping the ESLint version will resolve this, and I figured I'd just bump everything up to the latest stable versions.
| // | ||
| // Cannot read property 'removeEventListener' of undefined | ||
| this.scrollableParent.removeEventListener('scroll', this._handleScroll); | ||
| this.scrollableAncestor.removeEventListener('scroll', this._handleScroll); | ||
| window.removeEventListener('resize', this._handleScroll); |
There was a problem hiding this comment.
Shouldn't the removal of the resize listener from the window always occur, regardless of whether or not the scrollableAncestor is mounted?
There was a problem hiding this comment.
Ah yes, I think so. Good call.
lencioni
added a commit
that referenced
this pull request
Aug 17, 2015
Use "ancestor" instead of "parent"
lencioni
added a commit
that referenced
this pull request
Aug 17, 2015
As eagle-eyed @jeffkole pointed out [0], we always want to remove this event listener, even if the scrollableAncestor does not exist. [0]: #20 (comment)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The term "parent" means that the node is the direct ancestor of the
descendant component (child). This is not necessarily the case in this
code, so I am changing the terminology to use the more general and
accurate "ancestor".
I also made some minor changes that ESLint pointed out.