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

DEV: Upgrade Ember to 3.15 and remove jQuery and lodash as dependencies #107

Merged
merged 4 commits into from Feb 7, 2020
Merged

DEV: Upgrade Ember to 3.15 and remove jQuery and lodash as dependencies #107

merged 4 commits into from Feb 7, 2020

Conversation

OsamaSayegh
Copy link
Member

No description provided.

const MOVE_EVENTS = ["touchmove", "mousemove"];
const UP_EVENTS = ["touchend", "mouseup"];
const DOWN_EVENTS = ["touchstart", "mousedown"];

function bind(target, key, desc) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This decorator is a small experiment I did that I'd like someone to review. It solves the problem where we have a component method used as an event handler that we need to later remove when the component is destroyed, and we want this inside the method to refer to the component and not the DOM element the event is attached to.

Currently I'm doing this:

endDrag() {
  if (this._endDrag) return this._endDrag;

  const handler = () => {
    this.set("clicked", true);
    console.log("Element clicked!");
  };
  this.set("_endDrag", handler);
  return this._endDrag;
}

and in didInsertElement:

document.addEventListener("click", this.endDrag());

and in willDestroyElement:

document.removeEventListener("click", this.endDrag());

We have a few more event handlers in this component so this can get a bit repetitive.

This decorator allows us to do something like this:

@bind
endDrag() {
  this.set("clicked", true);
  console.log("Element clicked!");
}
// later
document.addEventListener("click", this.endDrag);
document.removeEventListener("click", this.endDrag);

Any objections to this?

Copy link
Member

Choose a reason for hiding this comment

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

I am probably not the best person to review this perhaps @jjaffeux or @CvX can have a look (and at the entire change) ... overall I am super happy with all of this

@SamSaffron
Copy link
Member

I say merge away, the odds of having problems are not enormous and we can catch them once it is deployed.

@OsamaSayegh OsamaSayegh merged commit 9f33639 into discourse:master Feb 7, 2020
@OsamaSayegh OsamaSayegh deleted the ember-upgrade branch February 7, 2020 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants