Utils.insertStylesheet #280

marckrenn · 2015-12-27T17:48:17Z

Please see http://share.framerjs.com/u8u9oefbf3mi/

jordandobson · 2015-12-27T21:09:13Z

Seems like we may want two functions here.

InsertCSS and InsertCSSPath as the way the it worked before this commit is to
Inject CSS in the page from a string.

koenbok · 2015-12-27T21:19:04Z

Yeah this breaks the previous function (see the failing test).

This could be useful and I'll add it but:

It needs tests.
It needs to adhere to styling (" over ' etc).
It needs to be a separate method with a good name :-)

marckrenn · 2015-12-27T21:36:37Z

Oh crap, I thought the existing Utils.insertCSS was supposed to work this way - do people really want to add single styles? - , hence why I replaced (fixed ha!) it. Obviously I didn't look carefully enough, I'm sorry!

Anyway, in this case, I'm not even sure if you should add it, as it could encourage people to use Framer with stylesheets ... which probably is not a good idea.

marckrenn · 2015-12-28T19:24:01Z

Test proposal:

describe "insertStyleSheet", ->
        it "should work", ->
            Utils.insertStyleSheet("https://fonts.googleapis.com/icon?family=Material+Icons")
            document.styleSheets[document.styleSheets.length - 1].href.should.notEqual(null)

Pros & Cons:

