fix: learn page breadcrumbs where label !== path slug

[abizek]

Description

Uses link = pathname check as a fallback for finding breadcrumbs in the navigation tree where the label does not match the path.

Consider the article "event-loop-timers-and-nexttick" where the label is "The Nodejs Event loop". Breadcrumbs cannot be found in the navigation tree. Of course we can always change the article name to match the label but what about the article "discover-javascript-timers"? Why, there is no problem here! Except there is. The "s" in JavaScript is capitalized. (Shocking, I know. I learned this today). "JavaScript" cannot be written as "java-script" in dash-case. Is it javascript or a script written in Java?

Previously docs should have the same article name and label for the breadcrumbs to work. Now it is not necessary. Unless a deeply nested document structure is adopted. In which case we have to DFS the navigation tree.

These four articles faced this breadcrumb issue:

• https://nodejs.org/en/learn/getting-started/ecmascript-2015-es6-and-beyond
• https://nodejs.org/en/learn/asynchronous-work/discover-javascript-timers
• https://nodejs.org/en/learn/asynchronous-work/event-loop-timers-and-nexttick
• https://nodejs.org/en/learn/manipulating-files/working-with-different-filesystems

Validation

Before	After

Related Issues

Related to #6679

Check List

• [x] I have read the Contributing Guidelines and made commit messages that follow the guideline.
• [x] I have run npm run format to ensure the code follows the style guide.
• [x] I have run npm run test to check if all tests are passing.
• [x] I have run npx turbo build to check if the website builds without errors.
• [x] I've covered new added functionality with unit tests if necessary.

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
nodejs-org	✅ Ready (Inspect)	Visit Preview	May 25, 2024 1:16pm

[bmuenzenmeyer]

fast tracking this bug fix - it's small, tested, and the broken behavior is sorta ugly in a small way

[mikeesto]

This PR seems to offer a (competing/similar?) approach https://github.com/nodejs/nodejs.org/pull/6728

[bmuenzenmeyer]

great catch @mikeesto - i suppose we should compare which is best

[abizek]

I didn't see that, nice catch @mikeesto. IMO, #6728's approach is better.

[bmuenzenmeyer]

@abizek appreciate your understanding here! will be closing this - hope you will continue to find ways to contribute

[abizek]

Hey @bmuenzenmeyer, sorry for the misunderstanding here. The digit regex only fixes the first link - https://nodejs.org/en/learn/getting-started/ecmascript-2015-es6-and-beyond. The other three cannot be fixed by it. I fixed both problems since belonged to the same category.

Let me know what you decide, I can revert just the regex changes in my PR if needed.

[bmuenzenmeyer]

haha yea - it looks like we need to slow down here - thanks 😄

[bmuenzenmeyer]

looking for consensus on this plan:

• apply regex solution from https://github.com/nodejs/nodejs.org/pull/6728
• close https://github.com/nodejs/nodejs.org/pull/6728

cc @TenzDelek @abizek @ovflowd @AugustinMauroy @mikeesto

[TenzDelek]

looking for consensus on this plan:

• apply regex solution from https://github.com/nodejs/nodejs.org/pull/6728

• close https://github.com/nodejs/nodejs.org/pull/6728

cc @TenzDelek @abizek @ovflowd @AugustinMauroy @mikeesto

+1 from me. Haha, looks like I overlooked the part where nodepath is not path which I believe @abizek cover in this pr. You can close my pr🤝 Overall it was great to learn about unit test thanks to @AugustinMauroy . Will try to contribute another time in future

[AugustinMauroy]

+1 for @TenzDelek pr with mixing both of solution. it's the first to arrive

[abizek]

+1 for applying regex solution from @TenzDelek

[ovflowd]

haha yea - it looks like we need to slow down here - thanks 😄

Yes, we shouldn't fast track this PR, small changes, but that touch a lot of stuff 😅

[abizek]

but that touch a lot of stuff

Does it though? It's only breadcrumbs.

[ovflowd]

but that touch a lot of stuff

Does it though? It's only breadcrumbs.

Yes, it touches the breadcrumbs of every single page. That's my definition of "touching a lot of stuff" 😅

[github-actions[bot]]

Lighthouse Results URL	Performance	Accessibility	Best Practices	SEO	Report
/en	🟢 92	🟢 100	🟢 100	🟢 91	🔗
/en/about	🟢 99	🟢 100	🟢 100	🟢 91	🔗
/en/about/previous-releases	🟢 99	🟢 100	🟢 100	🟢 92	🔗
/en/download	🟢 94	🟢 100	🟢 100	🟢 91	🔗
/en/blog	🟢 98	🟢 100	🟢 100	🟢 92	🔗

[github-actions[bot]]

Unit Test Coverage Report

Lines	Statements	Branches	Functions
	90.67% (593/654)	76.08% (175/230)	94.57% (122/129)

Unit Test Report

Tests	Skipped	Failures	Errors	Time
131	0 :zzz:	0 :x:	0 :fire:	5.991s :stopwatch:

[abizek]

@ovflowd gently nudging you in case you missed it 😄 https://github.com/nodejs/nodejs.org/pull/6729#discussion_r1608211020

[ovflowd]

@abizek are you going to make the changes we talked about?

[abizek]

@ovflowd I missed it. Done now. I understood what you said just now. There were too many ands and pluses :sweat_smile:. Pushing now.

[ovflowd]

@abizek can I ask you to do a last double-check on all about/learn pages that the breadcrumbs are working? 🙇

[abizek]

All systems are nominal. We are good to go. :+1: @ovflowd

[ovflowd]

All systems are nominal. We are good to go. 👍 @ovflowd

Beam me up, Scotty!