Skip to content

Commit

Permalink
Fix breadcrumbs when paths have partial match 🍞 (github#22414)
Browse files Browse the repository at this point in the history
* Add failing breadcrumb test for paths with partial match

* Handle overlapping paths when creating breadcrumbs
  • Loading branch information
rsese authored Oct 26, 2021
1 parent e3074d9 commit 693e36b
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 6 deletions.
39 changes: 33 additions & 6 deletions middleware/contextualizers/breadcrumbs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}
})
}
13 changes: 13 additions & 0 deletions tests/rendering/breadcrumbs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down

0 comments on commit 693e36b

Please sign in to comment.