Handle CSS dependencies from JS comments

[rxaviers]

As Jörn stated on jquery/jquery-ui#1029 (comment), handle CSS dependencies as discussed.

We've discussed this at the team meeting in Amsterdam. None of the solutions that load CSS through AMD are in a stage where we're willing to commit to them. In other words, specifying CSS dependencies through AMD is far from a 'solved problem', so we're not going to adopt that approach until it works.

Since we want to share the download builder between UI and Mobile as soon as possible, we'll adopt Mobile's approach of specifying CSS dependencies in comments in JS files. We'll port the implementation from their build (or download builder). Mobile used this long enough and never ran into any major problems. Once a better solution emerges, we can delete those comments and adopt the new solution.

The jQuery Mobile download builder already implements this.

[jzaefferer]

Did you get started on this?

PS: I've added a link to the Mobile implementation in the ticket description

[rxaviers]

No, I haven't started. Thank you for the link.

[rxaviers]

First step is to define "Mobile's approach of specifying CSS dependencies in comments in JS files".

It's currently not documented. But, it's not hard to deduce by looking at mobile's source files that it's of the form: //>>css.[shortname]: [relative-path-from-current-file-to-css]

@gseguin (cc @uGoMobi) could you confirm please?

If the above is correct, ui/button.js comment-header would become this. @scottgonzalez @jzaefferer any suggestion?

[jzaefferer]

That looks wrong. "css.button" doesn't match anything currently used in mobile, where you've got "css.structure" and "css.theme". We can use the same pattern in UI, since we also separate structure and theme, and not all components need the theme (like the interactions).

[jzaefferer]

Also the paths are relative from their js root, not from each file, as you can see here: https://github.com/jquery/jquery-mobile/blob/master/js/widgets/panel.js - a path relative to this file would have to go two directories up.

[rxaviers]

That looks wrong. "css.button" doesn't match anything currently used in mobile, where you've got "css.structure" and "css.theme". We can use the same pattern in UI, since we also separate structure and theme, and not all components need the theme (like the interactions).

Ah! I've overlooked that. You are correct. So, it looks like mobile uses "structure" and "theme" as bundle names when building the package.

Also the paths are relative from their js root, not from each file, as you can see here: https://github.com/jquery/jquery-mobile/blob/master/js/widgets/panel.js - a patch relative to this file would have to go two directories up.

You are correct. Although, I think it should be considered a change to either: (a) relative to the current file, or (b) absolute to the project's root. Being relative to the js path seems hard to generalize.

[jzaefferer]

(b) absolute to the project's root.

This seems to me like the better approach, since you won't have to adjust this path when moving files around.

Assuming jQuery Mobile and UI have CSS files in the same folder, they could also keep the path when importing widgets like tabs, instead of having to adjust that to the local file structure.

[arschmitz]

The plan is for mobile to change file structure to match so this sounds good to me

[gseguin]

Shouldn't it be relative to the AMD baseUrl config?

[rxaviers]

Why?

[gseguin]

To be consistent with the AMD modules. I don't have a strong opinion about it, that just seems like another viable alternative.

[rxaviers]

I'm confused. Because on define(), relative paths are relative to the current file.

Anyway, I am just spotting a place that could be improved in my opinion. But, I also don't have a strong opinion about it.

[rxaviers]

@gseguin, @jzaefferer can we import @gseguin's buildCssBundle() code? We'd have to do some tweaks to use the in-memory cached files, and to simplify some dependencies (eg. Project()) in terms of accommodating it to the existing DB environment. But, importing it would be definitely of a great value.

[gseguin]

Feel free to use and/or rewrite any part of that code.

[rxaviers]

Cool! 😀 Thanks. I guess we're good in terms of licensing, right?

[rxaviers]

Per today's meeting, we are not going to change the way mobile is currently handling CSS'es. @gseguin is going to check if it's possible to expose his buildCssBundle() function, so DownloadBuilder can make function calls to obtain the built CSSes.

[rxaviers]

I've been able to use @gseguin's css builder just fine. I am just concerned it makes us think this will drive us towards generalizing Download Builder in a way we can control the styles of the package in an arbitrary way by changing the style directives in the code comments. It's not going to happen unless we change things...

