Patches: Trivial (add div), Progressive Enhancement, Externalise CSS/JS

[alex sassmannshausen <alex.sassmannshausen@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2906 bytes --]

Hello,

Right, thanks for your feedback — and Cyril, for finding an HTML5 validator
that works. The NU validator seems to be well hidden if your starting
point is validator.w3.org.

Please find attached 3 patches. First is trivial, continuing Cyril's
work to make the package-list validate. It simply adds a missing closing
div, just after the footer.

The second patch re-introduces changes to the SXML and JavaScript to
fulfill the criteria of Progressive Enhancement:
- All content is shown when JavaScript is not enabled on GUI browsers.
In addition the patch implements JS that carries out the changes to the
HTML document successively rather than all at the end, which, with the
current size of the page, would cause a 'flicker'.

I have improved the changelog, as well as, I believe, made the functions
that these changes require, clearer.
Ludo, I appreciate set! being evil (maybe not the true extent yet
though…), but I believe it to be necessary on this occasion:
show_hide-grouper collects package description IDs in the local lid list
variable. This is achieved through repeated set!-ing. That list needs to
be cleared after each occasion that show_hide-grouper generates an sxml
JavaScript call to bulk_show_hide, with the collected package-IDs as
formals, to be inserted in the final HTML. bulk_show_hide initially
hides the package descriptions and creates the <a> element that allows
for the package description to be 'expanded' or 'collapsed'.

Currently show_hide-grouper inserts roughly 31 JavaScript calls to
bulk_show hide (1 every 15 package descriptions + 1 at the end to
collect the remaining package descriptions) in the final HTML
document. Hope this makes sense; if you can think of a better way of
doing it please let me know.

The third patch finally moves the CSS and JS into separate files, as
suggested by Ludo. I have implemented this so that by default, the page
simply links to those external files (this allows browsers to cache the
JS and CSS independently from the HTML). This does mean that those 2
additional files (package-list.js and package-list.css) need to be
pushed to the Guix site whenever changes are carried out to the CSS/JS.
The alternative, inlining the CSS and JS in the resulting HTML, which is
I think what was proposed, has been implemented as suggested, but I'm
running into an error — and I'm having difficulty locating it. It seems
to be happenning either during the (cute) evaluation, or the (dump-port)
evaluation. This functionality is currently disabled, and can be enabled
by calling insert-css or insert-js with an optional additional formal
parameter if you want to have a go at testing.

I'd be happy with the latter functionality removed or fixed — in terms
of HTML practice, I think the default, that is linking to external
files, is generally the done thing.

Best Wishes,

Alex

