Add se-edu branding + navigation bar to website #888
Conversation
v1@pyokagan submitted v1 for review.
Checkout this PR version locallygit fetch https://github.com/se-edu/addressbook-level4.git refs/pr/888/1/head:BRANCHNAMEwhere |
|
Argh, forgot: this includes some code from the asciidoctor project, and so the |
|
@pyokagan Sorry I didn't think of this before: perhaps we should present AB4 site as an independent product site? i.e., not have the first row in the nav bar. That's because students will use this to create their own product site. The top row is more appropriate for the se-edu site rather than AB4 site, don't you think? |
|
@damithc There is a configuration option to remove the SE-EDU bar (see [9/12]). |
Should be fine then. 👍 |
docs/templates/_header.html.slim
Outdated
| li.navitem | ||
| a.nav-link href=(attr 'site-githuburl') | ||
| span.fa.fa-github.fa-lg aria-hidden='true' | ||
| | GitHub |
There was a problem hiding this comment.
Currently this looks like it's a link to github.com.
I think the link text should be View on GitHub.
In the upcoming commits, we will be adding a navigation bar to the SE-EDU website using asciidoctor's template mechanism[1]. In preparation of that, let's teach the asciidoctor gradle plugin to look for our template files inside the docs/templates/ directory by specifying the template_dirs option. Likewise, to guarantee correctness of the build, teach Gradle to rebuild our documentation if any of the template files change by specifying the template files as task inputs[2]. [1] https://asciidoctor.org/docs/user-manual/#provide-custom-templates [2] https://docs.gradle.org/4.8.1/userguide/more_about_tasks.html#sec:runtime_api_for_adhoc
We are trying to implement a navigation bar in the SE-EDU website using asciidoctor's template mechanism[1]. Asciidoctor's template mechanism works by allowing template developers to override the rendering of certain blocks in asciidoctor's syntax tree. For our case, if we want to add a navigation bar to the top of the page, we will need to override asciidoctor's `document` block handling. The problem is: the rendering logic for the document block itself is quite coarse -- covering everything from the header <meta> and <script> tags to the footer's "last updated" text. If we just want to add a navigation bar without changing the rest of the document's output, we will need to re-implement a large part of asciidoctor's HTML5 backend. Fortunately, the asciidoctor-backends[2] repository has done most of the hard work for us -- it features complete re-implementation of templates that mirror asciidoctor's default HTML5 output. By using these templates as a base, we can then make minimal changes on top to get our navigation bar. The asciidoctor-backends repo has templates for three different template engines: ERB, HAML and Slim. All three template engines are supported by the asciidoctor Gradle plugin, since AsciidoctorJ bundles them inside its JAR file[3]. Following the recommendation of asciidoctor's project lead[4], let's use the Slim backend. Import the necessary template files, contained within the slim/html5/ directory, from the latest commit (8ffcad1f9194fe2638f673aea7e41ed28c654800) of the asciidoctor-backends repo. Since we only need to modify the handling of the `document` block, we just need to import `document.html.slim`, as well as its dependencies. [1] https://asciidoctor.org/docs/user-manual/#provide-custom-templates [2] https://github.com/asciidoctor/asciidoctor-backends [3] https://github.com/asciidoctor/asciidoctorj#project-layout "Also bundles optional RubyGems needed at runtime, such as coderay, tile, haml and slim." [4] asciidoctor/asciidoctor-backends#118 (comment) "We should point out that ERB is complex and that, in our experience, the templates were exceptionally hard to maintain when done in ERB. In contrast, the templates are simple and clear when done in Slim and we have convenient facilities for simplifying the templates further with utility methods. Slim is a smarter engineering choice."
In the previous commit, we imported helpers.rb, as well as other template files, from the asciidoctor-backends[1] repo. However, as we did not import all of the template files, some of the code in helpers.rb is unused. The unused code is an unnecessary maintenance and readability burden, so let's remove them. [1] https://github.com/asciidoctor/asciidoctor-backends/tree/master/slim/html5
pyokagan
force-pushed
the
tpl
branch
2 times, most recently
from
a6c8d2e
to
e4f6ade
Compare
|
Ah, figured out why Travis didn't want to trigger, I had a PR open to my own personal fork, so Travis probably got confused -- pyokagan#4 |
|
Changes:
|
|
@yamgent @yamidark btw, I'd like to merge this by Friday if possible, since I'll be overseas after that. I understand you all are busy, but I'm not demanding a full review either -- just glance through the commit messages to check that the intent behind makes sense, read through the developer guide and maybe play around with it? I'm quite confident that the code will work. (Unlike se-edu-bot which died due to a lack of reviews (and I'm fine with that), this PR is a bit too important to die :-P ) |
There was a problem hiding this comment.
Partial review. One of the few parts not checked is [v2 3/12] (but everything runs without issues).
[v2 4/12]
- Add instructions on how to restart the gradle daemon.
- Rationale: Students may not know how to restart the gradle daemon, and may spend ages debugging this issue.
| |`site-githuburl` | ||
| |URL to the site's repository on https://github.com[GitHub]. | ||
| Setting this will add a "View on GitHub" link in the navigation bar. | ||
| |_not set_ |
There was a problem hiding this comment.
Default is set to https://github.com/se-edu/addressbook-level4.
There was a problem hiding this comment.
Not really. Will this help clarify the documentation?
diff --git a/docs/DeveloperGuide.adoc b/docs/DeveloperGuide.adoc
index 32c4319..028af2c 100644
--- a/docs/DeveloperGuide.adoc
+++ b/docs/DeveloperGuide.adoc
@@ -443,7 +443,10 @@ The `build.gradle` file specifies some project-specific https://asciidoctor.org/
|`site-githuburl`
|URL to the site's repository on https://github.com[GitHub].
+
Setting this will add a "View on GitHub" link in the navigation bar.
+
+Leaving this unset will not add a "View on GitHub" link in the navigation bar.
|_not set_
|`site-seedu`There was a problem hiding this comment.
Oh nevernmind, I think I see the problem now.
How about something like this?
diff --git a/docs/DeveloperGuide.adoc b/docs/DeveloperGuide.adoc
index 32c4319..8c3e42a 100644
--- a/docs/DeveloperGuide.adoc
+++ b/docs/DeveloperGuide.adoc
@@ -432,6 +432,9 @@ image::chrome_save_as_pdf.png[width="300"]
The `build.gradle` file specifies some project-specific https://asciidoctor.org/docs/user-manual/#attributes[asciidoc attributes] which affects how all documentation files within this project are rendered.
+[TIP]
+Attributes left unset in the `build.gradle` file will use their *default value*, if they have any.
+
[cols="1,2a,1", options="header"]
.List of site-wide attributes
|===| |`site-seedu` | ||
| |Define this attribute if the project is an official SE-EDU project. | ||
| This will render the SE-EDU navigation bar at the top of the page, and add some SE-EDU-specific navigation items. | ||
| |_not set_ |
docs/DeveloperGuide.adoc
Outdated
| @@ -68,7 +69,13 @@ Optionally, you can follow the <<UsingCheckstyle#, UsingCheckstyle.adoc>> docume | |||
|
|
|||
| ==== Updating documentation to match your fork | |||
|
|
|||
| After forking the repo, links in the documentation will still point to the `se-edu/addressbook-level4` repo. If you plan to develop this as a separate product (i.e. instead of contributing to the `se-edu/addressbook-level4`) , you should replace the URL in the variable `repoURL` in `DeveloperGuide.adoc` and `UserGuide.adoc` with the URL of your fork. | |||
| After forking the repo, the documentation will still refer to the `se-edu/addressbook-level4` repo. | |||
There was a problem hiding this comment.
Can also mention to the students about the additional site navigation bar that is specific to SE-EDU.
If necessary, the site-seedu attribute can also be mentioned here to make the connection between the nav-bar and the attribute more obvious.
There was a problem hiding this comment.
The comment in build.gradle should be sufficient, main thing is just to direct devs to build.gradle, and everything else will be self-evident there.
I'll make the following changes:
diff --git a/docs/DeveloperGuide.adoc b/docs/DeveloperGuide.adoc
index 04156d1..7734925 100644
--- a/docs/DeveloperGuide.adoc
+++ b/docs/DeveloperGuide.adoc
@@ -69,7 +69,7 @@ Optionally, you can follow the <<UsingCheckstyle#, UsingCheckstyle.adoc>> docume
==== Updating documentation to match your fork
-After forking the repo, the documentation will still refer to the `se-edu/addressbook-level4` repo.
+After forking the repo, the documentation will still refer to SE-EDU and the `se-edu/addressbook-level4` repo.
If you plan to develop this fork as a separate product (i.e. instead of contributing to `se-edu/addressbook-level4`), you should do the following:
So, here's the thing. I strongly discourage students from modifying the template files, especially if they are not experienced enough with programming. If students are not able to figure out what the gradle damon is and how to restart it such that they "spend ages debugging the issue", it's also an indication that they should not be modifying the template files, which requires some knowledge of Ruby and asciidoctor APIs, and even so, are extremely easy to break. (hence the warning on not modifying the template files unless CSS stylesheets are not powerful enough) Well, maybe I shouldn't be passing premature judgement, so here's what I will do: diff --git a/docs/DeveloperGuide.adoc b/docs/DeveloperGuide.adoc
index 940fd8f..d46b6de 100644
--- a/docs/DeveloperGuide.adoc
+++ b/docs/DeveloperGuide.adoc
@@ -488,6 +488,11 @@ The files in link:{repoURL}/docs/templates[`docs/templates`] controls the render
These template files are written in a combination of https://www.ruby-lang.org[Ruby] and http://slim-lang.com[Slim].
You should only modify them if you need greater control over the site's layout than what stylesheets can provide.
+[NOTE]
+====
+The Gradle daemon must be <<UsingGradle#RestartDaemon, restarted>> after modifying any of the files in link:{repoURL}/docs/templates[`docs/templates`] for the changes to take effect.
+====
+
[[Testing]]
== Testing
diff --git a/docs/UsingGradle.adoc b/docs/UsingGradle.adoc
index d1be2f3..6b68061 100644
--- a/docs/UsingGradle.adoc
+++ b/docs/UsingGradle.adoc
@@ -111,3 +111,14 @@ See `build.gradle` ->
* **`compileTestJava`** +
Checks whether the project has the required dependencies to perform testing, and download any missing dependencies before compiling the test classes. +
See `build.gradle` -> `allprojects` -> `dependencies` -> `testCompile` for the list of dependencies required.
+
+[[RestartDaemon]]
+== Restarting the Gradle daemon
+
+By default, Gradle will spawn a long-lived background process (daemon) to avoid the cost of JVM startup for every build.
+This background process will also cache information about project structure, files, tasks, etc. in memory.
+
+If you'd like to invalidate the cache, you can restart the Gradle daemon as follows:
+
+. Stop the Gradle daemon by running `./gradlew --stop`.
+. Start the Gradle daemon again by running any task. |
What I actually don't want is students modifying the template files, seeing them break, and then blaming us for it (filing an issue and such, which has happened before). So, I think I'll add this: diff --git a/docs/DeveloperGuide.adoc b/docs/DeveloperGuide.adoc
index d46b6de..c353a6b 100644
--- a/docs/DeveloperGuide.adoc
+++ b/docs/DeveloperGuide.adoc
@@ -486,7 +486,13 @@ You can modify them to change some styling properties of the site.
The files in link:{repoURL}/docs/templates[`docs/templates`] controls the rendering of `.adoc` files into HTML5.
These template files are written in a combination of https://www.ruby-lang.org[Ruby] and http://slim-lang.com[Slim].
+
+[WARNING]
+====
+Modifying the template files in link:{repoURL}/docs/templates[`docs/templates`] requires some knowledge and experience with Ruby and Asciidoctor's APIs.
You should only modify them if you need greater control over the site's layout than what stylesheets can provide.
+The SE-EDU team does not provide support for modified template files.
+====
[NOTE]
====
I'll be dropping the stuff about restarting the gradle daemon, since otherwise it'll be unnecessary extra documentation to maintain. |
For the purpose of speed, the asciidoctor gradle plugin will "reuse the asciidoctor instance to avoid reinstantiating the jruby runtime"[1]. This has the effect of keeping the old template files in memory, meaning that if the template files are modified while the gradle daemon is running, the changes won't be reflected in the running gradle daemon. The documentation will still use the old template even when the `asciidoctor` task is rerun. This optimization is however, still beneficial: developers are more likely to edit the documentation adoc files compared to modifying the template files. As such, they will benefit from the speed increase gained from keeping the template files in memory. Thus, we will not make any attempt to work around this. Instead, let's add a comment to the top of every template file, reminding developers that if they wish for their changes to the template files to be reflected in the generated HTML files, they will need to restart the Gradle daemon. [1] asciidoctor/asciidoctor-gradle-plugin@0b9d614
The footer has unnecessary empty vertical space. This is because if the `last-update-label` attribute is specified, an extra <br> is added, which is meant to separate the "Last updated" line from the document revision number line. However, if the document has no revision number, then it would seem that the footer has unnecessary empty vertical space. Fix this by only emitting the <br> if there is a revnumber AND a last-update-label.
Sorry, I don't quite get this. Did you mean it should be
Done |
Whoop sorry, my mistake. |
To give readers a more cohesive browsing experience, let's add a
navigation bar to the top of every page.
This navigation bar will remind readers the name of the website they are
browsing (AddressBook-Level4), and will provide convenient links to
frequently-used documents, such as the User Guide and Developer Guide.
Implement this navigation bar using the templating mechanism that we
have set up over the past few commits, and document it in the Developer
Guide.
Some important concerns that we need to handle are:
1. Given that AddressBook-Level4 is meant to be used by students as a
base to build their own products, which may or may not be an address
book app, we should be able to easily specify the site name that is
displayed in the navigation bar.
Solution: Teach the navigation bar to use the asciidoc attribute
`site-name` as the site name.
2. Not all documents may wish to have a navigation bar displayed at the
top of their page.
Solution: Teach the navigation bar to check if the asciidoc
attribute `no-site-header` is set. If it is, then we don't render
the navigation bar.
3. The rendered documentation should be browsable locally. Thus, links
in the navigation bar must be relative, and the proper relative
links must be generated no matter how deep the documentation
directory tree is.
Solution: Introduce an asciidoc attribute `site-root`, which should
store the absolute path of the root documentation directory. In
other words, it should be the same as the `sourceDir` specified in
build.gradle. This is used to workaround the fact that the
asciidoctor gradle plugin only passes in individual filenames, and
not the sourceDir, to asciidoctor.
The navigation bar can then compare the current directory with the
`site-root` directory, and use it to generate the correct relative
paths.
4. It looks nicer if the item that the reader is currently browsing is
highlighted in the navigation bar. e.g. If the current page is the
Developer Guide, the "Developer Guide" item will be highlighted in
the navigation bar.
This requirement is complicated by the fact that the "Developer
Guide" actually comprises of many documents: UsingGradle, Using
Travis, UsingAppVeyor etc.
Solution: Teach the navigation bar to examine the `site-section`
attribute of the document, and highlight the appropriate navigation
item.
5. The templating mechanism is extremely advanced and requires some
knowledge of Ruby and Asciidoctor's API. Downstream projects may
modify the template file, break it, and then file an issue expecting
support from us.
Solution: Explicitly state in the Developer Guide that we do not
provide support for modified template files. If downstream projects
break the files, they get to keep the pieces.
Thanks to the additional configuration attributes that we have added,
downstream projects will need to do more configuration to tweak the
documentation settings to match their fork. Update the developer guide
to mention this.
We'd like to encourage visitors to contribute to our project via GitHub. As such, let's add an easily-accessible link to our GitHub repo page to the navigation bar. Our downstream forks may also wish to do the same, so make the URL configurable through the `site-githuburl` attribute.
The help window launched by the `help` command displays the rendered User Guide (UserGuide.html). However, it contains a navigation bar. This navigation bar is non-functional -- clicking on the links will not do anything because the files that they link to are not found. This is because we only copy in `UserGuide.html` to the resources directory, and not the rest of the documentation. As such, let's hide the navigation bar when the User Guide is displayed in the Help Window. To do this, introduce a separate document, HelpWindow.adoc, that is only meant to be displayed inside the help window. This document just includes the contents of UserGuide.adoc verbatim, with one exception: it sets the `no-site-header` attribute, thus disabling the rendering of the navigation bar.
We would like to give visitors easy access to other SE-EDU projects, as well as build a cohesive SE-EDU brand image across all SE-EDU projects. As such, let's add a SE-EDU navigation bar to the top of every page. This navigation bar will display the SE-EDU logo, and provide links to other SE-EDU project. It will also highlight which project the reader is currently viewing. Our downstream projects, however, are not official SE-EDU projects, and so they have no need for this SE-EDU navigation bar. As such, let's make it such that the SE-EDU navigation bar is only rendered if the `site-seedu` attribute is set.
While the styling defined by asciidoctor.css looks very pleasant overall, some of its colors and fonts don't look very cohesive with our site design. Fix this by overriding some select style rules in gh-pages.css.
Our navigation bars and page content are all horizontally centered, thus it makes sense to center the footer text as well.
Students browsing the SE-EDU website may wish to have easy access to the Learning Outcomes for AddressBook-Level4. As such, let's add a "Learning Outcomes" link to the navigation bar. Downstream projects have no use for the "Learning Outcomes" document, since they are supposed to be standalone products. Hence, let's make the "Learning Outcomes" link SE-EDU-only, visible only when the `site-seedu` attribute is set.
v3@pyokagan submitted v3 for review.
(📚 Archive) (📈 Interdiff between v2 and v3) Checkout this PR version locallygit fetch https://github.com/se-edu/addressbook-level4.git refs/pr/888/3/head:BRANCHNAMEwhere |
|
Changes:
|
Fixes #736