Fix 207: Simplify jQuery UI 1.11 package #214

rxaviers · 2014-05-14T01:30:35Z

Closes #207.

Produces the following jQuery UI 1.11 download package:

.
├── images
│   ├── animated-overlay.gif
│   ├── ui-bg_diagonals-thick_18_b81900_40x40.png
│   ├── ui-bg_diagonals-thick_20_666666_40x40.png
│   ├── ui-bg_flat_10_000000_40x100.png
│   ├── ui-bg_glass_100_f6f6f6_1x400.png
│   ├── ui-bg_glass_100_fdf5ce_1x400.png
│   ├── ui-bg_glass_65_ffffff_1x400.png
│   ├── ui-bg_gloss-wave_35_f6a828_500x100.png
│   ├── ui-bg_highlight-soft_100_eeeeee_1x100.png
│   ├── ui-bg_highlight-soft_75_ffe45c_1x100.png
│   ├── ui-icons_222222_256x240.png
│   ├── ui-icons_228ef1_256x240.png
│   ├── ui-icons_ef8c08_256x240.png
│   ├── ui-icons_ffd27a_256x240.png
│   └── ui-icons_ffffff_256x240.png
├── index.html
├── jquery.js
├── jquery-ui.css
├── jquery-ui.js
├── jquery-ui.min.css
├── jquery-ui.min.js
├── jquery-ui.structure.css
├── jquery-ui.structure.min.css
├── jquery-ui.theme.css
└── jquery-ui.theme.min.css

jzaefferer · 2014-05-14T11:14:08Z

app/src/download.js

You could use .toggle( boolean ) here, to avoid the duplicate selectors on the lines below.

This code only runs when the radio buttons are clicked, but it also needs to run on page load. For example, selecting 1.11, then customizing a theme, then coming back, 1.11 is selected but the input is still shown.

Thanks. Fixing...

jzaefferer · 2014-05-14T13:04:15Z

I've tested this a bit with grunt build-app and node server.js --console. Apart from the issue mentioned above, the only thing I'm missing is the demo file (formerly index.html in root). This was useful to verify all the JS and CSS is there and that the theme looks right. Kind of like a MANIFEST with hashes but for manual testing. @scottgonzalez any thoughts on keeping that? We didn't discuss this particular file in #207.

scottgonzalez · 2014-05-14T13:29:26Z

Yeah, we should probably keep that. It's really useful for quick testing and serves as a simple starting point for users.

jzaefferer · 2014-05-14T13:36:25Z

Alright. @rxaviers can you bring that back (the code is still there, anyway) and fix the other issue? I'll do another test run then, including building packages for release script and jqueryui.com.

scottgonzalez · 2014-05-14T13:45:32Z

Another thing I just thought of is whether we should include theme.css so users can easily include multiple themes without duplication.

scottgonzalez · 2014-05-14T16:21:50Z

We'll include structure + theme + concat.

rxaviers · 2014-05-14T20:41:06Z

Fixed. Ready for a new review.

jzaefferer · 2014-05-15T07:53:54Z

To check that 1.11.0-beta.2 will list selectmenu, I update config.json to point at master for the Pre-Release. After running grunt prepare, I still don't get selectmenu in the list of widgets. Since there isn't an error about missing manifest files, why is it showing the other components just fine, but not selectmenu?

My test download for jquery-ui-1.11.0-beta.1.custom didn't include the index.html file, can you add that back?

Looking at the banners, the js and css files are looking good. Would be a lot easier to test that properly with the demo file.

I also logged #216 while testing this, which seems even more useful with the separate structure css files.

rxaviers · 2014-05-15T17:22:13Z

I completely forgot to add index.html back 😳, sorry. Here it is now.

rxaviers · 2014-05-15T17:25:18Z

template/zip/index-1-11.html

I have changed the above paragraph to better describe what we currently do:

