Skip to content

Commit

Permalink
Grunt: remove base theme from release cdn
Browse files Browse the repository at this point in the history
  • Loading branch information
rxaviers committed Mar 14, 2013
1 parent 054449e commit 38fce3f
Showing 1 changed file with 2 additions and 9 deletions.
11 changes: 2 additions & 9 deletions Gruntfile.js
Expand Up @@ -253,9 +253,7 @@ grunt.initConfig({
"dist/jquery-ui.js": "jquery-ui.js",
"dist/jquery-ui.min.js": "jquery-ui.min.js",
"dist/i18n/jquery-ui-i18n.js": "i18n/jquery-ui-i18n.js",
"dist/i18n/jquery-ui-i18n.min.js": "i18n/jquery-ui-i18n.min.js",
"dist/jquery-ui.css": "themes/base/jquery-ui.css",
"dist/jquery-ui.min.css": "themes/base/minified/jquery-ui.min.css"
"dist/i18n/jquery-ui-i18n.min.js": "i18n/jquery-ui-i18n.min.js"
},
dest: "dist/<%= files.cdn %>"
},
Expand All @@ -274,11 +272,6 @@ grunt.initConfig({
strip: /^dist\/minified/,
dest: "dist/<%= files.cdn %>/ui"
},
cdn_min_images: {
src: "themes/base/images/*",
strip: /^themes\/base\//,
dest: "dist/<%= files.cdn %>/themes/base/minified"
},
cdn_themes: {
src: "dist/<%= files.themes %>/themes/**/*",
strip: "dist/<%= files.themes %>",
Expand Down Expand Up @@ -380,6 +373,6 @@ grunt.registerTask( "sizer_all", [ "concat:ui", "uglify", "compare_size" ] );
grunt.registerTask( "build", [ "concat", "uglify", "cssmin", "copy:dist_units_images" ] );
grunt.registerTask( "release", "clean build copy:dist copy:dist_min copy:dist_min_images copy:dist_css_min md5:dist zip:dist".split( " " ) );
grunt.registerTask( "release_themes", "release generate_themes copy:themes md5:themes zip:themes".split( " " ) );
grunt.registerTask( "release_cdn", "release_themes copy:cdn copy:cdn_min copy:cdn_i18n copy:cdn_i18n_min copy:cdn_min_images copy:cdn_themes md5:cdn zip:cdn".split( " " ) );
grunt.registerTask( "release_cdn", "release_themes copy:cdn copy:cdn_min copy:cdn_i18n copy:cdn_i18n_min copy:cdn_themes md5:cdn zip:cdn".split( " " ) );

};

21 comments on commit 38fce3f

@Seldaek
Copy link

Choose a reason for hiding this comment

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

I read the release notes saying to use smoothness instead of the base theme, but it does not have individual files like the base theme had. We include only themes/base/jquery.ui.slider.css since that's all we need, and this file is not available in smoothness. Any recommendation?

I also think doing such changes in patch releases should not be allowed.

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

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

So you're pulling individual CSS files from a CDN?

@etiennetremel
Copy link

Choose a reason for hiding this comment

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

Why don't you redirect the base theme to the smoothness one? A large amount of websites are broken since you did it.

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

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

We don't have that kind of control over the 3rd party CDNs. All sites that are broken from this change are doing something bad (linking to an untested, variable version of jQuery UI).

@StanAngeloff
Copy link

Choose a reason for hiding this comment

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

I also think doing such changes in patch releases should not be allowed.

I was also surprised to find a breaking change in a patch release. I am relying on jquery.ui.resizable.css alone and I don't really care for anything else. I am forced to pull in the entire smoothness theme to get something working. Very annoying.

@StanAngeloff
Copy link

Choose a reason for hiding this comment

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

So you're pulling individual CSS files from a CDN?
@scottgonzalez

Seems to have affected Twitter Bower installs as well:
https://github.com/components/jqueryui/tree/1.10.2/themes

More of an issue on their end really.

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

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

I was also surprised to find a breaking change in a patch release. I am relying on jquery.ui.resizable.css alone and I don't really care for anything else. I am forced to pull in the entire smoothness theme to get something working. Very annoying.

Don't use the CDN. You're better off just putting this in a local concatenated + minified file anyway.

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

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

Seems to have affected Twitter Bower installs as well

We don't control that in any way. They should fix their code.

@vendethiel
Copy link

Choose a reason for hiding this comment

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

Seems to have affected Twitter Bower installs as well:
https://github.com/components/jqueryui/tree/1.10.2/themes

More of an issue on their end really.

This package is maintained through the "Quick downloads: Stable (Themes)" links on jquery-ui's website. Are you telling me that these links point to "an untested, variable version of jQuery UI" ?

@StanAngeloff
Copy link

Choose a reason for hiding this comment

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

@Nami-Doc: I think what has happened, at least in my case, is that I was relying on a file in the themes/base/ directory which is now gone from the CDN release. Since you are maintaining by downloading from the stable copies, my Twitter Bower installation broke.

I understand why the base theme was removed, it doesn't really need to be in a CDN release. I am now linking Bower against jquery/jquery-ui which has all the files I need.

@Seldaek's case is probably different from mine.

@vendethiel
Copy link

Choose a reason for hiding this comment

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

I'm not even talking about base theme. This is in the changelog. I'm talking about separate css files.

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

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

@StanAngeloff It sounds like you want to be working with the development bundle, which is available from the quick download, but isn't included in Bower.

@Nami-Doc

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

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

This package is maintained through the "Quick downloads: Stable (Themes)" links on jquery-ui's website. Are you telling me that these links point to "an untested, variable version of jQuery UI" ? @Nami-Doc

No, but if a production site broke as a result of this, then that site was running against an untested, variable version of jQuery UI. This is the case for a large amount of sites. Google stupidly encourages this with the /1/ URLs.

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

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

I'm not even talking about base theme. This is in the changelog. I'm talking about separate css files. @Nami-Doc

I'm honestly not even sure what you're responding to. Individual theme files exist in the development bundle.

@vendethiel
Copy link

Choose a reason for hiding this comment

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

The separate CSS files weren't from the dev bundle. They were from the themes zip

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

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

The separate CSS files weren't from the dev bundle. They were from the themes zip

They don't belong there, which is why we removed the base theme. The themes zip is a collection of our processed, named themes. The base theme is not that.

@vendethiel
Copy link

Choose a reason for hiding this comment

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

Well then my apologies to the Bower users using the jQuery-UI package.

@Seldaek
Copy link

Choose a reason for hiding this comment

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

@StanAngeloff I think our case was similar. We grabbed it via bower, and linked against base/jquery.ui.slider.css. I guess we'll also switch to jquery/jquery-ui as source then, unless @Nami-Doc fixes the bower package to contain everything (now whether that would make sense or not, I am not sure).

@vendethiel
Copy link

Choose a reason for hiding this comment

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

I do think that makes sense. If you have a solution for me to avoid downloading each theme one by one, I'll gladly do it.

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

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

@Nami-Doc You can still use the themes zip, but get the base theme from the code zip. If you have questions, the team hangs out in #jqueryui-dev on freenode.

@dgwyer
Copy link

@dgwyer dgwyer commented on 38fce3f May 17, 2016

Choose a reason for hiding this comment

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

Does anyone know why the base theme was removed because on the theme roller gallery page the base theme doesn't match the smoothness theme at all:

http://jqueryui.com/themeroller/

I really like the clean look of the base theme here. Is there any way this can get included back in the CDN download?

Please sign in to comment.