Use AMD bootstrap in demos #1557
Conversation
| <link rel="stylesheet" href="../demos.css"> | ||
| <script src="../require-config.js"></script> | ||
| <script src="../../external/requirejs/require.js"></script> |
There was a problem hiding this comment.
Let's clean up the mixing of <script> and <style> as well?
There was a problem hiding this comment.
+1 i thought about doing this but wanted to leave as is until i asked
|
Overall looking good. Merging the two bootstrap files will make it even better. |
|
@jzaefferer addressed your comments |
|
Nice, looks good to me. |
|
I've pushed this branch to |
|
What's the status of this? |
|
@scottgonzalez Waiting for your review |
|
|
||
| require( modules, function() { | ||
| $( "body" ).css( "visibility", "visible" ); | ||
| eval( $( script ).html() ); |
There was a problem hiding this comment.
It might be better to use $.globalEval() here.
|
This is now update to cover all the demos |
| "effect-shake", | ||
| "effect-size", | ||
| "effect-slide" | ||
| ]; |
There was a problem hiding this comment.
I'm worried about this getting out of date (though honestly, I don't anticipate more effects). I suppose it will be obvious if this gets out of date though. Either the new effect just won't work in the demos or the demos will throw an error if we delete an effect.
There was a problem hiding this comment.
If we added a new effect it would also throw an error when you tried to run it if it was missing from this list.
|
None of the demos work for me. I load them, for example, as http://localhost/jquery-ui/demos/accordion/default.html It fails to load http://localhost/external/jquery/jquery.js - note the missing "jquery-ui" part in the path. I wanted to test |
|
Opening the globalize-dependent spinner demos causes errors when loading the locale files, e.g. demos/spinner/decimal.html throws "Uncaught TypeError: Cannot read property 'addCultureInfo' of undefined" in both globalize.culture.de-DE.js:28 and globalize.culture.ja-JP.js:28 The error doesn't always happen, so I guess sometimes they load in the "right" order, but mostly, I guess, this is failing due to missing dependencies from the culture files to the main globalize file. |
|
@jzaefferer thats odd works fine for me i originally had this problem but it should be fixed by |
|
That link points at a datepicker demo diff, probably not what you intended. I guess you're pointing at the shim in bootstrap.js - that makes sense, though I've never seen it uses with a wildcard. If I simulate a "slower" network ("Good 3G" is bad enough), I always get the error. |
|
Oh, just saw your new commit, that should fix it. |
Also fixes shim for globalize
|
Still need to add |
|
@jzaefferer will do |
|
@jzaefferer updated |
| @@ -0,0 +1,60 @@ | |||
| /* globals window:true, document:true, $:true */ | |||
There was a problem hiding this comment.
That's also why the Travis build failed.
|
@jzaefferer updated to remove |
|
Tested this properly, with the 1.12 DB changes and jquery/jqueryui.com#113. Still need to adjust the demo iframe and font-size, but that's a separate issue I'll deal with. For now, let's land this (and update core-brekaup on top) and land jquery/jqueryui.com#113 in a 1-12 branch. I'll then update the 1-12 branch with the font-size fix, along with jquery-wp-content (probably also a ui-1-12 branch). |
|
ping @arschmitz - let's finish this, as outlined above, will also massively simplify the demos-half of the core-breakup. |
Fixes #10119 Closes jquerygh-1557
Fixes #10119 Closes jquerygh-1557
Fixes #10119 Closes jquerygh-1557
This adds an AMD bootstrap based off the one we use in tests for loading demos. For now this just switches the accordion and datepicker demos to cover the different use cases we have. PR for jqueryui.com to properly handle new demo layout coming shortly.
It also removes the accordion hover intent demo and a ie 6 /7 specific work around in the accordion demos