@@ -154,7 +50,7 @@ <h1>Welcome to jQuery UI!</h1> <div class="ui-widget"> - <p>This page demonstrates the widgets you downloaded using the theme you selected in the download builder. We've included and linked to minified versions of <a href="js/{{jqueryCore}}">jQuery</a>, your personalized copy of <a href="js/jquery-ui-{{ui.version}}.custom.min.js">jQuery UI (js/jquery-ui-{{ui.version}}.custom.min.js)</a>, and <a href="css/{{theme}}/jquery-ui-{{ui.version}}.custom.min.css">css/{{theme}}/jquery-ui-{{ui.version}}.custom.min.css</a> which imports the entire jQuery UI CSS Framework. You can choose to link a subset of the CSS Framework depending on your needs. </p> + <p>This page demonstrates the widgets you downloaded using the theme you selected in the download builder. We've included and linked to a version of <a href="jquery.js">jQuery (jquery.js)</a>, your personalized <a href="jquery-ui.js">jQuery UI (jquery-ui.js)</a>, and the entire <a href="jquery-ui.css">jQuery UI CSS Framework (jquery-ui.css)</a>.</p> <p>You've downloaded components and a theme that are compatible with jQuery 1.6+. Please make sure you are using jQuery 1.6+ in your production environment.</p> </div>

rxaviers · 2014-05-15T21:56:04Z

To check that 1.11.0-beta.2 will list selectmenu, I update config.json to point at master for the Pre-Release. After running grunt prepare, I still don't get selectmenu in the list of widgets. Since there isn't an error about missing manifest files, why is it showing the other components just fine, but not selectmenu?

Interesting. Mine did included it just fine. A couple of differences I used: origin/master instead of master (but, both should work).

jzaefferer · 2014-05-16T11:57:30Z

I tried again with origin/master, that worked. Did some more test downloads, with all components and just selectmenu (and dependencies) and standard and custom theme.

The only thing missing is a selectmenu demo in index.html. Everything else looks good.

jzaefferer · 2014-05-16T11:58:16Z

I've fixed the missing demo. Could leave that commit as-is or squash it into something else. Will do some more testing with jqueryui.com and the release script...

jzaefferer · 2014-05-16T12:00:17Z

Found an unrelated issue, reported in #218.

jzaefferer · 2014-05-16T16:06:07Z

My local installation of jqueryui.com is messed up, it takes hours to deploy to the site and I have no idea why.

I also tried to test this as part of the release script. In my standalone script, building the CDN package just stopped without any error. It took a while to try the same with the regular script, but that failed with the same issue, whatever it actually is.

@rxaviers can you try to debug this? Go to your jquery-ui repo and put this in build/test-release.js:

var shell = require( "shelljs" );
var Release = {
    exec: shell.exec,
    abort: function( msg ) {
        console.error( msg );
        process.exit( 1 );
    },
    define: function( definitions ) {
        for ( var key in definitions ) {
            Release[ key ] = definitions[ key ];
        }
    },
    newVersion: "1.11.0-beta2"
};
var script = require("./release");
script( Release );
Release.generateArtifacts(function() {
    console.log( "Done generating artifacts", arguments );
});

Link DB (npm link download.jqueryui.com) and install shelljs (npm install shelljs@0.2.6), then run the script with node build/test-release.js. The last line logged should be "Building CDN package", afterwards it just stops without any error.

When I tried to track this down I found that it gets until here: https://github.com/jquery/download.jqueryui.com/blob/fix-207-simplify-package/lib/themes-packer.js#L64

build.commonFiles is undefined, which causes this to fail, but I have no idea why it is undefined or why there is no output.

rxaviers · 2014-05-16T16:31:14Z

@jzaefferer thanks for extensively testing this branch and for finding (and adding) the missing selectmenu demo.

As for the release script, it won't be able to use the simpler builder to build non-simpler CDN packages. We need discuss how we want CDN packages to be. Is it going to reflect our simpler download package changes?

rxaviers · 2014-05-17T00:31:06Z

