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 jQueryUI 1.11 AMD Support #161

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

added jQueryUI 1.11 AMD Support #161

wants to merge 1 commit into from

Conversation

larslaade
Copy link

jQueryUI 1.11.0-beta1 added AMD Support to load all modules via RequireJS.
I've added an AMD wrapper to this plugin to support this change.

│ │ ├── dialog.js
│ │ └── ...
│ ├── jquery.js
| ├── jquery.ui.touch-punch.js
Copy link

Choose a reason for hiding this comment

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

the first | is not the same as

Copy link
Author

Choose a reason for hiding this comment

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

the first is , so you want all those to be ?

for example:

│── index.html
│── js
│   │── app.js
│   │── jquery-ui
│   │   │── accordion.js
│   │   │── autocomplete.js
│   │   │── button.js
│   │   │── core.js
│   │   │── datepicker.js
│   │   │── dialog.js
│   │   │── ...
│   │── jquery.js
│   │── jquery.ui.touch-punch.js
│   └── require.js

Copy link

Choose a reason for hiding this comment

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

The first character of lines 60 and 61 are not alike. 60 is ok, 61 is not.

@larslaade
Copy link
Author

fixed first character of line 61 in readme

@JSteunou
Copy link

+1 this one is soooooo much needed!

@JSteunou
Copy link

Buuuuuut bug spotted.

You should have wrote ui/mouse and not ./jquery-ui/mouse

@theravel
Copy link

theravel commented Mar 4, 2015

@JSteunou according to the doc jquery-ui should be used and it works fine.

@furf When can we get this merged? This does not break BC but helps so much with modern projects with AMD support

@JSteunou
Copy link

JSteunou commented Mar 4, 2015

jquery-ui or ui or whatever alias, but not ./jquery-ui

@JSteunou
Copy link

JSteunou commented Mar 4, 2015

@furf please do not merge it until those require arent fixed.

@larslaade
Copy link
Author

My test setup:

/js/main.js // first js to be loaded via require (so this is the base path)
/js/libs/jquery-ui/ // all jquery-ui stuff
/js/libs/jquery.ui.touch-punch.js

with "jquery-ui/mouse" as a dependency requirejs did not found it, because of the wrong path: "js/jquery-ui/mouse.js"

with "./jquery-ui/mouse" as a dependency all is fine for requirejs, because the path now is relative to touch punch: "js/libs/jquery-ui/mouse.js".

Other setups (with bower or other) maybe don't work with this. I'm open for better suggestions to get this working in all setups.

@JSteunou
Copy link

JSteunou commented Mar 7, 2015

That's why your setup & ./jquery-ui/mouse is working, it's because your files are organized like that.

But you have to consider people using bower for example. Files will be inside a bower_components folder, and each library inside specific folder, maybe inside a dist or lib subfolder, etc.

With require you have to define all your lib path one by one

requirejs.config({
    paths: {
       'jquery-ui': 'bower_components/jquery-ui',
      ...
    }
});

And then all your own code and all your libraries should use require('jquery-ui/mouse');

That's why you should use absolute and not relative path in all jquery plugin that depend on jquery parts.

@theravel was right about quoting the doc he just read to quickly my messages I guess.

@larslaade
Copy link
Author

Thank you @JSteunou for your explanation. I have tested my setup with your config example and it works very well. So i changed it to jquery-ui/mouse.

@JSteunou
Copy link

JSteunou commented Mar 7, 2015

Thank you for this PR and your comprehension.

requirejs.config({
paths: {
// for example with bower
'jquery-ui': 'bower_components/jquery-ui',
Copy link

Choose a reason for hiding this comment

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

My example was not exact (writing from memory). its bower_components/jquery-ui/ui

@larslaade
Copy link
Author

@JSteunou i corrected the path in the README.md

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.

4 participants