Closed drewsberry closed 10 years ago
If people don't like the centre-adjusted headlines, that can easily be changed. As can the overall theme, in fact.
I don't like that the nav bar follows you down the page.
It uses up screen space (especially on mobile!) and doesn't provide any extra useful functionality.
Can also easily be changed by removing navbar-fixed-top
on line 26 of base_template.html
I'd also like to see if the page load speed can be optimised at all. It currently adds an extra 100ms/66% to the load time.
But I generally like the theme. Dunno what Jamie will think though
Things that could be improved: Concatenate all css into one css file. Concatenate all js into one js file. Is the js actually necessary or can it be removed entirely? Don't use a web font so the text displays sooner.
The js is necessary for the dropdown menu on the mobile version (I checked that)
Could that be simplified and inlined instead then to remove the extra request? If we're only going to use to to trigger that drop down. This could also allow us to get rid of jQuery.
Hmm, how could that be inlined?
Also, if we're minifying and concatenating css and js then maybe we should use grunt?
No we definitely shouldn't use grunt, grunt is horrible. We already have a build system, we can just add it as a step to the current process.
As to how it could be inlined, something along the lines of
document.getElementsByClassName("navbar-toggle")[0].addEventListener('click', function(e) {
var n = document.getElementsByClassName("navbar-collapse")[0];
if (n.classList.contains("in")) {
n.style["height"] = "1px";
n.style["display"] = "none";
} else {
n.style["display"] = "block";
n.style["height"] = "184px";
}
n.classList.toggle("in");
});
Although I think it's worth thinking a bit more about the navigation. Will people need such easy access to things like the articles list page and the courses list page? Should we just have a link to the homepage at the top and leave it at that (for mobile anyway)?
And maybe a link to the course that the article is part of, if it is.
Could you explain what that snippet does?
It makes sense to have home, about, and courses tabs in the navbar, and a search box can be put in pretty easily too (at a later date) for searching through the articles. There wouldn't be an articles tab once we have a decent number of articles, as they would all be filed under corresponding courses, but at the moment they're not, so it seems useful till we get the courses sorted out.
Do you think I should make the minifying and concatenating part of the build.py
or part of the megphys
script? Making it part of the script would be easier, but I could just have a compress.py
which does it too, which is run as part of build.py
If we are going to keep the original files (which presumably we have to to separate our CSS from bootstrap CSS or we'd get a big mess) then the concatenation should happen as part of the build process, whenever the build is generated, leaving the original files unchanged.
And that snippet basically shows or hides the navbar when the button is clicked. It roughly replicates the current functionality, just without the animation.
For the concatenation, I would let the whole assets folder be copied across as it is now, and then concatenate the files from within the build folder, if that makes sense.
Although we should profile all these changes to make sure that we're not adding complexity for no performance benefit
It seems like copying the whole /assets
folder across is unnecessary, seeing as at the moment we're only loading /assets/flatly-bootstrap.min.css
and /assets/megaphysics.css
and /assets/bootstrap/js/bootstrap.min.js
.
Would it not be easier to just concatenate all css files into a single styles.css
file in the /assets
folder? The original files would still be unchanged.
We already copy the whole assets folder into build/, since it has all the images etc for the individual assets. My point was that if we concatenate from within the build folder then we don't end up with a generated style.css file in the repo that would get out of date and confuse things
Fair enough, the css files are now cat
ed into /build/assets
Actually I think leaving things in external javascript files is much better, but I'm still not comfortable with including the whole of jquery plus a big bootstrap js file for one animation
But for now I'm happy to just go with it, we can always optimise this stuff later.
I imagine later we'll use jQuery for more stuff as well, like if we want people to be able to search through the website (we can even autocomplete the article names), but yeah, we can always optimise stuff later once we've got a bit more definite structure.
All the js is loaded at the end of the page, so at least it doesn't prevent the page being presented quickly.
Except the web font doesn't load until all the javascript has loaded anyway
So do you think we should change to an non-web font?
This is what it looks like with Helvetica:
our website:
Personally I think serif fonts are better for long form writing, but I realise it's somewhat subjective.
Georgia?
Yeah that looks good. Just make the paragraph headings left aligned and I think it looks good
Should the pictures be center or left-aligned, do you think? I quite like centre-aligned
I think centre.
Final screenshot:
Looks good. @Ap0c thoughts?
@drewsberry two things: can you remove the unminified versions and the source maps, and include jquery locally instead of loading it from google? That way we reduce the js to 1 HTTP request to our single concatted js file
What if we want to edit the css?
hmmm well ok but there's no reason to have unminified js knocking about
Because we have our own bootstrap theme, we don't actually need /assets/bootstrap/css
, so I could just get rid of that entirely, along with the unminified bootstrap.js?
sounds good.
Successfully rebased. Looks like it can be fast-forwarded now.
Desktop screenshot:
Mobile screenshot: