Skip to content
This repository has been archived by the owner on Apr 3, 2023. It is now read-only.

feat(Visible): add Observer #11

Merged
merged 8 commits into from
Oct 23, 2017
Merged

feat(Visible): add Observer #11

merged 8 commits into from
Oct 23, 2017

Conversation

daybrush
Copy link
Member

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.6%) to 68.421% when pulling 251488d on itoolsg:master into 08ca738 on naver:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 70.789% when pulling dc8d3d8 on itoolsg:master into 08ca738 on naver:master.

src/Visible.js Outdated
@@ -55,6 +55,7 @@ class Visible extends Component {

this._targets = [];
this._timer = null;
this._supportObserver = !!window.IntersectionObserver;
Copy link
Contributor

Choose a reason for hiding this comment

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

How about static method?
This code is generated each time you create an instance.

src/Visible.js Outdated
* @param {Boolean} [containment=false] Whether to check only elements that are completely contained within the reference area.<ko>기준 영역 안에 완전히 포함된 엘리먼트만 체크할지 여부.</ko>
* @return {eg.Visible} An instance of a module itself<ko>모듈 자신의 인스턴스</ko>
*/
observe(...params) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about change following code?
use defalut parameters.

observe(delay = -1, containment = false) {
 //            let delay = params[0];
 //		let containment = params[1];
  		  
 //		if (typeof delay !== "number") {
 //			containment = delay;
 //			delay = -1;
 //		}
 //
 //		if (typeof delay === "undefined") {
 //			delay = -1;
 //		}
 //
 //		if (typeof containment === "undefined") {
 //			containment = false;
 //		}

src/Visible.js Outdated
@@ -55,6 +55,7 @@ class Visible extends Component {

this._targets = [];
this._timer = null;
this._supportObserver = !!window.IntersectionObserver;
this._supportElementsByClassName = (() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about static method?
This code is generated each time you create an instance.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.8%) to 70.235% when pulling 4916ad2 on itoolsg:master into 08ca738 on naver:master.

@sculove sculove merged commit 731dc6e into naver:master Oct 23, 2017
@sculove
Copy link
Contributor

sculove commented Oct 23, 2017

LGTM

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

Successfully merging this pull request may close these issues.

None yet

3 participants