Conversation
🔍 Preview links for changed docs |
|
@Mpdreamz @bmorelli25 Wanted to give this one a try. Seems to work (check the Testing section, last two items). If the code is ugly, please feel free to salvage whatever looks useful in this PR, or commit directly. |
src/Elastic.Markdown/IO/Navigation/NavigationCrossLinkValidator.cs
Outdated
Show resolved
Hide resolved
src/Elastic.Markdown/IO/Navigation/NavigationCrossLinkValidator.cs
Outdated
Show resolved
Hide resolved
|
@Mpdreamz Anything missing for this one? Minus the remaining comment. |
|
Not sure @theletterf, I need some time to get to this PR still :) |
|
No probs! |
|
@Mpdreamz Seems like I broke link resolution with the last commit. Checking. |
|
This PR is finally failing like I was expecting it too :) https://github.com/elastic/docs-builder/actions/runs/17245378751/job/48933555065?pr=1615#step:5:154 The sitemap builder does not yet know about the new CrosslinkReference type, will amend the PR. |
…slinks (#1784) * Remove `Fetch` from CrossLinkResolver, enforce eager fetching of crosslinks. This removes a hidden requirement on `DocumentationSet` that its resolver is not usable until `DocumentationGenerator.GenerateAll()` has been called. We now enforce `DocumentationSet` receives a `ICrossLinkResolver` that is ready to resolve crosslinks. * Found more cases of unhandled navigation types * Remove new caching behavior in DocSetConfigurationCrossLinkFetcher
|
Excitedly jumps around Time to merge? :) |
|
Before you merge, I have questions:
|
|
@KOTungseth The way this is being implemented is to allow crosslinks in the nav. In that sense, a doc will only exist once and in one repo only, but we'll be able to link to it from other navs. Answering your questions:
Use of crosslinks might be unnecessary if @georgewallace's proposal for a unified nav UI prospers, but that might take a while. In the meantime, the need to solve some immediate navigation pains seems to exist. |
|
@Mpdreamz Could you comment with @KOTungseth? Can we move ahead? |
|
👋 just adding another use-case for this feature:
I'm looking forward to this PR being merged as it will help with closing #2738 and #2218, and avoid duplicated pages and drifting info. |
|
@KOTungseth I view this as another tool in our toolbox to solve some of our visibility issues in a lightweight way. agree that we shouldn't use it everywhere all the time, but I would like to see it merged. |
|
Approved ✅ |
|
@theletterf 🙏 thanks for this big contribution and bearing with us! 😸 Please wait for a release with this change to production before actually using the feature, will ping when its on production. Let's also trial 1/2 crosslinks in the navigation before going all in. |
|
@Mpdreamz Sounds good — thank you! Will start a small test after you give me green light to go ahead. :) |
This PR adds crosslinks as accepted item types in docset.yml toc directives.
For example: