Opened 10 years ago

Closed 21 months ago

#5402 closed enhancement (patchwelcome)

dojo/css! plugin

Reported by: jburke Owned by: kzyp
Priority: low Milestone: 1.13
Component: Loader Version: 1.0
Keywords: reviewed Cc: rcgill
Blocked by: Blocking:

Description (last modified by bill)

Consider including some form of a dojo.requireCss for 1.1. There is a good summary on dojo-contributors by Bill of the options. This is just a placeholder ticket for now as a reminder, and as a final design is worked out, post it here.

See

Attachments (4)

requireCss.js (4.2 KB) - added by jburke 10 years ago.
First shot at requireCss, but has issue with cross domain css in Firefox: we cannot determine when it gets loaded.
requireCss.html (954 bytes) - added by jburke 10 years ago.
Test file. Place as a sibling to the dojo dir.
phlebotomy.png (147.5 KB) - added by codeseo 4 years ago.
http://phlebotomyguide.org
main.inc.php (1.6 KB) - added by Slavon 4 years ago.
http://remedyby.tumblr.com/

Download all attachments as: .zip

Change History (29)

comment:1 Changed 10 years ago by jburke

Attached a first shot at this. The big blocker right now is Firefox and css files that are loaded from another domain. Normally the linkNode.sheet.cssRules property could tell us if the file was loaded (or at least parsed, not sure about applied). Unfortunately, Firefox complains with a security error if cssRules is accessed when the css file is loaded from another domain. Safari 3 (on mac, haven't tested windows) seems to wait to define linkNode.sheet until the file has been loaded, so that works out in the xd case.

So, just xd css in Firefox is the holdup. Unfortunately I do not see an easy way around the issue yet. I tried in FF 3 beta 2, and same issue as FF 2.

The only thing I can think of is accepting a dojo.query string and a style property that should be set if the css file is loaded, but this is pretty weak: first you have to be sure the queried node will be in the document and match a style in the stylesheet, and since we would be inspecting a node.style.property, *no other stylesheet* should be setting that property. So yeah, very lame.

comment:2 Changed 10 years ago by bill

  • Description modified (diff)

comment:3 Changed 10 years ago by bill

Hi James. Sorry, no bright ideas about FF. We could require every CSS file to have some CSS class name defined, related to the name of the file itself, but that's a bit unpleasant.

Don't forget also that CSS files can include other CSS files (and we do this in dijit -- tundra.css includes dijit.css). Not sure how to handle that.

comment:4 Changed 10 years ago by jburke

  • Milestone changed from 1.1 to 1.2

comment:5 Changed 10 years ago by bill

FYI, this would solve #4058 and #5824 in a non-hackish way. (But we have a hack ways to fix them already, so no rush.)

Changed 10 years ago by jburke

First shot at requireCss, but has issue with cross domain css in Firefox: we cannot determine when it gets loaded.

Changed 10 years ago by jburke

Test file. Place as a sibling to the dojo dir.

comment:6 Changed 10 years ago by bill

Also need to confirm that (for example) doing a dojo.requireCss("tundra.css"); wouldn't be marked as completed until tundra's imports (like dijit.css and TabContainer.css) had finished downloading.

comment:7 Changed 10 years ago by jburke

The issue that is blocking a checkin of this code is outlined in this mozilla bug. If that bug gets traction, we might have something worth trying:

https://bugzilla.mozilla.org/show_bug.cgi?id=185236

comment:8 Changed 9 years ago by jburke

  • Description modified (diff)
  • Milestone changed from 1.2 to 1.3

Moving to 1.3. Need to see movement on the mozilla bug mentioned above. See if anything happens in the 1.3 timeframe.

comment:9 Changed 9 years ago by dylan

Given that this is blocked on a 6-year-old Mozilla bug, what should we do? Can we focus instead on a build system approach to roll-up all import statements? Can we add cssTemplatePath back as a mixin for Dijit (like Templated, and again make it an optional thinger)?

comment:10 Changed 9 years ago by jburke

  • Milestone changed from 1.3 to future

The build system already inlines @import calls inside CSS files, are you suggesting that the build system could process requireCss calls, and inline the CSS as a string in the JS file, then add the file's content to a newly created style node in the page?

I guess that might be an option for this bug: do the normal CSS link tag in non-build scenarios, documenting that it only works for CSS on the same domain, then have a build option that inlines the CSS with the dojo.requireCss call if you want xdomain support.

And/or have the build convert the CSS file into a .js file that can be loaded xdomain, and then take the contents and inject it into the page. That version of requireCss would require an extra param to indicate we are loading a JS file.

This would require a function, probably in loader.js (Dojo Base) that can take a string of CSS and append to head as a style tag, and fixing paths to images in the process.

It would be a lot easier/simpler/less code if FF would fix the bug. :p So much hoping for that.

Marking as future until actual work begins.

comment:11 Changed 9 years ago by bill

See also #5136.

comment:12 Changed 6 years ago by skaegi

I just noticed that https://bugzilla.mozilla.org/show_bug.cgi?id=185236 was fixed. Hoping there is a future again for something like requireCss...

comment:13 Changed 6 years ago by neonstalwart

  • Keywords needsreview added
  • Priority changed from high to low

comment:14 Changed 6 years ago by bill

  • Cc rcgill added
  • Component changed from Core to Loader
  • Description modified (diff)
  • Keywords reviewed added; needsreview removed
  • Summary changed from dojo.requireCss? to dojo/css! plugin

We definitely still want a way to ensure that a widget's CSS is loaded before the widget is instantiated.

dojo/text! solves half the problem, but we should have something that additionally:

  1. ensures the CSS gets added to the document, and only added once (regardless of how many widgets depend on it)
  2. path correction? I don't think we are using any filter:alpha() though, so maybe we don't need that

So I'd still like to keep this open, and hopefully get it fixed one day.

See https://gist.github.com/1370465 for some code we may be able to use.

Last edited 6 years ago by bill (previous) (diff)

comment:15 Changed 5 years ago by bill

#15544 is a duplicate of this ticket.

comment:16 Changed 5 years ago by dylan

We've discussed the possibility of splitting this portion of xstyle out of that package and making it available as a Dojo Loader plugin...

comment:17 Changed 5 years ago by thomasbachem

Would love to see the relevant xstyle parts as a Loader plugin!

comment:18 Changed 5 years ago by dylan

  • Milestone changed from future to 2.0
  • Owner changed from jburke to kzyp
  • Status changed from new to assigned

Reassigning to Kris, since he's the maintainer of xstyle.

comment:19 Changed 4 years ago by kzyp

I think this strategy sounds. I've been doing a lot of dev on xstyle lately, getting build stuff ready, and extensions, but once we are ready for us to break this out and put the CSS loading functionality in Dojo, I can certainly do so.

Changed 4 years ago by codeseo

Changed 4 years ago by Slavon

comment:20 Changed 3 years ago by thomasbachem

Would still love to see the relevant xstyle parts transformed into a Dojo plugin. Can I help to make that happen?

comment:21 Changed 3 years ago by bill

See also https://github.com/ibm-js/delite/blob/master/css.js. Ignoring the has("builder") blocks, which are for requirejs builds, this is a minimal implementation that leverages existing dojo code (i.e. Deferred). Not sure if it works on IE8 or not though.

comment:22 Changed 3 years ago by thomasbachem

It should work in IE 8, as it's using the same method as xstyle 0.2.1, which supports IE 6+: https://github.com/kriszyp/xstyle/blob/v0.2.1/core/load-css.js.

Btw, I would like to start a discussion around browser sniffing to detect onload support for <style> elements, as it's impossible to feature detect this (see comments in linked xstyle code), but IE 6+, FF 9+, Chrome 19+ and Safari 6+ seem to support it (http://pieisgood.org/test/script-link-events/). So we basically need the other method just for FF 3.5/3.6/4 and older Safaris...

comment:23 Changed 3 years ago by bill

IIUC "old safaris" includes many people's (android?) phones. That's why we don't just get rid of the fallback code path.

comment:24 Changed 3 years ago by thomasbachem

I agree, and FF 3.5/3.6/4 is also still around quite a lot. But in order to use the native onload events for the other 90% of the users out there, it seems like we need browser sniffing.

comment:25 Changed 21 months ago by dylan

  • Milestone changed from 2.0 to 1.12
  • Resolution set to patchwelcome
  • Status changed from assigned to closed

With the evolution of CSS preprocessors, and the difficulty in making the build system do the right thing, I think the current consensus is that loading CSS via AMD is probably an anti-pattern.

That said, if someone wants to finish this up, we'll consider it for 1.12, so marking as patchwelcome.

Note: See TracTickets for help on using tickets.