[-- Attachment #1.2: Type: text/html, Size: 3153 bytes --]

[-- Attachment #2: 0001-list-packages-Add-missing-closing-div-after-footer-i.patch --]
[-- Type: application/octet-stream, Size: 1021 bytes --]

From 01d1183d310a404500ecfda2e3bee4c996d580f8 Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Sun, 18 Aug 2013 13:34:05 +0200
Subject: [PATCH 1/3] list-packages: Add missing closing </div> after footer
 include.

* build-aux/list-packages.scm (list-packages): Add missing closing </div>
  after footer include.
---
 build-aux/list-packages.scm |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index bae25ac..5fbe359 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -244,7 +244,8 @@ with gnu.org server-side include and all that."
    (format #t "<!--#include virtual=\"/server/banner.html\" -->")
 
    (sxml->xml (packages->sxml packages))
-   (format #t "<!--#include virtual=\"/server/footer.html\" -->
+   (format #t "</div>
+<!--#include virtual=\"/server/footer.html\" -->
 <div id=\"footer\">
 
 <p>Please send general FSF &amp; GNU inquiries to
-- 
1.7.10.4


[-- Attachment #3: 0002-list-packages-Progressive-Enhancement-approach-to-JS.patch --]
[-- Type: application/octet-stream, Size: 5756 bytes --]

From fcbf81a5164e49a385bd1cd28a67e59d868a1cbf Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Sun, 18 Aug 2013 13:43:05 +0200
Subject: [PATCH 2/3] list-packages: Progressive Enhancement approach to JS.

* build-aux/list-packages.scm (package->sxml): Remove <a> element and JS
  function calls. These are now provided through (show_hide-grouper) below.
  Add call to (show_hide-grouper) for every package added to the table.
  (show_hide-grouper): New function.
  (packages->sxml): Add final call to (show_hide-grouper).
  (insert-js): show_hide: add compatibility check, introduce, use thingLink.
               prep: new JS function.
               bulk_show_hide: new JS function.
---
 build-aux/list-packages.scm |   80 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 72 insertions(+), 8 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index 5fbe359..161116b 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -103,13 +103,8 @@ exec guile -l "$0"                              \
                   (title "Link to the Guix package source code"))
                ,(package-name package) " "
                ,(package-version package)))
-        (td (a (@ (href "javascript:void(0)")
-                  (title "show/hide package description")
-                  (onClick ,(format #f "javascript:show_hide('~a')"
-                                    description-id)))
-               ,(package-synopsis package))
-            (div (@ (id ,description-id)
-                    (style "display: none;"))
+        (td (span (strong ,(package-synopsis package)))
+            (div (@ (id ,description-id))
                  ,(match (package-logo (package-name package))
                     ((? string? url)
                      `(img (@ (src ,url)
@@ -122,7 +117,44 @@ exec guile -l "$0"                              \
                  (a (@ (href ,(package-home-page package))
                        (title "Link to the package's website"))
                     ,(package-home-page package))
-                 ,(status package))))))
+                 ,(status package))
+            ,(show_hide-grouper description-id)))))
+
+(define show_hide-grouper
+  (let ((lid '())
+        (group-counter 15))
+    (lambda (id)
+      "Collect GROUP-COUNTER element IDs, then apply them to
+bulk_show_hide. If ID is #f, force the application of collected IDs to
+bulk_show_hide even when the number of IDs is smaller than GROUP-COUNTER."
+      (define (insert-js-call)
+        "Return an sxml call to bulk_show_hide."
+        (define (lid->js-argl)
+          "Parse a Scheme list into a JavaScript style arguments list."
+          (define (helper l)
+            (if (null? l)
+                (begin
+                  (set! lid '()) ; lid, now processed, safe to reset.
+                  "")
+                (string-append ", '" (car l) "'"      ; further args.
+                               (helper (cdr l)))))
+          (string-append "'" (car lid) "'"             ; initial arg.
+                         (helper (cdr lid))))
+        `(script (@ (type "text/javascript"))
+                 ,(format #f "bulk_show_hide(~a)"
+                          (lid->js-argl))))
+      (if id
+          (begin
+            ;; If ID is not false, then we add ID to LID.
+            (set! lid (cons id lid))
+            ;; If LID is now equal to GROUP-COUNTER it is time to insert a
+            ;; call to bulk_show_hide in the HTML.
+            (if (= (length lid) group-counter)
+                (insert-js-call)
+                ""))
+          ;; if ID is false then we force the insert of a call to
+          ;; bulk_show_hide for (< n GROUP-COUNTER) packages in the HTML.
+          (insert-js-call)))))
 
 (define (packages->sxml packages)
   "Return an HTML page as SXML describing PACKAGES."
@@ -146,6 +178,7 @@ exec guile -l "$0"                              \
                (th "Package version")
                (th "Package details"))
            ,@(map package->sxml packages))
+    ,(show_hide-grouper #f)
     (a (@ (href "#intro")
           (title "Back to top.")
           (id "top"))
@@ -210,14 +243,45 @@ color:#fff;
 // license: CC0
 function show_hide(idThing)
 {
+  if(document.getElementById && document.createTextNode) {
     var thing = document.getElementById(idThing);
+    /* Used to change the link text, depending on whether description is
+       collapsed or expanded */
+    var thingLink = thing.previousSibling.lastChild.firstChild;
     if (thing) {
       if (thing.style.display == \"none\") {
         thing.style.display = \"\";
+        thingLink.data = 'Collapse';
       } else {
         thing.style.display = \"none\";
+        thingLink.data = 'Expand';
       }
     }
+  }
+}
+/* Add controllers used for collapse/expansion of package descriptions */
+function prep(idThing)
+{
+  var tdThing = document.getElementById(idThing).parentNode;
+  if (tdThing) {
+    var aThing = tdThing.firstChild.appendChild(document.createElement('a'));
+    aThing.setAttribute('href', 'javascript:void(0)');
+    aThing.setAttribute('title', 'show/hide package description');
+    aThing.appendChild(document.createTextNode('Expand'));
+    aThing.onclick=function(){show_hide(idThing);};
+    /* aThing.onkeypress=function(){show_hide(idThing);}; */
+  }
+}
+/* Take n element IDs, prepare them for javascript enhanced
+display and hide the IDs by default. */
+function bulk_show_hide()
+{
+  if(document.getElementById && document.createTextNode) {
+    for(var i=0; i<arguments.length; i++) {
+      prep(arguments[i])
+      show_hide(arguments[i]);
+    }
+  }
 }
 </script>"))
 
-- 
1.7.10.4


[-- Attachment #4: 0003-list-packages-Externalise-CSS-and-JavaScript.patch --]
[-- Type: application/octet-stream, Size: 9774 bytes --]

From 3c3bfb6dea1b20447ba8bb48ec95a8cc4b556129 Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Sun, 18 Aug 2013 15:23:03 +0200
Subject: [PATCH 3/3] list-packages: Externalise CSS and JavaScript.

* build-aux/list-packages.scm (%load-directory): New variable.
  (%css-file): New variable.
  (%js-file): New variable.
  (insert-css): Rewrite to, by default, generate a <link> element calling the
  external CSS stylesheet. Contains structure for inlining the CSS from the
  external stylesheet.
  (insert-js): Rewrite to, by default, call external JS file, rather than
  inlining it. Contains structure for inlining the JS from the external file.
  (list-packages): generate sxml from insert-css and insert-js.
* package-list.css: New CSS file, with minor tweaks.
* package-list.js: New JS file, identical to last commit.
---
 build-aux/list-packages.scm |  134 +++++++++++--------------------------------
 build-aux/package-list.css  |   81 ++++++++++++++++++++++++++
 build-aux/package-list.js   |   43 ++++++++++++++
 3 files changed, 158 insertions(+), 100 deletions(-)
 create mode 100644 build-aux/package-list.css
 create mode 100644 build-aux/package-list.js

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index 161116b..883c32b 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -27,11 +27,13 @@ exec guile -l "$0"                              \
   #:use-module (guix packages)
   #:use-module (guix licenses)
   #:use-module (guix gnu-maintenance)
+  #:use-module (guix build utils)
   #:use-module (gnu packages)
   #:use-module (sxml simple)
   #:use-module (web uri)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-26)
   #:export (list-packages))
 
 ;;; Commentary:
@@ -185,105 +187,37 @@ bulk_show_hide even when the number of IDs is smaller than GROUP-COUNTER."
        "^")))
 
 \f
-(define (insert-css)
-  "Return the CSS for the list-packages page."
-  (format #t
-"<style>
-a {transition: all 0.3s}
-div#intro {margin-bottom: 5em}
-div#intro div, div#intro p {padding:0.5em}
-div#intro div {float:left}
-table#packages, table#packages tr, table#packages tbody, table#packages td,
-table#packages th {border: 0px solid black}
-div.package-description {position: relative}
-table#packages tr:nth-child(even) {background-color: #FFF}
-table#packages tr:nth-child(odd) {background-color: #EEE}
-table#packages tr:hover, table#packages tr:focus, table#packages tr:active {background-color: #DDD}
-table#packages tr:first-child, table#packages tr:first-child:hover, table#packages tr:first-child:focus, table#packages tr:first-child:active {
-background-color: #333;
-color: #fff;
-}
-table#packages td
-{
-margin:0px;
-padding:0.2em 0.5em;
-}
-table#packages td:first-child {
-width:10%;
-text-align:center;
-}
-table#packages td:nth-child(2){width:30%;}
-table#packages td:last-child {width:60%}
-img.package-logo {
-float: left;
-padding-right: 1em;
-}
-table#packages span a {float: right}
-a#top {
-position:fixed;
-right:2%;
-bottom:2%;
-font-size:150%;
-background-color:#EEE;
-padding:1.125% 0.75% 0% 0.75%;
-text-decoration:none;
-color:#000;
-border-radius:5px;
-}
-a#top:hover, a#top:focus {
-background-color:#333;
-color:#fff;
-}
-</style>"))
+(define %load-directory
+  (string-append (dirname (current-filename)) "/"))
 
-(define (insert-js)
-  "Return the JavaScript for the list-packages page."
-  (format #t
-"<script type=\"text/javascript\">
-// license: CC0
-function show_hide(idThing)
-{
-  if(document.getElementById && document.createTextNode) {
-    var thing = document.getElementById(idThing);
-    /* Used to change the link text, depending on whether description is
-       collapsed or expanded */
-    var thingLink = thing.previousSibling.lastChild.firstChild;
-    if (thing) {
-      if (thing.style.display == \"none\") {
-        thing.style.display = \"\";
-        thingLink.data = 'Collapse';
-      } else {
-        thing.style.display = \"none\";
-        thingLink.data = 'Expand';
-      }
-    }
-  }
-}
-/* Add controllers used for collapse/expansion of package descriptions */
-function prep(idThing)
-{
-  var tdThing = document.getElementById(idThing).parentNode;
-  if (tdThing) {
-    var aThing = tdThing.firstChild.appendChild(document.createElement('a'));
-    aThing.setAttribute('href', 'javascript:void(0)');
-    aThing.setAttribute('title', 'show/hide package description');
-    aThing.appendChild(document.createTextNode('Expand'));
-    aThing.onclick=function(){show_hide(idThing);};
-    /* aThing.onkeypress=function(){show_hide(idThing);}; */
-  }
-}
-/* Take n element IDs, prepare them for javascript enhanced
-display and hide the IDs by default. */
-function bulk_show_hide()
-{
-  if(document.getElementById && document.createTextNode) {
-    for(var i=0; i<arguments.length; i++) {
-      prep(arguments[i])
-      show_hide(arguments[i]);
-    }
-  }
-}
-</script>"))
+(define %css-file
+  "package-list.css")
+(define %js-file
+  "package-list.js")
+
+(define (insert-css . inline)
+  "Return an sxml link to the CSS file for the list-packages page.  If
+the optional inline argument is present, inline the CSS instead.
+Inlining currently fail with wrong-num-args error."
+  (if (null? inline)
+      `(link (@ (type "text/css")
+		(rel "stylesheet")
+		(href ,%css-file)))
+      `(style (@ (type "text/css"))
+	 ,(with-input-from-file (string-append %load-directory %css-file)
+	    (cute dump-port <> (current-output-port))))))
+
+(define (insert-js . inline)
+  "Return an sxml link to the CSS file for the list-packages page.  If
+the optional inline argument is present, inline the CSS instead.
+Inlining currently fail with wrong-num-args error."
+  (if (null? inline)
+      `(script (@ (type "text/javascript")
+                  (src ,%js-file))
+               " ")
+      `(style (@ (type "text/javascript"))
+	 ,(with-input-from-file (string-append %load-directory %js-file)
+	    (cute dump-port <> (current-output-port))))))
 
 \f
 (define (list-packages . args)
@@ -303,8 +237,8 @@ with gnu.org server-side include and all that."
 <!-- Parent-Version: 1.70 $ -->
 <title>GNU Guix - GNU Distribution - GNU Project</title>
 ")
-   (insert-css)
-   (insert-js)
+   (sxml->xml `(,(insert-css)
+                ,(insert-js)))
    (format #t "<!--#include virtual=\"/server/banner.html\" -->")
 
    (sxml->xml (packages->sxml packages))
diff --git a/build-aux/package-list.css b/build-aux/package-list.css
new file mode 100644
index 0000000..67d423a
--- /dev/null
+++ b/build-aux/package-list.css
@@ -0,0 +1,81 @@
+/* license: CC0 */
+a {
+  transition: all 0.3s;
+}
+
+div#intro {
+  margin-bottom: 2em;
+}
+div#intro div, div#intro p {
+  padding:0.5em;
+}
+div#intro div {
+  float:left;
+}
+div#intro img {
+  float:left;
+  padding:0.75em;
+}
+
+table#packages, table#packages tr, table#packages tbody, table#packages td, table#packages th {
+  border: 0px solid black;
+  clear: both;
+}
+
+div.package-description {
+  position: relative;
+}
+
+table#packages tr:nth-child(even) {
+  background-color: #FFF;
+}
+table#packages tr:nth-child(odd) {
+  background-color: #EEE;
+}
+table#packages tr:hover, table#packages tr:focus, table#packages tr:active {
+  background-color: #DDD;
+}
+table#packages tr:first-child, table#packages tr:first-child:hover, table#packages tr:first-child:focus, table#packages tr:first-child:active {
+  background-color: #333;
+  color: #fff;
+}
+table#packages td
+{
+ margin:0px;
+ padding:0.2em 0.5em;
+}
+table#packages td:first-child {
+width:10%;
+text-align:center;
+}
+table#packages td:nth-child(2){
+  width:30%;
+}
+table#packages td:last-child {
+  width:60%;
+}
+
+img.package-logo {
+float: left;
+padding: 0.75em;
+}
+
+table#packages span a {
+  float: right;
+}
+
+a#top {
+position:fixed;
+right:10px;
+bottom:10px;
+font-size:150%;
+background-color:#EEE;
+padding:10px 7.5px 0 7.5px;
+text-decoration:none;
+color:#000;
+border-radius:5px;
+}
+a#top:hover, a#top:focus {
+background-color:#333;
+color:#fff;
+}
\ No newline at end of file
diff --git a/build-aux/package-list.js b/build-aux/package-list.js
new file mode 100644
index 0000000..8b2971a
--- /dev/null
+++ b/build-aux/package-list.js
@@ -0,0 +1,43 @@
+// license: CC0
+function show_hide(idThing)
+{
+  if(document.getElementById && document.createTextNode) {
+    var thing = document.getElementById(idThing);
+    /* Used to change the link text, depending on whether description is
+       collapsed or expanded */
+    var thingLink = thing.previousSibling.lastChild.firstChild;
+    if (thing) {
+      if (thing.style.display == "none") {
+        thing.style.display = "";
+        thingLink.data = 'Collapse';
+      } else {
+        thing.style.display = "none";
+        thingLink.data = 'Expand';
+      }
+    }
+  }
+}
+/* Add controllers used for collapse/expansion of package descriptions */
+function prep(idThing)
+{
+  var tdThing = document.getElementById(idThing).parentNode;
+  if (tdThing) {
+    var aThing = tdThing.firstChild.appendChild(document.createElement('a'));
+    aThing.setAttribute('href', 'javascript:void(0)');
+    aThing.setAttribute('title', 'show/hide package description');
+    aThing.appendChild(document.createTextNode('Expand'));
+    aThing.onclick=function(){show_hide(idThing);};
+    /* aThing.onkeypress=function(){show_hide(idThing);}; */
+  }
+}
+/* Take n element IDs, prepare them for javascript enhanced
+display and hide the IDs by default. */
+function bulk_show_hide()
+{
+  if(document.getElementById && document.createTextNode) {
+    for(var i=0; i<arguments.length; i++) {
+      prep(arguments[i])
+      show_hide(arguments[i]);
+    }
+  }
+}
-- 
1.7.10.4