+ also checks if inserted stylesheet('s href) is valid
o couldn't get reliable results on fileLoad, correct results after reload
- only works with externally hosted files

As an alternative, .type could be evaluated, which should return a string of "text/css". This would also work with local stylesheets, however the downside of this method is that it doesn't care if the href is valid or not.

jordandobson · 2015-12-28T19:43:57Z

I believe all that you can accurately do is test that the node was added to
the DOM. Maybe test the path provided is actually added correctly.
On Mon, Dec 28, 2015 at 11:24 AM marckrenn notifications@github.com wrote:

Test proposal:

describe "insertStyleSheet", ->
it "should work", ->
Utils.insertStyleSheet("https://fonts.googleapis.com/icon?family=Material+Icons")
document.styleSheets[document.styleSheets.length - 1].href.should.notEqual(null)

Pros & Cons:

+ also checks if inserted stylesheet('s href) is valid

o couldn't get reliable results on fileLoad, correct results after
reload

- only works with externally hosted files

As an alternative, .type could be evaluated, which should return a
string of "text/css". This would also work with local stylesheets, however
the downside of this method is that it doesn't care if the href is valid or
not.

—
Reply to this email directly or view it on GitHub
#280 (comment).

marckrenn · 2015-12-29T00:06:12Z

Test proposal v2:

describe "insertStyleSheet", ->
        it "should be added", ->
            Utils.delay 0.1, ->
                styleSheetCount = document.styleSheets.length
                Utils.insertStyleSheet("https://fonts.googleapis.com/icon?family=Material+Icons")
                document.styleSheets.length.should.equal(styleSheetCount + 1)

This works reliable, but still only for external stylesheets! To be honest, I can't get my head around why local stylesheets seemingly don't increase document.styleSheets.length while working perfectly fine. Evaluating document.styleSheets.length seems to be the way to go as it's a definitive confirmation that a new stylesheet was successfully added ... contrary to document.body.childNodes.length, which ...

... I believe is prone to "background noise" ( = other nodes being added at the same time) and therefore being unreliable.
... doesn't validate the file / happily accepts invalid paths, types etc. which would render the test itself obsolete.

Name proposals:

Utils.insertStyleSheet
Utils.insertStylesheet
Utils.injectStyleSheet
Utils.injectStylesheet

koenbok · 2015-12-29T08:54:44Z

Isn't this just an async thing? So to test it you'll have to wait for the style sheet to load (use the onload attr): http://www.phpied.com/when-is-a-stylesheet-really-loaded/

Otherwise, I'm happy to merge this.

koenbok · 2015-12-29T08:55:52Z

framer/Utils.coffee

@@ -513,6 +513,16 @@ Utils.insertCSS = (css) ->
 	Utils.domComplete ->
 		document.body.appendChild(styleElement)

+Utils.insertStyleSheet = (path) ->
+


Maybe change path to url as path implies it's local.

marckrenn · 2015-12-29T21:45:07Z

Isn't this just an async thing? So to test it you'll have to wait for the style sheet to load (use the onload attr): http://www.phpied.com/when-is-a-stylesheet-really-loaded/

#4 document.styleSheets.length is pretty much what I've tried above. This method doesn't like locally hosted stylesheets for some reason.

#1 To me it seems like styleElement.onload needs to be in the same scope where styleElement was created (please correct me if I'm wrong) ...

Utils.insertStyleSheet = (url) ->

    styleElement = document.createElement("link")
    styleElement.rel = "stylesheet"
    styleElement.type = "text/css"
    styleElement.href = url

    Utils.domComplete ->
        document.body.appendChild(styleElement)

    styleElement.onload = ->
        return true

... which should make it testable by doing something like this ...

describe "insertStyleSheet", ->
    it "should be added", ->
        Utils.insertStyleSheet("https://fonts.googleapis.com/icon?family=Material+Icons").should.equal(true)
        Utils.insertStyleSheet("bogusUrl").should.notEqual(true)
        Utils.insertStyleSheet("mdl/material.css").should.equal(true)

... but since the stylesheet obviously hasn't been loaded at the time of calling the Utils.insertStyleSheet-function, it will always return the styleElement.onload-function (and never true, even if it was successfully added).

Am I missing something here?

koenbok · 2015-12-29T22:00:18Z

Yep. Let me show you, I'll make a JSFiddle to explain.

koenbok · 2015-12-29T22:26:29Z

Here you go: https://jsfiddle.net/LtgLw3pv/1/

Try and understand how the event listener works
Implement the function above with the signature: Utils.insertStyleSheet = (url, callback) ->
Make the test work
Use Framers dom event wrapper (ask Koen)

If this clicks, you're pretty close to being a JavaScript ninja :-)

koenbok · 2015-12-29T22:32:54Z

Also:

it will always return the styleElement.onload-function (and never true, even if it was successfully added).

This is a sometimes confusing part of CoffeeScript: implicit returns:

a = -> b = 1 actually gets translated to a = -> return b = 1

It's sometimes handy, and sometimes it bites you.

More here: http://blog.artillery.com/2014/06/working-with-coffeescript-common-gotchas.html

marckrenn · 2015-12-29T23:11:26Z

Thanks for taking the time, Koen! Really appreciate it. :)
I'll try my best tomorrow, please keep your fingers crossed, haha.

And I'm truly sorry for causing so much trouble for a function only maybe a handful of people will ever use. As I said before, using Framer with stylesheets - the way it works right now - is borderline useless anyway but I believe this topic holds some untapped potential.

This is a sometimes confusing part of CoffeeScript: implicit returns:

Believe me or not, I grasped the concept of implicit returns rather quickly some time ago. Actually, I think it's pretty nifty and quite in line with the overall concept and semantics(?) of coffeescript. What confused me in this particular case was the additional component of time, which gave me some head-scratching "inconsistencies". The unexpected implicit returns were just a sign that my code wasn't working properly.

koenbok · 2015-12-29T23:21:40Z

Don't worry, getting you to be able to contribute is an upside for me too :-)

marckrenn · 2015-12-30T19:10:19Z

Don't worry, getting you to be able to contribute is an upside for me too :-)

I'll take that as a compliment, yet I'd like to dampen your expectations: With 25 I'm pretty late to the party anyway and after failing to come up with a working solution to this problem, it seems like I'll never become a decent coder - let alone a code ninja.

Anyway, after putting way too many hours into solving this problem, coming up with four vastly different approaches - with at least one or two promising ones - I'm drained, out of ideas and I finally have to give up.

I think one major piece I'm missing is how to successfully pass an argument (url) to a function (Utils.insertStylesheet) with a callback. I've looked up at least 5 different tutorials, stackoverflow threads etc. and I'd say I get the concept, but once I'm trying to adapt them, it all falls apart ...

Utils.insertStylesheet = (url, callback) ->

    styleElement = document.createElement("link")
    styleElement.rel = "stylesheet"
    styleElement.type = "text/css"
    styleElement.href = url

    Utils.domComplete ->
        document.body.appendChild(styleElement)

    styleElement.addEventListener "load", ->
        callback()

confirm = ->
    return true

Utils.insertStylesheet("https://fonts.googleapis.com/icon?family=Material+Icons") ->
    print confirm() # should equal true

(Also, now that I'm looking at this code again, I think this would also need a callback timeout for testing non-working URLs)

jordandobson · 2015-12-30T20:07:52Z

Tests are hard and take practice.

One main thing. Make sure you add the event listener to the Dom object
before you add it to the document. If you do it afterwards I'm not sure it
will get called.

On Wed, Dec 30, 2015 at 11:10 AM marckrenn notifications@github.com wrote:

Don't worry, getting you to be able to contribute is an upside for me too
:-)

I'll take that as a compliment, yet I'd like to dampen your expectations:
With 25 I'm pretty late to the party anyway and after failing to come up
with a working solution to this problem, it seems like I'll never become a
decent coder - let alone a code ninja.

Anyway, after putting way too many hours into solving this problem, coming
up with four vastly different approaches - with at least one or two
promising ones - I'm drained, out of ideas and I finally have to give up.

I think one major piece I'm missing is how to successfully pass an
argument (url) to a function (Utils.insertStylesheet) with a callback.
I've looked up at least 5 different tutorials, stackoverflow threads etc.
and I'd say I get the concept, but once I'm trying to adapt them, it all
falls apart ...

Utils.insertStylesheet = (url, callback) ->
styleElement = document.createElement("link")
styleElement.rel = "stylesheet"
styleElement.type = "text/css"
styleElement.href = url

Utils.domComplete ->
    document.body.appendChild(
styleElement)
styleElement.addEventListener "load", ->
    callback()
confirm = ->
return true

Utils.insertStylesheet("https://fonts.googleapis.com/icon?family=Material+Icons") ->
print confirm() # should equal true

(Also, now that I'm looking at this code again, I think this would also
need a callback timeout for testing non-working URLs)

—
Reply to this email directly or view it on GitHub
#280 (comment).

marckrenn · 2015-12-30T22:24:03Z

Haha, true. That's (unfortunately) not causing the trouble, though ;)

marckrenn · 2015-12-30T23:41:08Z

Utils.insertStylesheet = (url, callback) ->

    styleElement = document.createElement("link")
    styleElement.rel = "stylesheet"
    styleElement.type = "text/css"
    styleElement.href = url

    styleElement.addEventListener "load", ->
        callback() if callback? # can someone confirm this line?

    Utils.domComplete ->
        document.body.appendChild(styleElement)

Utils.confirmStylesheet = ->
    return true

Utils.insertStylesheet "https://fonts.googleapis.com/icon?family=Material+Icons", ->
    print Utils.confirmStylesheet() # returns true

WHAT THE HELL.

Now onto testing non-working URLs ...
EDIT: eeeeh, let's see if I can get away with not having a negative test result for non-working URLS (.should.notEqual(true))

marckrenn · 2015-12-31T00:31:46Z

Utils.insertStylesheet

Utils.insertStylesheet = (url, callback) ->

    styleElement = document.createElement("link")
    styleElement.rel = "stylesheet"
    styleElement.type = "text/css"
    styleElement.href = url

    styleElement.addEventListener "load", ->
        callback() if callback?

    Utils.domComplete ->
        document.body.appendChild(styleElement)

Utils.insertStylesheet.inserted = -> # this is a terrible solution/wording, right?
    true # do you prefer return true?

Use Framers dom event wrapper (ask Koen)

Well, I guess I should ask you about Framer's dom event wrapper. :)

Test:

describe "insertStylesheet", ->

    it "should be inserted", ->

        Utils.insertStylesheet "https://fonts.googleapis.com/icon?family=Material+Icons", ->
            Utils.insertStylesheet.inserted().should.equal(true)

        Utils.insertStylesheet "foo://bar", ->
            Utils.insertStylesheet.inserted().should.notEqual(true) # legit?

        Utils.insertStylesheet "framer/style.css", ->
            Utils.insertStylesheet.inserted().should.equal(true)

Do you think this will do @koenbok ?

Framer project:

http://share.framerjs.com/ofv8wnvlihxp/

marckrenn · 2015-12-31T12:36:15Z

Test returns the following:

// dunno if that's a good thing or not:
Error loading resource foo://bar (301). Details: Protocol "foo" is unknown

 // yeah, didn't thought about that one: the local 'framer/style.css' is N/A.
Error loading resource file:///pipeline/source/test/phantomjs/framer/style.css (203).
Details: Error opening /pipeline/source/test/phantomjs/framer/style.css: No such file or directory
․․․․․․․․․

Is there a "save" CSS somewhere in test/static/ExternalDocument that I could or should use instead?

btw love your guys' humbleness 😂

Utils.insertCSS fix

dcfb12b

Please see http://share.framerjs.com/u8u9oefbf3mi/

changed function name and quotation marks

85ccb64

marckrenn changed the title ~~Utils.insertCSS fix~~ Utils.insertStyleSheet Dec 28, 2015

removed non-working condition

b571b19

koenbok reviewed Dec 29, 2015
View reviewed changes

changed 'path' to 'url'

ad906c1

changed name, added EventListener & .inserted

c09e793

marckrenn changed the title ~~Utils.insertStyleSheet~~ Utils.insertStylesheet Dec 31, 2015

added test for Utils.insertStylesheet

c50e0d3

marckrenn closed this Feb 13, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Utils.insertStylesheet #280

Utils.insertStylesheet #280

marckrenn commented Dec 27, 2015

jordandobson commented Dec 27, 2015

koenbok commented Dec 27, 2015

marckrenn commented Dec 27, 2015

marckrenn commented Dec 28, 2015

jordandobson commented Dec 28, 2015

marckrenn commented Dec 29, 2015

koenbok commented Dec 29, 2015

koenbok Dec 29, 2015

marckrenn commented Dec 29, 2015

koenbok commented Dec 29, 2015

koenbok commented Dec 29, 2015

koenbok commented Dec 29, 2015

marckrenn commented Dec 29, 2015

koenbok commented Dec 29, 2015

marckrenn commented Dec 30, 2015

jordandobson commented Dec 30, 2015

marckrenn commented Dec 30, 2015

marckrenn commented Dec 30, 2015

marckrenn commented Dec 31, 2015

marckrenn commented Dec 31, 2015

Utils.insertStylesheet #280

Utils.insertStylesheet #280

Conversation

marckrenn commented Dec 27, 2015

jordandobson commented Dec 27, 2015

koenbok commented Dec 27, 2015

marckrenn commented Dec 27, 2015

marckrenn commented Dec 28, 2015

jordandobson commented Dec 28, 2015

marckrenn commented Dec 29, 2015

Test proposal v2:

Name proposals:

koenbok commented Dec 29, 2015

koenbok Dec 29, 2015

Choose a reason for hiding this comment

marckrenn commented Dec 29, 2015

koenbok commented Dec 29, 2015

koenbok commented Dec 29, 2015

koenbok commented Dec 29, 2015

marckrenn commented Dec 29, 2015

koenbok commented Dec 29, 2015

marckrenn commented Dec 30, 2015

jordandobson commented Dec 30, 2015

marckrenn commented Dec 30, 2015

marckrenn commented Dec 30, 2015

marckrenn commented Dec 31, 2015

Utils.insertStylesheet

Test:

Framer project:

marckrenn commented Dec 31, 2015