loadScript() takes a css option to load stylesheet by URL

[tomwayson]

• Describe the proposed changes:

loadScript() now takes a css option that is a URL (string) and (if present) passes that to loadCss() to load a stylesheet by URL

• Is there an example or test page to demonstrate any new or changed features?

not yet

• Does your PR include appropriate tests for source code alterations?

yes

• If you're adding or changing a public API, did you update the docs Usage sections of the README? If not, please provide a code snippet that demonstrates how to consume the new or updated API(s).

not yet. Basically, it's either loadScript({ css: cssUrl }) or loadModules(['esri/map'], { css: cssUrl }), but it seems like it's going to be tricky to figure out how that fits into the README, and I may have had too much 🍷to deal w/ that at this hour.

• Provide a reference to any related issue.

yet another step towards resolving #6

[davetimmins]

Couple of comments, I think that the default CSS version should match the default JS lib version. But having said that, the main usage for this library is to circumvent issues using modern JS tooling with dojo which referencing the CSS doesn't suffer from so I'm not sure that it should be used this way (I think the option of having it is fine but only as a backup).

[jwasilgeo]

@tomwayson I'll review this today and will also test like before with my riotjs framework example.

[tomwayson]

thanks @davetimmins!

I agree on all accounts.

I think that the default CSS version should match the default JS lib version

This PR is a testable incremental step towards the final api, which is going to allow what I think will be a more common use: loadModules(['esri/map'], { css: true }). In that case, rather than have a default CSS URL, loadScript() will try to interpolate the CSS URL from the JS URL based on @jwasilgeo's handy dandy table. If it can't figure it out, it will throw an error, so users that are not using the CDN hosted versions of the script will be encouraged to use css: '/path/to/my/esri/css/main.css' if they want esri-loader to handle the CSS loading.

the main usage for this library is to circumvent issues using modern JS tooling with dojo which referencing the CSS doesn't suffer from so I'm not sure that it should be used this way (I think the option of having it is fine but only as a backup)

I agree, loading the CSS is a convenience, and entirely optional. This is why I didn't merge #80 which made it the default behavior, and am instead slowly and deliberately building up the capability from the outside in to make sure it doesn't interfere w/ the core mission of this library.

[jwasilgeo]

I tried this in a sample application and it works as described both in your PR description, src code, and tests.

🚢 it

[tomwayson]

@jwasilgeo I'mma hold of on documenting/releasing this for the moment, unless you need me to do so.

I'm going to take a stab at #87 w/ the hope of making it easier to do #88 - if so we can document and release after those are done.

[jwasilgeo]

Sounds good, thanks!

[tomwayson]

@jwasilgeo, I lied, I released 2.2.0 w/ this and just punted on doc'ing it for now. Looks like it may be a while before I get a chance to work on #87 etc.

[jwasilgeo]

¯_(ツ)_/¯

It's all good either way for me. Thanks @tomwayson.