First of all, there's nothing wrong with @gseguin's code. It does what it has to do perfectly (build both UI style bundles: structure and theme in an excellent generic way). The issue is the extra steps we have in the UI package #207.

[rxaviers]

The issue is the extra steps we have in the UI package #207.

Package has been simplified. Not an issue anymore.

[jzaefferer]

So are you able to move ahead with this now?

[rxaviers]

Yes, there are no blockers to use @gseguin's code in order to address this issue. (that I am aware of)

[arschmitz]

This has gone stale, and i'm fairly sure the plan has changed, and there are blockers here.

To update on several related discussions, that seem to have no documentation, which happened since this issue was last updated...

There is a problem that came up just using @gseguin 's module as is. The css dependencies in the comments will map to non existent files, when the widgets are pulled from ui into mobile. To fix this we need something similar to an AMD paths config.

I believe this was discussed between @rxaviers and @gseguin in Chicago, and they decided to use amd for the css, under the hood. To avoid the issues previously discussed around that and forcing devs to use css dep management in their amd, the comments will stay, but they will be parsed into AMD.
That way we can actually use a paths config to solve this, just like we already do for the JS components we are pulling into mobile.

Related but perhaps separate is the need to separate css dependency from JS dependency. This comes up in 2 forms with the new button re-write.

1.) Some components now depend on button CSS, but not the JS, like selectmenu. Selecting selectmenu in the builder should pull in button CSS but not the JS.

2.) Because we now support "CSS Only" buttons and controlgroups, there should be a way in builder, to select button css, with out including the JS widget.

ref #246

[rxaviers]

Hi @arschmitz, yes your overall understanding is correct. We decided not to reinvent the wheel. Under the hoods, we'll transform the CSS comments into AMD definitions and let r.js handle that (via one of its css plugins) to generate the bundles (JS and CSS).

1.) Some components now depend on button CSS, but not the JS, like selectmenu. Selecting selectmenu in the builder should pull in button CSS but not the JS.

Have selectmenu to define its dependency on the button css (e.g., //>>css.structure: ../css/structure/jquery.mobile.button.css)?

2.) Because we now support "CSS Only" buttons and controlgroups, there should be a way in builder, to select button css, with out including the JS widget.

Any suggestion on how this should look like?

[arschmitz]

So, have selectmenu to define its dependency on the button css (e.g., //>>css.structure: ../css/structure/jquery.mobile.button.css)?

As long as this will support multiple comma seperated or whatever files thats fine because it would need to be like //>>css.structure: ../css/structure/jquery.mobile.button.css, ../css/structure/jquery.mobile.selectmenu.css

As far as how the ui would work i'm thinking the css would be its own module maybe? and the main button module would depend on the button-css module? @scottgonzalez I know we talked about this at one point and you agreed in concept that this should be possible. Don't remember if you had a suggestion on exactly how it would work though?

[rxaviers]

Yes, having multiple CSSes is possible.

[scottgonzalez]

I'd suggest an overhaul of the whole page layout. We can add a CSS Components section and rename Components to JS Components. Instead of three columns, we should have the headers full width with their descriptions. The component names and descriptions don't need so much visual spacing, they can just be thrown together with the name being bold and followed by a colon. The Theme section can probably just become part of the CSS Components section. We can probably put JS Components and CSS Components side by side in two columns to prevent the page from becoming super long.

I'm not sure what we'd do about older versions though.

[jzaefferer]

@arschmitz you said on IRC that this needs to land in 1.12. Based on the discussion here since that comment, I'm concerned that this will delay the 1.12 release, potentially quite a lot. Can you explain how this a blocker for Mobile? Are there workarounds?

[arschmitz]

@jzaefferer well part of this is required. The new ui and ability to download seperate css / js for components is NOT required. What is required is the ability to set paths for the css with a r.js config and for mobile to be able to actually use the UI download builder. These from what I understand are not far off ( unlike the new ui ).

[jzaefferer]

Okay, so let's split this into two tasks. One takes care of the underlying infrastructure, the other overhauls the user interface for custom downloads. I'd appreciate if someone else could create that second ticket, since I'm still not completely clear about the exact split.

[arschmitz]

On it!

[rxaviers]

Created #250