Skip to content

Commit 693e36b

Browse files
authored
Fix breadcrumbs when paths have partial match 🍞 (#22414)
* Add failing breadcrumb test for paths with partial match * Handle overlapping paths when creating breadcrumbs
1 parent e3074d9 commit 693e36b

2 files changed

Lines changed: 46 additions & 6 deletions

File tree

middleware/contextualizers/breadcrumbs.js

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,11 @@ async function getBreadcrumbs(
3131
intendedLanguage
3232
) {
3333
// Find the page that starts with the requested path
34-
let childPage = pageArray.find((page) =>
35-
currentPathWithoutLanguage.startsWith(page.href.slice(3))
36-
)
34+
let childPage = findPageWithPath(currentPathWithoutLanguage, pageArray)
3735

3836
// Find the page in the fallback page array (likely the English sub-tree)
3937
const fallbackChildPage =
40-
(fallbackPageArray || []).find((page) => {
41-
return currentPathWithoutLanguage.startsWith(page.href.slice(3))
42-
}) || childPage
38+
findPageWithPath(currentPathWithoutLanguage, fallbackPageArray || []) || childPage
4339

4440
// No matches, we bail
4541
if (!childPage && !fallbackChildPage) {
@@ -74,3 +70,34 @@ async function getBreadcrumbs(
7470
return [breadcrumb]
7571
}
7672
}
73+
74+
// Finds the page that starts with or equals the requested path in the array of
75+
// pages e.g. if the current page is /actions/learn-github-actions/understanding-github-actions,
76+
// depending on the pages in the pageArray agrument, would find:
77+
//
78+
// * /actions
79+
// * /actions/learn-github-actions
80+
// * /actions/learn-github-actions/understanding-github-actions
81+
function findPageWithPath(pageToFind, pageArray) {
82+
return pageArray.find((page) => {
83+
const pageWithoutLanguage = page.href.slice(3)
84+
const numPathSegments = pageWithoutLanguage.split('/').length
85+
const pageToFindNumPathSegments = pageToFind.split('/').length
86+
87+
if (pageToFindNumPathSegments > numPathSegments) {
88+
// if the current page to find has more path segments, add a trailing
89+
// slash to the page comparison to avoid an overlap like:
90+
//
91+
// * /github-cli/github-cli/about-github-cli with /github
92+
return pageToFind.startsWith(`${pageWithoutLanguage}/`)
93+
} else if (pageToFindNumPathSegments === numPathSegments) {
94+
// if the current page has the same number of path segments, only match
95+
// if the paths are the same to avoid an overlap like:
96+
//
97+
// * /get-started/using-github with /get-started/using-git
98+
return pageToFind === pageWithoutLanguage
99+
} else {
100+
return false
101+
}
102+
})
103+
}

tests/rendering/breadcrumbs.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,19 @@ describe('breadcrumbs', () => {
5757
expect($breadcrumbs[0].attribs.title).toBe('product: Billing and payments')
5858
})
5959

60+
test('works for pages that have overlapping product names', async () => {
61+
const $ = await getDOM(
62+
// article path has overlap with `/en/github`
63+
'/en/github-cli/github-cli/about-github-cli'
64+
)
65+
const $breadcrumbs = $('[data-testid=breadcrumbs] a')
66+
console.log(`dbg: breadcrumbs`, $breadcrumbs)
67+
expect($breadcrumbs).toHaveLength(3)
68+
expect($breadcrumbs[0].attribs.title).toBe('product: GitHub CLI')
69+
expect($breadcrumbs[1].attribs.title).toBe('category: GitHub CLI')
70+
expect($breadcrumbs[2].attribs.title).toBe('article: About GitHub CLI')
71+
})
72+
6073
test('parses Liquid variables inside titles', async () => {
6174
const $ = await getDOM('/en/enterprise/admin/enterprise-support')
6275
const $breadcrumbs = $('[data-testid=breadcrumbs] a')

0 commit comments

Comments
 (0)