Create default css theme for styling elements (body, h1, figure, etc) #171
Conversation
There was a problem hiding this comment.
Likely that we will finish this PR before #169. I don't think there should be any problems with merging this first right @vincerubinetti? (besides that users won't immediately see the effect of all the theme changes).
Can you comment on the fonts? Are all fonts downloaded or are system fonts used? Will all browsers and OSes have the same fonts... etcetera.
| /*! normalize.css v2.1.3 | MIT License | git.io/normalize */ | ||
| /* import google fonts */ | ||
| @import url("https://fonts.googleapis.com/css?family=Open+Sans:400,600,700"); | ||
| @import url("https://fonts.googleapis.com/css?family=Source+Code+Pro"); |
There was a problem hiding this comment.
Do we have to worry about the dependency on Google? For example, doesn't china great 🔥 🗿 block google domains? Would it make sense to host these fonts on https://github.com/greenelab/manubot-resources/ (license permitting).
There was a problem hiding this comment.
Do they completely block all google.com? I wasn't aware of that. Will they somehow block the whole html document if they see one link to google there?
If they just block the font import, then it will fall back to helvetica, and if the user doesn't have that installed, it'll fall back to whatever sans-serif they have (usually arial).
There was a problem hiding this comment.
So would it be possible to add the font files to manubot-resources and only use those if the googleapis import failed? It looks like they both have licenses that should allow this.
There was a problem hiding this comment.
Actually it looks like github doesn't support this:
https://stackoverflow.com/questions/39693837/how-do-i-use-github-to-host-a-webfont
=/
There was a problem hiding this comment.
I think a workaround would be to set GitHub pages to deploy the master branch of manubot-resources. One downside is that we wouldn't be able to provide versioning by commit hash then. Ideally, we would keep all links to repos versioned such that they are less likely to break.
There was a problem hiding this comment.
Or on second thought, we could make a gh-pages branch that stores versioned files and we commit to not removing them. We use the custom domain resources.manubot.org such that we could even switch web hosts.
There was a problem hiding this comment.
Would we be able to get the resource from gh-pages properly? Github can host fonts just fine, but when trying to link to the "raw" file, it doesn't work. According to that stackoverflow question, the reason is that gh raw files only support the text mime type.
There was a problem hiding this comment.
Is the font issue blocking this merge? I'm thinking gh-pages won't solve the issue if regular branches can't provide the right mime type. We can always add in our own hosting location of the fonts later. The only change that would be required would be adding an extra @import url() statement below the google ones.
There was a problem hiding this comment.
I think the GitHub Pages would properly serve the font files as it seems to be smart with mime types and is intended for hosting.
Not a blocker for merging, as we can always change this later. I think we potentially should, especially since some users do not like the omnipresence of google and would prefer not to use them if possible. In addition, to any potential issues with googleapis being blocked in china.
|
@dongbohu can you review this? |
| * Remove border when inside `a` element in IE 8/9. | ||
| */ | ||
| /* -------------------------------------------------- */ | ||
| /* headings */ |
There was a problem hiding this comment.
You may want to delete the trailing */ on this line to make the comments style consistent.
| overflow: hidden; | ||
| } | ||
| /* -------------------------------------------------- */ | ||
| /* manuscript header */ |
| margin: 0; | ||
| } | ||
| /* -------------------------------------------------- */ | ||
| /* text elements */ |
| * Define consistent border, margin, and padding. | ||
| */ | ||
| /* -------------------------------------------------- */ | ||
| /* section elements */ |
There was a problem hiding this comment.
Same issue on lines 142, 175, 256, 269, 287, 313, 324 and 381. (I am not going to label them separately.)
| /*! normalize.css v2.1.3 | MIT License | git.io/normalize */ | ||
| /* import google fonts */ | ||
| @import url("https://fonts.googleapis.com/css?family=Open+Sans:400,600,700"); | ||
| @import url("https://fonts.googleapis.com/css?family=Source+Code+Pro"); |
There was a problem hiding this comment.
I don't know exactly how the html page is rendered. But does it make sense to put everything between line #7 and last line into a separate CSS file?
There was a problem hiding this comment.
We're trying to minimize the amount of file inclusion/movement/handling the user has to do. The idea was to have everything needed for one theme in one and only one file. In normal web development circumstances I would have much more separation of files and code, but in this case, where the user has to deal directly with the files locally and also (presumably) doesn't know anything about web development, we wanted the files to be as self-contained as possible. In fact everything except the images are embedded right into the (single) .html file.
This build is based on 658bcd7. This commit was created by the following Travis CI build and job: https://travis-ci.org/greenelab/manubot-rootstock/builds/487684768 https://travis-ci.org/greenelab/manubot-rootstock/jobs/487684769 [ci skip] The full commit message that triggered this build is copied below: HTML themes: rewrite default.html with new style Merges #171 Various display improvements, simplified code structure with modern best practices.
This build is based on 658bcd7. This commit was created by the following Travis CI build and job: https://travis-ci.org/greenelab/manubot-rootstock/builds/487684768 https://travis-ci.org/greenelab/manubot-rootstock/jobs/487684769 [ci skip] The full commit message that triggered this build is copied below: HTML themes: rewrite default.html with new style Merges #171 Various display improvements, simplified code structure with modern best practices.
Merges manubot/rootstock#171 Various display improvements, simplified code structure with modern best practices.
No description provided.