Fix styling #62

pyokagan · 2018-07-28T07:29:16Z

No description provided.

asciidoctor.css is meant to be used with asciidoctor-generated documents. This repo uses MarkBind for documentation generation, and MarkBind primarily uses Bootstrap for styling. Having asciidoctor.css present in the page may conflict with Bootstrap's styling, resulting in awkward looking pages. So, let's remove asciidoctor.css.

We currently link the gh-pages.css stylesheet, which contains our website-specific style rules, in the <body> element. This is not valid HTML. Fix this by using Markbind's `head` feature to link the gh-pages.css stylesheet in the <head> element. This is done by introducing the _markbind/head/head.md document, and by adding the following to the frontmatter of every markdown file: head: head.md The following script is used to accomplish this: for x in *.md $(find contents -name '*.md'); do sed -i '/^ footer: footer\.md/a\ head: head.md' "$x" done

The items of the navigation bar are displayed as a column, which looks ugly and is not consistent with the rest of the SE-EDU website. Fix this by adding the `navbar-expand` class, which is a facility provided by Bootstrap to display the navigation bar items in a row at all times.

Some of the links in the navigation bar point to invalid locations. Let's fix them.

This makes the Learning Resources website more consistent with the other SE-EDU sites, which all have a similar secondary navigation bar.

pyokagan · 2018-07-28T07:30:54Z

@damithc Heh nice, I see Netlify has already been set up.

MarkBind automatically wraps the entire document in a div with the `container-fluid` style class set: <div id="app" class="container-fluid"> ... the document ... </div> Bootstrap defines the container-fluid style class to have padding on the left and right: .container-fluid { width: 100%; padding-right: 15px; padding-left: 15px; margin-right: auto; margin-left: auto; } This makes our top navigation bar have empty space on the left and right, which looks ugly. Unfortunately, we can't remove the container-fluid style class since MarkBind controls that part of the rendering. So, let's fix this by overriding the container-fluid style rules on #app in our gh-pages.css file. MarkBind's default footer styling actually uses a (rather strange) workaround: footer { margin-left: -15px; margin-right: -15px; } Once the #app.container-fluid padding is overridden, footer's workaround is no longer needed (and in fact will produce worse results by adding scrollbars to the document since its margins will extend outside the document). So, let's workaround footer's workaround by overriding it.

We override .nav-link in gh-pages.css to perform some SE-EDU specific styling. However, Bootstrap's CSS uses a more specific selector: .navbar-expand .navbar-nav .nav-link which overrides our styling. Fix this by making our selector more specific.

This makes the rendered documents consistent with the rest of the website.

This allows us to style the body of each document by adding style rules on .website-content. This was accomplished by running the following script: for x in *.md $(find contents -name '*.md'); do sed -i '/^{{ navbar | safe }}/a\<div class="website-content">' "$x" && sed -i '/^{{ navbar | safe }}/G' "$x" && echo >> "$x" && echo '</div>' >> "$x" done

This makes it consistent with the overall design of SE-EDU websites.

Bootstrap sets the line-height of navbars to 1.5. This makes the top SE-EDU navbar have a lot of extra vertical space, which is inconsistent with the rest of the SE-EDU website. Fix this by setting the line-height of large navbars (.navbar-lg) to 1.

The styling of the other SE-EDU websites (e.g. AddressBook-Level4) is such that headers will have some margins to separate them with whitespace from the rest of the document. This is not so with Bootstrap, where headers have extremely tiny margins. This causes the document body to be uncomfortably close to the site navigation bar, which looks bad. Fix this by adding some space below the #site-header.

damithc

Nice 👍 Thanks for doing this.
Hope it wasn't a lot of work to figure out the MarkBind stuff. We haven't optimized the author-friendliness aspect to our target levels yet.

damithc · 2018-07-28T07:49:12Z

@pyokagan do you want to keep the commits or shall I squash?

pyokagan · 2018-07-28T07:53:45Z

@damithc

