Add css-dependency-comments based on jQuery Mobile #1440

rxaviers · 2015-01-29T18:55:42Z

Ref #1029
Ref jquery/download.jqueryui.com#178

rxaviers · 2015-01-29T19:00:14Z

Need someone to review the CSS references. Don't worry with the paths for now. It's important that all CSS files are being referenced and they are correct (each JS is correctly referencing its CSS'es).

jzaefferer · 2015-01-30T08:41:26Z

ui/core.js

@@ -13,6 +13,9 @@
 //>>description: The core of jQuery UI, required for all interactions and widgets.
 //>>docs: http://api.jqueryui.com/category/ui-core/
 //>>demos: http://jqueryui.com/
+//>>css.structure: ../themes/base/base.css


Why would we reference this anywhere? Its just a bunch of @import statements.

True, removed by ed74d55

rxaviers · 2015-01-30T10:36:12Z

@jzaefferer said:

Out of scope of this PR, but would be good to figure out: Both draggable.css and sortable.css are there simpy to define touch-action: none; for handles. We could move this to core.css as a generic class. @tjvantoll @scottgonzalez thoughts?

I'm repeating his question here, because it will get lost as soon as I push an update.

scottgonzalez · 2015-01-30T12:23:29Z

Regarding touch-action: none;, I think we'd want to wait a bit and see how it's being used in the wild, especially with Chassis.

jzaefferer · 2015-01-30T12:52:08Z

Wait and see is fine to me. With the (more) explicit dependencies in place, it'll be reasonable to change.

arschmitz · 2015-01-30T12:59:33Z

So in general things that don't actually depend on a css file should not depend on it in comments here either I would say. Any other use kinda breaks the meaning of "dependency".

rxaviers · 2015-01-30T14:06:22Z

Removed theme.css from all effects.

arschmitz · 2015-01-30T14:06:27Z

There is no agreement on how button CSS should be defined yet.

I don't think this is true at all actually. What I said was that we discussed, and any widget which depends on it, will include it explicitly. This is not yet an issues however as no widget does. It is only an issue once the re-write for button lands and i will update that PR once this one lands.

rxaviers · 2015-01-30T14:09:12Z

any widget (that) depends on (button.css) will include it explicitly ... once the re-write for button lands.

Ok.

rxaviers · 2015-01-31T19:56:51Z

Based on the conversation we had so far, below is the CSS dependency list I understood for each ui/*js.

UI Core

core.js: none
widget.js: none
mouse.js: none
position.js: none

Interactions

draggable.js: draggable.css
droppable: none
resizable.js: core.css, resizable.css, theme.css
selectable.js: selectable.css
sortable.js: sortable.css

Widgets

accordion.js: core.css, accordion.css, theme.css
autocomplete.js: core.css, autocomplete.css, theme.css
button.js: core.css, button.css, theme.css
datepicker.js: core.css, datepicker.css, theme.css
dialog.js: core.css, dialog.css, theme.css
menu.js: core.css, menu.css, theme.css
progressbar.js: core.css, progressbar.css, theme.css
selectmenu.js: core.css, selectmenu.css, theme.css
slider.js: core.css, slider.css, theme.css
spinner.js: core.css, spinner.css, theme.css
tabs.js: core.css, tabs.css, theme.css
tooltip.js: core.css, tooltip.css, theme.css

Effects

effect*: none

@scottgonzalez @jzaefferer @arschmitz please, can you confirm?

jzaefferer · 2015-02-01T11:31:08Z

Droppable is missing, datepicker is listed twice.

Do interactions actually depend on core.css? I don't think so.

rxaviers · 2015-02-01T11:44:23Z

Droppable is missing

There's no droppable.css. Does it still depend on core.css? Or no CSS at all?

rxaviers · 2015-02-01T11:44:52Z

datepicker is listed twice.

I've updated my above comment removing the duplicate.

jzaefferer · 2015-02-01T20:10:14Z

You've listed other components with "none", so it makes sense to list droppable as well. Afaik none of the interactions except resizable need core.css.

arschmitz · 2015-02-01T20:12:43Z

@jzaefferer @rxaviers that had been my understanding on interactions as well. Only resizable needs it for the icon.

rxaviers · 2015-02-01T20:13:09Z

Great, thank you @jzaefferer and @arschmitz. I've updated my above comment listing droppable and updating other interactions.

jzaefferer · 2015-02-01T20:26:10Z

Looks good to me

rxaviers · 2015-02-02T15:56:47Z

Please, could you re-check?

scottgonzalez · 2015-02-02T16:00:21Z

Looks correct.

rxaviers · 2015-02-02T16:08:30Z

Great thanks

- Handle CSS dependencies from JS comments (based on jQuery Mobile); - Use archive-packager, builder-amd and builder-jquery-css to build UI 1.12.0 download package; Fixes #178 Fixes #255 Ref jquery/jquery-ui#1440

rxaviers · 2015-02-04T14:53:12Z

Updated

rxaviers · 2015-02-04T17:15:19Z

https://gist.github.com/rxaviers/01ab3bc43bf9f42c769f can be used to test building the CSS bundle.

jzaefferer · 2015-02-05T11:59:23Z

ui/spinner.js

@@ -12,6 +12,8 @@
 //>>description: Displays buttons to easily input numbers via the keyboard or mouse.
 //>>docs: http://api.jqueryui.com/spinner/
 //>>demos: http://jqueryui.com/spinner/
+//>>css.structure: ../themes/base/spinner.css


Missing core.css

jzaefferer · 2015-02-05T12:02:52Z

Can you rebase this on master?

Ref #1029

rxaviers · 2015-02-05T12:03:58Z

Rebased

jzaefferer · 2015-02-05T12:07:08Z

Looks like the structure.css file is just concatenated, that is, all the header comments are still there. Do we keep those and leave them to the minifier to remove?

rxaviers · 2015-02-05T12:13:56Z

Looks like the structure.css file is just concatenated, that is, all the header comments are still there. Do we keep those and leave them to the minifier to remove?

The scope of this PR is making sure CSS dependencies are being correctly defined. Although, that's a good question for jquery/download.jqueryui.com#178 (or its PR jquery/download.jqueryui.com#256). Currently, the individual banners are stripped out when they are concatenated. So, we can keep doing it unless we don't want to worry about it anymore.

jzaefferer · 2015-02-05T17:29:00Z

Seems fine to ignore that. Unless someone else wants to review this as well, this is good to land.

rxaviers · 2015-02-11T18:19:07Z

On today's meeting, the decision was to drop theme.css as a dependency for all JS components. Whether to include it or not will be a manual decision made by the developer/user completely independent of the used JS components. The structural CSSes are still kept (listed as dependencies for a JS component) though.

More details can be found on http://irc.jquery.org/%23jquery-meeting/default_%23jquery-meeting_20150211.log.html

jzaefferer · 2015-02-11T18:30:48Z

On today's meeting, the decision was to drop theme.css as a dependency for all JS components.

I don't think that was the consensus. As I understood it: We keep the comments in the JS files. We do nothing with them, for now. We have the option to warn the user, using the DB UI, when they're selecting components that soft-depend on the theme while no theme is selected. The latter should be filed as a separate ticket.

scottgonzalez · 2015-02-11T18:37:13Z

@jzaefferer Correct, that's what was decided.

rxaviers · 2015-02-11T18:52:13Z

Ok, so #1440 (comment) will be kept.

scottgonzalez · 2015-02-23T23:03:50Z

So I think this is correct. @jzaefferer @arschmitz @rxaviers Can you all confirm?

rxaviers · 2015-02-24T12:23:01Z

Confirmed. The changes are correct according to what we have defined.

jzaefferer · 2015-02-24T14:25:00Z

👍

- Handle CSS dependencies from JS comments (based on jQuery Mobile); - Use archive-packager, builder-amd and builder-jquery-css to build UI 1.12.0 download package; Fixes #178 Fixes #255 Ref jquery/jquery-ui#1440

- Handle CSS dependencies from JS comments (based on jQuery Mobile); - Use node-packager, builder-amd, builder-jquery-css, and jquery-ui-themeroller to build UI 1.12.0 download package; - Cache support; Fixes #178 Fixes #255 Ref jquery/jquery-ui#1440

jzaefferer reviewed Jan 30, 2015
View reviewed changes

rxaviers mentioned this pull request Feb 4, 2015

Packer: Support building 1.12.0 package jquery/download.jqueryui.com#256

Closed

9 tasks

rxaviers mentioned this pull request Feb 4, 2015

Bug: using appDir and CSSes that beggin with "../" rxaviers/builder-amd-css#1

Closed

jzaefferer reviewed Feb 5, 2015
View reviewed changes

Various: Add css-dependency-comments based on jQuery Mobile

edf7dc4

Ref #1029

rxaviers force-pushed the ref-1029-css-dependency-comments branch from c24f6f0 to edf7dc4 Compare February 5, 2015 12:03

jquerybot added CLA: Valid and removed CLA: Error labels Feb 6, 2015

rxaviers closed this in 45744ef Feb 26, 2015

scottgonzalez deleted the ref-1029-css-dependency-comments branch March 18, 2015 00:44

Add css-dependency-comments based on jQuery Mobile #1440

Add css-dependency-comments based on jQuery Mobile #1440

Uh oh!

Conversation

rxaviers commented Jan 29, 2015

Uh oh!

rxaviers commented Jan 29, 2015

Uh oh!

jzaefferer Jan 30, 2015

Choose a reason for hiding this comment

Uh oh!

rxaviers Jan 30, 2015

Choose a reason for hiding this comment

Uh oh!

rxaviers commented Jan 30, 2015

Uh oh!

scottgonzalez commented Jan 30, 2015

Uh oh!

jzaefferer commented Jan 30, 2015

Uh oh!

arschmitz commented Jan 30, 2015

Uh oh!

rxaviers commented Jan 30, 2015

Uh oh!

arschmitz commented Jan 30, 2015

Uh oh!

rxaviers commented Jan 30, 2015

Uh oh!

rxaviers commented Jan 31, 2015

Uh oh!

jzaefferer commented Feb 1, 2015

Uh oh!

rxaviers commented Feb 1, 2015

Uh oh!

rxaviers commented Feb 1, 2015

Uh oh!

jzaefferer commented Feb 1, 2015

Uh oh!

arschmitz commented Feb 1, 2015

Uh oh!

rxaviers commented Feb 1, 2015

Uh oh!

jzaefferer commented Feb 1, 2015

Uh oh!

rxaviers commented Feb 2, 2015

Uh oh!

scottgonzalez commented Feb 2, 2015

Uh oh!

rxaviers commented Feb 2, 2015

Uh oh!

rxaviers commented Feb 4, 2015

Uh oh!

rxaviers commented Feb 4, 2015

Uh oh!

jzaefferer Feb 5, 2015

Choose a reason for hiding this comment

Uh oh!

jzaefferer commented Feb 5, 2015

Uh oh!

rxaviers commented Feb 5, 2015

Uh oh!

jzaefferer commented Feb 5, 2015

Uh oh!

rxaviers commented Feb 5, 2015

Uh oh!

jzaefferer commented Feb 5, 2015

Uh oh!

rxaviers commented Feb 11, 2015

Uh oh!

jzaefferer commented Feb 11, 2015

Uh oh!

scottgonzalez commented Feb 11, 2015

Uh oh!

rxaviers commented Feb 11, 2015

Uh oh!

scottgonzalez commented Feb 23, 2015

Uh oh!

rxaviers commented Feb 24, 2015

Uh oh!

jzaefferer commented Feb 24, 2015