-
Notifications
You must be signed in to change notification settings - Fork 340
Clean up: Refactor JS includes, use SVG Logo (with fallback) #73
Conversation
PageSpeed Insights suggested those two.
Makes the code a lot clearer.
Thanks. How do future maintainers update the minified js? |
Future maintainers need to minify the JS themselves, e.g. using UglifyJS (online). Should I add an explanation… and a Readme file? Sadly, I haven't seen a way to automate this with Jekyll. If there is one, I'm of course open to suggestions (Github Pages doesn't allow Jekyll plugins, though). |
@killercup Thanks for the explanation. Since this makes maintenance more complicated I'm inclined to not minify. The Rust home page doesn't strike me as performance critical. |
As a web frontend developer I try to squeeze every possible bit, but I might agree with you here. I'll have a look at how much bigger the How often is this page updated anyway? Minifying one file that is changed every four months or so might not be a big deal. |
This also tries to fix some stuff in the custom JS code, e.g. wrapping code in self-invoking functions to avoid setting global variables.
b659fd8
to
ea9e26d
Compare
It's updated at least every six weeks for releases. |
A new release means a new download link, I presume. This shouldn't change any JS code, though. The platform detection code (to set the correct download link) could be easily refactored so that the package name is specified in the HTML code (or the YAML metadata). The
It's the first thing a lot of people might see in regard to Rust, so I think it'd be nice to have it loaded as fast as possible. Just let me know what you want to do, @brson. I have a patch ready to use the non-minified sources. |
@killercup Hey sorry for the delay. I do think I prefer the non-minified version because it is easier for me to deal with. Let me know if I'm being unreasonable though. |
Clean up: Refactor JS includes, use SVG Logo (with fallback)
Thanks! |
While writing the small PR earlier today to add the rustbyexample.com link (#72), I saw some pretty easy optimization opportunities.
This PR contains:
_includes
to make it more readableThere is no new functionality added in this PR. I have tested the site using Chrome on OS X and IE9 on WIn7 (browser stack, only shortly, though). I would welcome it if some people on other OS' could confirm it still works.