From 740bafc26c605ef1cd7c7fb1a038a5cfe8a82fd7 Mon Sep 17 00:00:00 2001 From: Paul Hayes Date: Wed, 25 May 2022 09:43:49 +0100 Subject: [PATCH 1/2] If the current page is in the navigation, do not link to it When using the `includeInBreadcrumbs` option, the current page gets shown as the last item in the breadcrumb. Currently it shows as a link to itself and there is no indication that it is the current page. In this change, if the current page is in the navigation, no href is included, so the page does not get linked to by the breadcrumb component. This creates 2 options for breadcrumbs: 1. Hide the current page, only show the parents (current design system recommendation) 2. Show the current page, and indicate that it's the current page by not making it a link (previous design system recommendation) --- lib/filters/items-from-navigation.js | 35 ++++++++------ tests/lib/filters/items-from-navigation.mjs | 52 +++++++++++++++++---- 2 files changed, 66 insertions(+), 21 deletions(-) diff --git a/lib/filters/items-from-navigation.js b/lib/filters/items-from-navigation.js index 2340b10e..2f39cf31 100644 --- a/lib/filters/items-from-navigation.js +++ b/lib/filters/items-from-navigation.js @@ -14,19 +14,28 @@ module.exports = (eleventyNavigation, pageUrl = false, options = {}) => { const currentUrl = pageUrl ? url(pageUrl, pathPrefix) : false const items = [] - eleventyNavigation.map(item => items.push({ - current: pageUrl ? url(item.url, pathPrefix) === currentUrl : false, - parent: pageUrl ? pageUrl.startsWith(item.url, pathPrefix) : false, - href: url(item.url, pathPrefix), - text: item.title, - children: item.children - ? item.children.map(child => ({ - current: pageUrl ? url(child.url, pathPrefix) === currentUrl : false, - href: url(child.url, pathPrefix), - text: child.title - })) - : false - })) + eleventyNavigation.map(item => { + const isCurrentPage = pageUrl && url(item.url, pathPrefix) === currentUrl + const navItem = { + current: isCurrentPage, + parent: pageUrl ? pageUrl.startsWith(item.url, pathPrefix) : false, + text: item.title, + children: item.children + ? item.children.map(child => ({ + current: pageUrl ? url(child.url, pathPrefix) === currentUrl : false, + href: url(child.url, pathPrefix), + text: child.title + })) + : false + } + + // If the current page is being shown in the navigation, do not link to it + if (!isCurrentPage) { + navItem.href = url(item.url, pathPrefix) + } + + items.push(navItem) + }) if (options?.parentSite) { items.unshift({ diff --git a/tests/lib/filters/items-from-navigation.mjs b/tests/lib/filters/items-from-navigation.mjs index bd3ee5df..60bcfbf4 100644 --- a/tests/lib/filters/items-from-navigation.mjs +++ b/tests/lib/filters/items-from-navigation.mjs @@ -28,10 +28,20 @@ const eleventyNavigationBreadcrumb = [{ title: 'Child page', _isBreadcrumb: true }] +}, { + key: 'child', + parent: 'parent', + excerpt: false, + url: '/parent/child', + pluginType: 'eleventy-navigation', + parentKey: 'parent', + title: 'Child page', + _isBreadcrumb: true, + children: false }] test('Converts navigation data to items array', t => { - const result = itemsFromNavigation(eleventyNavigationBreadcrumb, '/parent/') + const result = itemsFromNavigation(eleventyNavigationBreadcrumb, '/parent/child') t.deepEqual(result, [{ href: '/', @@ -42,13 +52,18 @@ test('Converts navigation data to items array', t => { }, { href: '/parent/', text: 'Parent page', - current: true, + current: false, parent: true, children: [{ href: '/parent/child/', text: 'Child page', current: false }] + }, { + text: 'Child page', + current: true, + parent: true, + children: false }]) }) @@ -71,6 +86,12 @@ test('Converts navigation data to items array without page URL', t => { text: 'Child page', current: false }] + }, { + href: '/parent/child', + text: 'Child page', + current: false, + parent: false, + children: false }]) }) @@ -78,7 +99,7 @@ test('Converts navigation data to items array using path prefix', t => { const config = { pathPrefix: '/prefix/' } - const result = itemsFromNavigation(eleventyNavigationBreadcrumb, '/parent/', config) + const result = itemsFromNavigation(eleventyNavigationBreadcrumb, '/parent/child', config) t.deepEqual(result, [{ href: '/prefix/', @@ -89,13 +110,18 @@ test('Converts navigation data to items array using path prefix', t => { }, { href: '/prefix/parent/', text: 'Parent page', - current: true, + current: false, parent: true, children: [{ href: '/prefix/parent/child/', text: 'Child page', current: false }] + }, { + text: 'Child page', + current: true, + parent: true, + children: false }]) }) @@ -106,7 +132,7 @@ test('Converts navigation data to items array adding parent site', t => { name: 'Example' } } - const result = itemsFromNavigation(eleventyNavigationBreadcrumb, '/parent/', config) + const result = itemsFromNavigation(eleventyNavigationBreadcrumb, '/parent/child', config) t.deepEqual(result, [{ href: 'https://example.org', @@ -120,13 +146,18 @@ test('Converts navigation data to items array adding parent site', t => { }, { href: '/parent/', text: 'Parent page', - current: true, + current: false, parent: true, children: [{ href: '/parent/child/', text: 'Child page', current: false }] + }, { + text: 'Child page', + current: true, + parent: true, + children: false }]) }) @@ -138,7 +169,7 @@ test('Converts navigation data to items array adding parent site and using path }, pathPrefix: '/prefix/' } - const result = itemsFromNavigation(eleventyNavigationBreadcrumb, '/parent/', config) + const result = itemsFromNavigation(eleventyNavigationBreadcrumb, '/parent/child', config) t.deepEqual(result, [{ href: 'https://example.org', @@ -152,12 +183,17 @@ test('Converts navigation data to items array adding parent site and using path }, { href: '/prefix/parent/', text: 'Parent page', - current: true, + current: false, parent: true, children: [{ href: '/prefix/parent/child/', text: 'Child page', current: false }] + }, { + text: 'Child page', + current: true, + parent: true, + children: false }]) }) From f637e6dca09c630a2bd6a888f3a3ae563307976e Mon Sep 17 00:00:00 2001 From: Paul Hayes Date: Wed, 25 May 2022 09:51:52 +0100 Subject: [PATCH 2/2] Use forEach not map We don't want to be modifying the eleventNavigation object, just looping over it to generate a list of items for navigation components. --- lib/filters/items-from-navigation.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/filters/items-from-navigation.js b/lib/filters/items-from-navigation.js index 2f39cf31..575075b7 100644 --- a/lib/filters/items-from-navigation.js +++ b/lib/filters/items-from-navigation.js @@ -14,7 +14,7 @@ module.exports = (eleventyNavigation, pageUrl = false, options = {}) => { const currentUrl = pageUrl ? url(pageUrl, pathPrefix) : false const items = [] - eleventyNavigation.map(item => { + eleventyNavigation.forEach(item => { const isCurrentPage = pageUrl && url(item.url, pathPrefix) === currentUrl const navItem = { current: isCurrentPage,