-
Notifications
You must be signed in to change notification settings - Fork 286
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
Remove jQuery from app #768
Conversation
Also had to switch to a new context menu plugin. This should get us closer to #590
@@ -14,6 +14,9 @@ module.exports = { | |||
env: { | |||
browser: true | |||
}, | |||
globals: { | |||
basicContext: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 what is basicContext
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the new contextMenu plugin we are using.
.on(`mouseup.${namespace} mouseleave.${namespace}`, () => { | ||
this.stopDragging(); | ||
}); | ||
document.body.addEventListener(`mousemove.${namespace}`, this._mouseMoveHandler); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These also need to be cleaned up inwillDestroy
or willDestroyElement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
willDestroyElement
calls this.stopDragging();
which has the removeEventListener
calls. Did I mess them up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, I see, thank you for explaining!
package.json
Outdated
@@ -36,6 +36,7 @@ | |||
"aws-sdk": "^2.3.3", | |||
"babel-eslint": "^8.2.2", | |||
"babel-plugin-transform-es2015-modules-amd": "^6.24.0", | |||
"basiccontext": "^3.5.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing a comma here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Thanks! Can you please add a screenshot of the new context menu? |
@teddyzeenny I added a screenshot 👍 |
* Remove jQuery from app Also had to switch to a new context menu plugin. This should get us closer to emberjs#590 * Add missing comma
Also had to switch to a new context menu plugin. This should get us closer to #590