From 693e36bfdd4675c691e14615b231e2c03923f6fe Mon Sep 17 00:00:00 2001 From: Robert Sese Date: Tue, 26 Oct 2021 16:24:54 -0500 Subject: [PATCH] =?UTF-8?q?Fix=20breadcrumbs=20when=20paths=20have=20parti?= =?UTF-8?q?al=20match=20=F0=9F=8D=9E=20(#22414)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add failing breadcrumb test for paths with partial match * Handle overlapping paths when creating breadcrumbs --- middleware/contextualizers/breadcrumbs.js | 39 +++++++++++++++++++---- tests/rendering/breadcrumbs.js | 13 ++++++++ 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/middleware/contextualizers/breadcrumbs.js b/middleware/contextualizers/breadcrumbs.js index 35a69d487fe1..737201d44274 100644 --- a/middleware/contextualizers/breadcrumbs.js +++ b/middleware/contextualizers/breadcrumbs.js @@ -31,15 +31,11 @@ async function getBreadcrumbs( intendedLanguage ) { // Find the page that starts with the requested path - let childPage = pageArray.find((page) => - currentPathWithoutLanguage.startsWith(page.href.slice(3)) - ) + let childPage = findPageWithPath(currentPathWithoutLanguage, pageArray) // Find the page in the fallback page array (likely the English sub-tree) const fallbackChildPage = - (fallbackPageArray || []).find((page) => { - return currentPathWithoutLanguage.startsWith(page.href.slice(3)) - }) || childPage + findPageWithPath(currentPathWithoutLanguage, fallbackPageArray || []) || childPage // No matches, we bail if (!childPage && !fallbackChildPage) { @@ -74,3 +70,34 @@ async function getBreadcrumbs( return [breadcrumb] } } + +// Finds the page that starts with or equals the requested path in the array of +// pages e.g. if the current page is /actions/learn-github-actions/understanding-github-actions, +// depending on the pages in the pageArray agrument, would find: +// +// * /actions +// * /actions/learn-github-actions +// * /actions/learn-github-actions/understanding-github-actions +function findPageWithPath(pageToFind, pageArray) { + return pageArray.find((page) => { + const pageWithoutLanguage = page.href.slice(3) + const numPathSegments = pageWithoutLanguage.split('/').length + const pageToFindNumPathSegments = pageToFind.split('/').length + + if (pageToFindNumPathSegments > numPathSegments) { + // if the current page to find has more path segments, add a trailing + // slash to the page comparison to avoid an overlap like: + // + // * /github-cli/github-cli/about-github-cli with /github + return pageToFind.startsWith(`${pageWithoutLanguage}/`) + } else if (pageToFindNumPathSegments === numPathSegments) { + // if the current page has the same number of path segments, only match + // if the paths are the same to avoid an overlap like: + // + // * /get-started/using-github with /get-started/using-git + return pageToFind === pageWithoutLanguage + } else { + return false + } + }) +} diff --git a/tests/rendering/breadcrumbs.js b/tests/rendering/breadcrumbs.js index aa384a397ec8..4f424d8b424f 100644 --- a/tests/rendering/breadcrumbs.js +++ b/tests/rendering/breadcrumbs.js @@ -57,6 +57,19 @@ describe('breadcrumbs', () => { expect($breadcrumbs[0].attribs.title).toBe('product: Billing and payments') }) + test('works for pages that have overlapping product names', async () => { + const $ = await getDOM( + // article path has overlap with `/en/github` + '/en/github-cli/github-cli/about-github-cli' + ) + const $breadcrumbs = $('[data-testid=breadcrumbs] a') + console.log(`dbg: breadcrumbs`, $breadcrumbs) + expect($breadcrumbs).toHaveLength(3) + expect($breadcrumbs[0].attribs.title).toBe('product: GitHub CLI') + expect($breadcrumbs[1].attribs.title).toBe('category: GitHub CLI') + expect($breadcrumbs[2].attribs.title).toBe('article: About GitHub CLI') + }) + test('parses Liquid variables inside titles', async () => { const $ = await getDOM('/en/enterprise/admin/enterprise-support') const $breadcrumbs = $('[data-testid=breadcrumbs] a')