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

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

[-- Attachment #1: Type: text/plain, Size: 3906 bytes --]



Hi,

Sooner or later we will get there! :-)

To make the process easier, I've split the patches into 2 series. For
now, the JavaScript Progressive Enhancement patches.

>>>>> "Ludovic" == Ludovic Courtès <ludo@gnu.org> writes:

 alex sassmannshausen <alex.sassmannshausen@gmail.com>
 > 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'.

 Ludovic> Nice.  (What’s Progressive Enhancement?)

Progressive Enhancement is a web design/development approach where you
make sure that all functionality is available using a very low baseline
— i.e. no JS, smallest files possible, attempt to take old browsers into
account — and then add functionality as 'icing on the cake'. In this
way, users are (almost) never left behind.

 alex sassmannshausen <alex.sassmannshausen@gmail.com> skribis:
 > The second patch re-introduces changes to the SXML and JavaScript to

 [...]

 > +        (td (span (strong ,(package-synopsis package)))

 Ludovic> Shouldn’t we use CSS instead of <strong>?

You are right. I've now changed this (patch 1 no longer uses strong,
patch 3 implements CSS font-weight heaviness.

 > +(define show_hide-grouper

 Ludovic> I should have mentioned it: the name of a procedure should
 Ludovic> describe its result or computation (when it’s a pure function,
 Ludovic> like ‘expt’), or its effect (like ‘display’).  Otherwise it’s
 Ludovic> harder to tell what it does etc.

In an attempt to do this I have now renamed the JS function
prep_pkg_descs (for prepare_package_descriptions), and the Scheme
function you are discussing here is now called
insert-prep_pkg_descs. Hope that does the trick.

 > +  (let ((lid '())
 > +        (group-counter 15))
 > +    (lambda (id)

 [...]

 > +      (if id
 > +          (begin
 > +            ;; If ID is not false, then we add ID to LID.

 Ludovic> But this function still has a single call with #f as its
 Ludovic> argument.  Can you remove the parameter and the dead code?  Or
 Ludovic> am I missing something?

Actually that function (now called insert-prep_pkg_descs is called in 2
places in the Scheme code, once with #f as argument (in packages->sxml),
and once (at the end) of package->sxml, with each package-id as
argument. The code is not dead, as it is what accumulates up to 15
package-ids, which are then used when prep_pkg_descriptions is inserted
into the HTML table (do a search for insert-prep_pkg_descs, and you
should get three hits, one being the definition.

 [...]

 > +/* Take n element IDs, prepare them for javascript enhanced
 > +display and hide the IDs by default. */
 > +function bulk_show_hide()

 Ludovic> Align ‘display’ with ‘Take’.

Done.

 > From 3c3bfb6dea1b20447ba8bb48ec95a8cc4b556129 Mon Sep 17 00:00:00 2001
 > From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>

 [...]

This part deals with the splitting of the CSS and JS into separate
source files; I'll address that with a second patch series later (or
tomorrow).

 [...]

 > 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

 Ludovic> Can you indent this file in C-mode in Emacs or something
 Ludovic> similar?

This has been done in patch no. 2, if I understood you correctly.

 Ludovic> Thanks for following up!

No problem, thanks for the thorough feedback — let me know if these
patches cause any problems.

Best wishes,

Alex


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-list-packages-Progressive-Enhancement-approach-to-JS.patch --]
[-- Type: text/x-patch, Size: 5742 bytes --]

From 145d2c1dfd05c8c7182df4d6383629bfee070ba9 Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Tue, 20 Aug 2013 00:26:37 +0200
Subject: [PATCH 1/3] list-packages: Progressive Enhancement approach to JS.

* build-aux/list-packages.scm (package->sxml): Remove <a> elements and JS
  function calls. These are created through JS (prep_pkg_descs). Add call to
  insert-prep_pkg_descs for every package in the table.
  (insert-prep_pkg_descs): New function.
  (packages->sxml): Add final call to insert-prep_pkg_descs.
  (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 9cb07c1..1964c82 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 ,(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))
+            ,(insert-prep_pkg_descs description-id)))))
+
+(define insert-prep_pkg_descs
+  (let ((lid '())
+        (group-counter 15))
+    (lambda (id)
+      "Collect GROUP-COUNTER element IDs, then apply them to
+prep_pkg_descs. If ID is #f, force the application of collected IDs to
+prep_pkg_descs even when the number of IDs is smaller than GROUP-COUNTER."
+      (define (insert-js-call)
+        "Return an sxml call to prep_pkg_descs."
+        (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 "prep_pkg_descs(~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 prep_pkg_descs in the HTML.
+            (if (= (length lid) group-counter)
+                (insert-js-call)
+                ""))
+          ;; if ID is false then we force the insert of a call to
+          ;; prep_pkg_descs 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))
+    ,(insert-prep_pkg_descs #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 prep_pkg_descs()
+{
+  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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-list-packages-Tidy-CSS-in-preparation-for-split-into.patch --]
[-- Type: text/x-patch, Size: 3383 bytes --]

From 5b6e848d26204df0b7a0e3ee6a915ba13f903bb8 Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Tue, 20 Aug 2013 00:37:39 +0200
Subject: [PATCH 2/3] list-packages: Tidy CSS in preparation for split into
 external file.

* build-aux/list-packages.scm (insert-css): Tidy CSS alignment etc.
---
 build-aux/list-packages.scm |   96 ++++++++++++++++++++++++++++---------------
 1 file changed, 62 insertions(+), 34 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index 1964c82..4ef1ce9 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -189,50 +189,78 @@ prep_pkg_descs even when the number of IDs is smaller than GROUP-COUNTER."
   "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}
+/* 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;
+    background-color: #333;
+    color: #fff;
 }
-table#packages td
-{
-margin:0px;
-padding:0.2em 0.5em;
+table#packages td {
+    margin:0px;
+    padding:0.2em 0.5em;
 }
 table#packages td:first-child {
-width:10%;
-text-align:center;
+    width:10%;
+    text-align:center;
+}
+table#packages td:nth-child(2) {
+    width:30%;
+}
+table#packages td:last-child {
+    width:60%;
 }
-table#packages td:nth-child(2){width:30%;}
-table#packages td:last-child {width:60%}
 img.package-logo {
-float: left;
-padding-right: 1em;
+    float: left;
+    padding: 0.75em;
+}
+table#packages span a {
+    float: right;
 }
-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;
+    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;
+    background-color:#333;
+    color:#fff;
 }
 </style>"))
 
-- 
1.7.10.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-list-packages-Improve-CSS-for-legibility-remove-redu.patch --]
[-- Type: text/x-patch, Size: 1140 bytes --]

From be839fada47678b869199760eb351ff8151d2be6 Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Tue, 20 Aug 2013 00:56:37 +0200
Subject: [PATCH 3/3] list-packages: Improve CSS for legibility, remove
 redundant CSS.

* build-aux/list-packages.scm (insert-css): Improve CSS for legibility, remove redundant CSS.
---
 build-aux/list-packages.scm |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index 4ef1ce9..6303066 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -210,9 +210,6 @@ table#packages, table#packages tr, table#packages tbody, table#packages td, tabl
     border: 0px solid black;
     clear: both;
 }
-div.package-description {
-    position: relative;
-}
 table#packages tr:nth-child(even) {
     background-color: #FFF;
 }
@@ -244,8 +241,12 @@ img.package-logo {
     float: left;
     padding: 0.75em;
 }
+table#packages span {
+    font-weight: 700;
+}
 table#packages span a {
     float: right;
+    font-weight: 500;
 }
 a#top {
     position:fixed;
-- 
1.7.10.4