Nah, took about 1 hour tops. I copied most of the stuff from se-book/website-base.

Keep the commits please.

pyokagan · 2018-07-28T07:54:41Z

btw, I needed to do some workarounds with strange MarkBind defaults in d8f206f . cc @yamgent

damithc · 2018-07-28T08:19:22Z

Deployed.

pyokagan · 2018-07-29T13:49:15Z

Hope it wasn't a lot of work to figure out the MarkBind stuff. We haven't optimized the author-friendliness aspect to our target levels yet.

Oh, I realized I probably should have left some usability feedback for MarkBind:

I found MarkBind constantly clearing the console kind of annoying, esp for non-daemon usage like markbind build.

I locally installed MarkBind (i.e. in the node_modules directory). MarkBind tried to convert the Markdown documents inside the node_modules directory. I tried to ignore it by adding node_modules/* to the site.json, but it didn't seem to work? In the end I hacked my way out by doing the following: (Note, I didn't read the user guide)

commit f1f32e4a2b4c4793af8707476e853b6b8562fbd2
Author: Paul Tan <pyokagan@pyokagan.name>
Date:   Sat Jul 28 13:55:21 2018 +0800

    TMP

diff --git a/site.json b/site.json
index fcbb30b..43ea59e 100644
--- a/site.json
+++ b/site.json
@@ -5,11 +5,21 @@
     "_markbind/logs/*",
     "_site/*",
     "site.json",
-    "*.md"
+    "*.md",
+    "node_modules/*"
   ],
   "pages": [
     {
-      "glob": "**/*.md"
+      "glob": "contents/**/*.md"
+    },
+    {
+        "glob": "index.md"
+    },
+    {
+        "glob": "GuidelinesForContributors.md"
+    },
+    {
+        "glob": "Contact.md"
     }
   ],
   "deploy": {

node printed the following on markbind serve, which is quite scary :-P

(node:25288) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 error listeners added. Use emitter.setMaxListeners() to increase limit

The navbar variable is used in almost all the markdown documents. However, when I modified the navbar in _markbind/variables.md, none of the documents got rebuilt. In the end I just resorted to Ctrl-C-ing markbind serve and restarting it each time.
A progress indicator for markbind build would be nice.

pyokagan · 2018-07-29T13:52:17Z

Also feels kind of obtuse to be adding head: head.md to every document as in 90f6492 , can it be set in the site.json?

damithc · 2018-07-29T14:24:53Z

Thanks for the comments @pyokagan Good points.
Hope to fix them in a future release (focus for this summer was reader-friendliness. Hope to switch focus to author-friendliness in the coming semester). Your comments will be useful at that time.

@yamgent Please create issues as necessary, when you have time.

yamgent · 2018-07-30T00:48:51Z

I found MarkBind constantly clearing the console kind of annoying, esp for non-daemon usage like markbind build.

Yes, I agree (I use the terminal constantly, and the clearing of console just seems to be counter-intuitive). Does the big logo printed at the beginning annoys you too as well? :P

I locally installed MarkBind (i.e. in the node_modules directory). MarkBind tried to convert the Markdown documents inside the node_modules directory. I tried to ignore it by adding node_modules/* to the site.json, but it didn't seem to work? In the end I hacked my way out by doing the following: (Note, I didn't read the user guide)

Not sure what the "hack" here is, because the change that you made is actually correct.

node printed the following on markbind serve, which is quite scary :-P
(node:25288) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 error listeners added. Use emitter.setMaxListeners() to increase limit

This looks quite familiar, will look into it. Does it always happen for you?

The navbar variable is used in almost all the markdown documents. However, when I modified the navbar in _markbind/variables.md, none of the documents got rebuilt. In the end I just resorted to Ctrl-C-ing markbind serve and restarting it each time.

A progress indicator for markbind build would be nice.

👌

Also feels kind of obtuse to be adding head: head.md to every document as in 90f6492 , can it be set in the site.json?

We will be reconsidering the design of this in the coming semester (footer and site-nav currently suffers the same problem).

damithc · 2018-07-30T00:59:26Z

node printed the following on markbind serve, which is quite scary :-P

Happened to me several times. In my case it was always caused by a mistake in my nunjucks code e.g., an array not closed properly.

pyokagan · 2018-07-30T03:16:30Z

@yamgent

Not sure what the "hack" here is, because the change that you made is actually correct.

I was surprised that the ignore configuration did not affect pages.

(Looking at the User Guide now, I see that there is no way to exclude files from a pages glob?)

pyokagan · 2018-07-30T03:43:56Z

This looks quite familiar, will look into it. Does it always happen for you?

Yeah it always happens on this repo, even from the start. Dunno if its different depending on node version, I'm using v9.11.2.

yamgent · 2018-07-30T03:54:35Z

I was surprised that the ignore configuration did not affect pages.

It is not supposed to. pages affects what files are getting rendered from markdown to html. ignore affects what files are being copied over to the final built site.

I know it can get quite confusing. I also found it confusing when I first started with the MarkBind project, but apparently no one else was disturbed by this problem, so... ¯\_(ツ)_/¯

(Looking at the User Guide now, I see that there is no way to exclude files from a pages glob?)

Nope. This problem is raised in MarkBind/markbind#228.

pyokagan added 5 commits July 28, 2018 15:28

navbar: fix up links to point to correct locations

a1ca877

Some of the links in the navigation bar point to invalid locations. Let's fix them.

navbar: add a secondary bar with the site title and search bar

281c8ac

This makes the Learning Resources website more consistent with the other SE-EDU sites, which all have a similar secondary navigation bar.

pyokagan requested a review from damithc July 28, 2018 07:30

pyokagan added 7 commits July 28, 2018 15:34

Contact, GuidelinesForContributors: add navbar

ab8e037

This makes the rendered documents consistent with the rest of the website.

css: limit website-content max-width

e3f92d1

This makes it consistent with the overall design of SE-EDU websites.

pyokagan force-pushed the fix-styling branch from 35f62bf to 5df1790 Compare July 28, 2018 07:36

damithc approved these changes Jul 28, 2018

View reviewed changes

damithc merged commit d8b1eb2 into se-edu:master Jul 28, 2018

This was referenced Jul 30, 2018

markbind serve may sometimes cause memory leak for EventEmitter MarkBind/markbind#385

Closed

site.json: Add configuration options to specify head, footer, site-nav for specific pages MarkBind/markbind#389

Closed

luyangkenneth mentioned this pull request Apr 26, 2019

Remove extra HTML tags #121

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix styling #62

Fix styling #62

pyokagan commented Jul 28, 2018

pyokagan commented Jul 28, 2018

damithc left a comment

damithc commented Jul 28, 2018

pyokagan commented Jul 28, 2018

pyokagan commented Jul 28, 2018

damithc commented Jul 28, 2018

pyokagan commented Jul 29, 2018

pyokagan commented Jul 29, 2018

damithc commented Jul 29, 2018

yamgent commented Jul 30, 2018

damithc commented Jul 30, 2018

pyokagan commented Jul 30, 2018

pyokagan commented Jul 30, 2018

yamgent commented Jul 30, 2018

Fix styling #62

Fix styling #62

Conversation

pyokagan commented Jul 28, 2018

pyokagan commented Jul 28, 2018

damithc left a comment

Choose a reason for hiding this comment

damithc commented Jul 28, 2018

pyokagan commented Jul 28, 2018

pyokagan commented Jul 28, 2018

damithc commented Jul 28, 2018

pyokagan commented Jul 29, 2018

pyokagan commented Jul 29, 2018

damithc commented Jul 29, 2018

yamgent commented Jul 30, 2018

damithc commented Jul 30, 2018

pyokagan commented Jul 30, 2018

pyokagan commented Jul 30, 2018

yamgent commented Jul 30, 2018