Merge pull request #3 from grip-on-software/nav-canonical-update

Improve canonical URL and full navigation update

lib/Locale.js

@@ -83,43 +83,63 @@ class Locale {
     // Create a navigation element. The page is the base URL to link to and the
     // query is the query string to append the language to. The query string
     // may have other parameters as long as the final part is the query string
-    // key (which may be empty) without the key-value separator.
-    generateNavigation(nav, page='', query='', linkActive=false, classes='', hash='') {
+    // key (which may be empty) without the key-value separator, but it is also
+    // possible to provide the parameters in the page. Anchor portions should be
+    // provided in the hash, starting with a hash sign.
+    generateNavigation(nav, page='', query='', linkActive='item', classes='', hash='') {
         if (!(nav instanceof d3.selection)) {
             nav = d3.select(nav);
         }
-        const item = nav.append('ul')
+        let url = null;
+        let separator = null;
+        try {
+            url = new URL(page);
+            url.searchParams.delete(query);
+            url.hash = '';
+            page = url.toString();
+            separator = url.search === '' ? '?' : '&';
+        }
+        catch (error) {
+            separator = page.includes('?') ? '&' : '?';
+        }
+
+        const updateItems = nav.selectAll('ul')
+            .data(['visualization-ui-locale-nav'])
+            .join('ul')
             .selectAll('li')
-            .data(Object.keys(this.specs))
-            .enter()
+            .data(Object.keys(this.specs));
+        const newItems = updateItems.enter()
             .append('li');
-        const link = item.append('a')
-            .call(this.updateNavigationLinks, page, query, hash)
+        newItems.append('a');
+        const items = updateItems.merge(newItems);
+        const links = items.selectAll('a')
+            .call(this.updateNavigationLinks, page, query, hash, separator)
             .attr('hreflang', d => d)
             .text(d => this.specs[d].language);
-        const active = linkActive === 'link' ? link : item;
+        const active = linkActive === 'link' ? links : items;
         active.classed('is-active', d => this.specs[d] == this.selectedLocale)
             .classed(classes, true);
 
-        // Update HEAD
-        if (page != '') {
+        // Update HEAD with canonical/alternate URLs
+        if (url !== null) {
             const head = d3.select('head');
-            head.selectAll('link[rel=canonical]').data([''])
+            head.selectAll('link[rel=canonical]')
+                .data(['visualization-ui-locale-canonical'])
                 .join('link')
                 .attr('rel', 'canonical')
-                .attr('href', page);
+                .attr('href', url);
             head.selectAll('link[rel=alternate][hreflang]')
                 .data(Object.keys(this.specs))
                 .join('link')
                 .attr('rel', 'alternate')
                 .attr('hreflang', d => d)
-                .call(this.updateNavigationLinks, page, query);
+                .call(this.updateNavigationLinks, url, query, '', separator);
         }
     }
 
-    updateNavigationLinks(links, page='', query='', hash='') {
+    updateNavigationLinks(links, page='', query='', hash='', separator='?') {
         links.attr('href',
-            d => `${page}?${query ? `${query}=` : ''}${d}${hash}`
+            d => `${page}${separator}${query}${query ? '=' : ''}${d}${hash}`
         );
     }
 

tests/locale.js

@@ -100,28 +100,60 @@ describe('Locale', () => {
         assert.equal(second.attr('hreflang'), 'nl');
         assert.equal(second.text(), 'Nederlands');
         assert.isTrue(items.filter(":nth-child(2)").classed("is-active"));
-        locales.updateNavigationLinks(items.selectAll('a'), 'page', 'lang',
-            '#location'
+        // Calling again performs update and does not add more navigation items
+        locales.generateNavigation("#languages", '?again');
+        assert.equal(d3.selectAll("#languages ul li").size(), 2);
+        assert.equal(d3.select('#languages ul li a').attr('href'), '?again&en');
+        done();
+    });
+    it('Updates navigation', (done) => {
+        const specs = require('./locales.json');
+        const { d3, Locale } = setupPage('<div id="languages"></div>', done);
+        const locales = new Locale(specs, "nl");
+        locales.generateNavigation("#languages");
+        const items = d3.selectAll("#languages ul li");
+        locales.updateNavigationLinks(items.selectAll('a'), 'page?good=yes',
+            'lang', '#location', '&'
         );
-        assert.equal(first.attr('href'), 'page?lang=en#location');
-        assert.equal(second.attr('href'), 'page?lang=nl#location');
+        const first = items.filter(":nth-child(1)").select('a');
+        assert.equal(first.attr('href'), 'page?good=yes&lang=en#location');
+        const second = items.filter(":nth-child(2)").select('a');
+        assert.equal(second.attr('href'), 'page?good=yes&lang=nl#location');
         locales.updateNavigationLinks(items.selectAll('a'));
         assert.equal(first.attr('href'), '?en');
         assert.equal(second.attr('href'), '?nl');
-        // Head links with page parameter
+        done();
+    });
+    it('Generates navigation with head links', (done) => {
+        const specs = require('./locales.json');
+        const { d3, Locale } = setupPage('<div id="languages"></div>', done);
+        const locales = new Locale(specs, "nl");
         locales.generateNavigation("#languages", "https://example.test/");
         const headLinks = d3.selectAll("head link");
         assert.equal(headLinks.size(), 3);
         assert.equal(headLinks.filter(":nth-child(1)").attr('href'),
             'https://example.test/'
         );
+        assert.equal(headLinks.filter(":nth-child(2)").attr('href'),
+            'https://example.test/?en'
+        );
         assert.equal(headLinks.filter(":nth-child(2)").attr('hreflang'), 'en');
         assert.equal(headLinks.filter(":nth-child(3)").attr('hreflang'), 'nl');
         // Calling generateNavigation again does not add more head link elements
-        locales.generateNavigation("#languages", "https://example.test/sub/");
+        // but updates the existing canonicalized links
+        locales.generateNavigation("#languages",
+            "https://example.test/sub/?a=b&lang=en#anchor", "lang", false, '',
+            '#anotheranchor'
+        );
         assert.equal(d3.selectAll("head link").size(), 3);
         assert.equal(d3.select("head link:nth-child(1)").attr('href'),
-            'https://example.test/sub/'
+            'https://example.test/sub/?a=b'
+        );
+        assert.equal(d3.select("head link:nth-child(2)").attr('href'),
+            'https://example.test/sub/?a=b&lang=en'
+        );
+        assert.equal(d3.select("head link:nth-child(3)").attr('href'),
+            'https://example.test/sub/?a=b&lang=nl'
         );
         done();
     });