-
Notifications
You must be signed in to change notification settings - Fork 53
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
Waves on aurelia-cli project #238
Conversation
symptoms: in aurelia-cli waves is double attached and not working as expected. This fix check if bridge is running with global requirejs. If so, waves is already attached to elements.
I'm still a little confused by this comment: #168 (comment) |
yeah.. maybe wait until latest cli can be tested better. Worked with an earlier version. |
@@ -34,7 +34,9 @@ export class MdWaves { | |||
} | |||
|
|||
this.attributeManager.addClasses(classes); | |||
Waves.attach(this.element); | |||
if (!(typeof window.require === 'function' && typeof window.require.specified === 'function' && window.require.specified('waves'))) { |
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.
In my build, window.require
is not defined but instead window.requirejs
.. Is it a typo or does that actually work in your build?
Also: What do you think of a build condition instead? Since it's only required with AMD modules we could simply add it to AMD build (via a gulp "remove" task).
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.
Any idea why this works at all with elements added after DOMContentLoaded
? I still don't get it (had a look at the fork's changes already).
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.
I am not sure why it works. Its just an observation.
window.require vs window.requirejs.. maybe it has something to do how materialize amd is build?
The remove task sound interesting 😄
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.
In my project both window.require and window.requirejs is defined. I will add a commit in the branch and change it to window.requirejs so it can be tested more.
i merged and did a gulp build-release in:
npm install github:ullfis/aurelia-materialize-bridge#build-branch --save
Only change from your master is the waves-amd-check branch.
instead of window.require
tested on aurelia-cli, webpack and the sample project. Works (on my computer) 🙈 |
Took the AMD build condition route. 😄 |
Checking if the bridge is running with global requirejs and only attach waves if not, makes it work with aurelia-cli. Also tested with webpack and jspm.