Skip to content
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

added tap / click elements example to readme #55

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

added tap / click elements example to readme #55

wants to merge 2 commits into from

Conversation

edencorbin
Copy link

It took me a little to figure out how to implement the click / tap functionality in react so I thought it would be helpful to add react specific code to the documentation. Let me know if you think so too.

@schovi
Copy link
Owner

schovi commented May 5, 2017

@edencorbin Is it anyhow different when you use it without react-iscroll? If yes, then it makes sense and I will provide some feedback on code itself :)

@edencorbin
Copy link
Author

Well, although it is not technically different, it requires setting up a ref, which is a react concept to my knowledge, and then adding a listener event to that ref. I'm a fan of verbose documentation. I noticed there was another closed issue or two about it so I thought I would share the steps. If it doesn't fit in with the documentation, no problem.

@schovi
Copy link
Owner

schovi commented May 6, 2017

@edencorbin My point is, that this is something many times solved and when you google you find same solution or package solving that like https://github.com/zilverline/react-tap-event-plugin
So maybe find best solution and just link it in FAQ?

Copy link
Owner

@schovi schovi left a comment

Choose a reason for hiding this comment

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

There is some better practises how to make it clean and durable.

}}
>
<div>
<div id={0} ref={el => this.refDict[0] = el}>
Copy link
Owner

Choose a reason for hiding this comment

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

With simple example like this you can save ref directly.

ref={el => this.myElementRef = el}

}

componentDidMount() {
this.refDict[0].addEventListener("tap", () => this.handleTap(this.refDict[0]));
Copy link
Owner

Choose a reason for hiding this comment

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

With comlicated code it would be better to check presence of element

if (this.myElementRef) {
  // add listener
}

}

componentDidMount() {
this.refDict[0].addEventListener("tap", () => this.handleTap(this.refDict[0]));
Copy link
Owner

Choose a reason for hiding this comment

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

With binded handler to class you can do simple

.addEventListener("tap", this.handleTap)

this.refDict[0].addEventListener("tap", () => this.handleTap(this.refDict[0]));
}

handleTap(e) {
Copy link
Owner

Choose a reason for hiding this comment

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

Bind tap handler in constructor:

  this.handleTap = this.handleTap.bind(this)

or use class props syntax:

handleTap = (ev) => {
 ....
}

Then you dont need to call it () => this.handleTap()

componentDidMount() {
this.refDict[0].addEventListener("tap", () => this.handleTap(this.refDict[0]));
}

Copy link
Owner

Choose a reason for hiding this comment

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

And when you add listener, you have to remove it too

componentWillUnmount() {
  if(this.myElementRef) {
    this.myElementRef.removeEventListener("tap", this.handleTap)
  }
}

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.

2 participants