@jzaefferer, d3a75eb reverts the pieces of builder 1.11 that can't go away, because it's used by ThemePacker (used by cdn package). Now, release script should work just fine.

rxaviers · 2014-05-17T00:36:11Z

I've created a separate ticket #219, so we create unit tests for ThemePacker.

rxaviers · 2014-05-17T00:38:16Z

Updated description to include the .structure, .theme, and index.html files.

jzaefferer · 2014-05-19T10:58:42Z

Tested this again, with the local server, release script and deployment to local jqueryui.com. My local site isn't working that well (I can't test WordPress /resources/ properly). But based on the files in the filesystem etc. everything looks good now. Let's land this and do some more testing on stage, then we should be good to release beta.2.

Closes gh-207

rxaviers · 2014-05-19T16:39:14Z

All squashed

Closes gh-207 gh-214

Drop unused variables. Amend 462b732 Ref gh-207 gh-214

Amend 462b732 Ref gh-207 gh-214

rxaviers · 2014-05-19T17:57:38Z

On http://stage.jqueryui.com/

jzaefferer · 2014-05-20T09:16:14Z

Looks good.

jzaefferer reviewed May 14, 2014
View reviewed changes

rxaviers reviewed May 15, 2014
View reviewed changes

Packer: Simplify jQuery UI 1.11 package

f5d25e4

Closes gh-207

rxaviers added a commit that referenced this pull request May 19, 2014

Packer: Simplify jQuery UI 1.11 package

462b732

Closes gh-207 gh-214

rxaviers closed this May 19, 2014

rxaviers deleted the fix-207-simplify-package branch May 19, 2014 16:42

rxaviers added a commit that referenced this pull request May 19, 2014

Packer: Simplify jQuery UI 1.11 package

f91b6b2

Drop unused variables. Amend 462b732 Ref gh-207 gh-214

rxaviers added a commit that referenced this pull request May 19, 2014

Test: Add JqueryUiFiles < 1.11 tests back

5ab0c06

Amend 462b732 Ref gh-207 gh-214

ferda-mravenec mentioned this pull request Dec 16, 2020

Themes: Start, Trontastic, Vader #569

Open

Fix 207: Simplify jQuery UI 1.11 package #214

Fix 207: Simplify jQuery UI 1.11 package #214

Uh oh!

Conversation

rxaviers commented May 14, 2014

Uh oh!

jzaefferer May 14, 2014

Choose a reason for hiding this comment

Uh oh!

jzaefferer May 14, 2014

Choose a reason for hiding this comment

Uh oh!

rxaviers May 14, 2014

Choose a reason for hiding this comment

Uh oh!

jzaefferer commented May 14, 2014

Uh oh!

scottgonzalez commented May 14, 2014

Uh oh!

jzaefferer commented May 14, 2014

Uh oh!

scottgonzalez commented May 14, 2014

Uh oh!

scottgonzalez commented May 14, 2014

Uh oh!

rxaviers commented May 14, 2014

Uh oh!

jzaefferer commented May 15, 2014

Uh oh!

rxaviers commented May 15, 2014

Uh oh!

rxaviers May 15, 2014

Choose a reason for hiding this comment

Uh oh!

rxaviers commented May 15, 2014

Uh oh!

jzaefferer commented May 16, 2014

Uh oh!

jzaefferer commented May 16, 2014

Uh oh!

jzaefferer commented May 16, 2014

Uh oh!

jzaefferer commented May 16, 2014

Uh oh!

rxaviers commented May 16, 2014

Uh oh!

rxaviers commented May 17, 2014

Uh oh!

rxaviers commented May 17, 2014

Uh oh!

rxaviers commented May 17, 2014

Uh oh!

jzaefferer commented May 19, 2014

Uh oh!

rxaviers commented May 19, 2014

Uh oh!

rxaviers commented May 19, 2014

Uh oh!

jzaefferer commented May 20, 2014

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants