search

sciactive / pnotify

Beautiful JavaScript notifications with Web Notifications support.
https://sciactive.com/pnotify/
Apache License 2.0
3.65k stars 513 forks source link

Material CSS should not reference external URL #309

Closed codymullins closed 6 years ago

codymullins commented 7 years ago

When my internet went down, I noticed a suspicious failed URL call to https://fonts.googleapis.com/css?family=Material+Icons - which was odd, since our app is internal and we do not make external requests for resources like this. Any resource we have needs to be available locally.

I'm sure for the vast majority of people this doesn't matter, but for me (and likely many enterprise app developers) wouldn't want this. I'd like to request/suggest that this be an optional dependency that the developer installing the package should intentionally load this font in whatever format they need.

anotherchrisatwork commented 6 years ago

Agreed -- and it's worse for us, in that I'm developing an intranet application that has no connection to the internet, so the application hangs for a significant amount of time trying to load the font, each time the page loads. Please fix!

hperrin commented 6 years ago

I don't think I can package the font with PNotify, but I can make it so the font's location is configurable.

hperrin commented 6 years ago

@codymullins Do you want to take this one?

codymullins commented 6 years ago

Yeah, I can grab it.

codymullins commented 6 years ago

I've attached the pull request #330 - felt it was simplest to just have whoever is loading the module explicitly add the style sheet reference to make it clear of the dependency. Thoughts?

hperrin commented 6 years ago

Merged. :)