Skip to content

Commit 55437f8

Browse files
MpdreamzMpdreamz
authored
Fix Previous/Next and related navigation glitches after refactor (#2231)
* Ensure SiteNavigation implements INavigationTraverable * Ensure we get the current navigation in renderlayout from INavigationTraversable not DocumentationSet. * remove NavigationIndexedByCrossLink * SiteNavigation and DocumentationSet share assignments for INavigationTraversable * Fix tests, SiteNavigation was assuming the narrative docs were there * fix integration tests * fix tests for windows * Refactor INavigationTraversable methods to use `GetNavigationFor` instead of `GetNavigationItem` or `GetCurrent`.
1 parent c1c32ba commit 55437f8

File tree

22 files changed

+283
-126
lines changed

22 files changed

+283
-126
lines changed

‎src/Elastic.Documentation.Navigation/Assembler/SiteNavigation.cs‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,21 @@
22
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
33
// See the LICENSE file in the project root for more information
44

5+
using System.Collections;
6+
using System.Collections.Frozen;
57
using System.Collections.Immutable;
68
using System.Diagnostics;
9+
using System.Runtime.CompilerServices;
710
using Elastic.Documentation.Configuration.Assembler;
811
using Elastic.Documentation.Configuration.Toc;
912
using Elastic.Documentation.Extensions;
13+
using Elastic.Documentation.Navigation.Isolated.Leaf;
1014
using Elastic.Documentation.Navigation.Isolated.Node;
1115

1216
namespace Elastic.Documentation.Navigation.Assembler;
1317

1418
[DebuggerDisplay("{Url}")]
15-
public class SiteNavigation : IRootNavigationItem<IDocumentationFile, INavigationItem>
19+
public class SiteNavigation : IRootNavigationItem<IDocumentationFile, INavigationItem>, INavigationTraversable
1620
{
1721
private readonly string? _sitePrefix;
1822

@@ -51,7 +55,22 @@ public SiteNavigation(
5155
UnseenNodes = [.. _nodes.Keys];
5256
// Build NavigationItems from SiteTableOfContentsRef items
5357
var items = new List<INavigationItem>();
54-
var index = 0;
58+
// The root file leafs of the narrative repository act as root leafs for the overall site
59+
if (_nodes.TryGetValue(new Uri($"{NarrativeRepository.RepositoryName}://"), out var root))
60+
{
61+
if (root is INavigationHomeAccessor accessor)
62+
accessor.HomeProvider = new NavigationHomeProvider(_sitePrefix ?? "/", this);
63+
root.Parent = this;
64+
root.Index.Parent = this;
65+
items.Add(root.Index);
66+
foreach (var leaf in root.NavigationItems.OfType<ILeafNavigationItem<INavigationModel>>())
67+
{
68+
leaf.Parent = root;
69+
items.Add(leaf);
70+
}
71+
}
72+
73+
var index = items.Count;
5574
foreach (var tocRef in siteNavigationFile.TableOfContents)
5675
{
5776
var navItem = CreateSiteTableOfContentsNavigation(
@@ -82,6 +101,9 @@ public SiteNavigation(
82101
value.Parent = this;
83102
}
84103

104+
// Build positional navigation lookup tables from all navigation items in a single traversal
105+
NavigationDocumentationFileLookup = [];
106+
NavigationIndexedByOrder = this.BuildNavigationLookups(NavigationDocumentationFileLookup);
85107
}
86108

87109
public HashSet<Uri> DeclaredPhantoms { get; }
@@ -136,6 +158,12 @@ public SiteNavigation(
136158
void IAssignableChildrenNavigation.SetNavigationItems(IReadOnlyCollection<INavigationItem> navigationItems) =>
137159
throw new NotSupportedException("SetNavigationItems is not supported on SiteNavigation");
138160

161+
/// <inheritdoc />
162+
public ConditionalWeakTable<IDocumentationFile, INavigationItem> NavigationDocumentationFileLookup { get; }
163+
164+
/// <inheritdoc />
165+
public FrozenDictionary<int, INavigationItem> NavigationIndexedByOrder { get; }
166+
139167
/// <summary>
140168
/// Normalizes the site prefix to ensure it has a leading slash and no trailing slash.
141169
/// Returns null for null or empty/whitespace input.
@@ -270,4 +298,5 @@ void IAssignableChildrenNavigation.SetNavigationItems(IReadOnlyCollection<INavig
270298
}
271299
return node;
272300
}
301+
273302
}

src/Elastic.Markdown/IO/IPositionalNavigation.cs renamed to src/Elastic.Documentation.Navigation/INavigationTraversable.cs

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,34 +4,57 @@
44

55
using System.Collections.Frozen;
66
using System.Runtime.CompilerServices;
7-
using Elastic.Documentation.Navigation;
87

9-
namespace Elastic.Markdown.IO;
8+
namespace Elastic.Documentation.Navigation;
109

11-
public interface IPositionalNavigation
10+
public interface INavigationTraversable
1211
{
13-
ConditionalWeakTable<MarkdownFile, INavigationItem> MarkdownNavigationLookup { get; }
12+
ConditionalWeakTable<IDocumentationFile, INavigationItem> NavigationDocumentationFileLookup { get; }
1413
FrozenDictionary<int, INavigationItem> NavigationIndexedByOrder { get; }
15-
FrozenDictionary<string, ILeafNavigationItem<MarkdownFile>> NavigationIndexedByCrossLink { get; }
1614

17-
INavigationItem? GetPrevious(MarkdownFile current)
15+
IEnumerable<INavigationItem> YieldAll()
16+
{
17+
if (NavigationIndexedByOrder.Count == 0)
18+
yield break;
19+
var current = NavigationIndexedByOrder.Values.First();
20+
yield return current;
21+
do
22+
{
23+
current = GetNext(current);
24+
if (current is not null)
25+
yield return current;
26+
27+
} while (current is not null);
28+
}
29+
30+
INavigationItem? GetPrevious(IDocumentationFile current)
31+
{
32+
var currentNavigation = GetNavigationFor(current);
33+
return GetPrevious(currentNavigation);
34+
}
35+
36+
private INavigationItem? GetPrevious(INavigationItem currentNavigation)
1837
{
19-
var currentNavigation = GetCurrent(current);
2038
var index = currentNavigation.NavigationIndex;
2139
do
2240
{
2341
var previous = NavigationIndexedByOrder.GetValueOrDefault(index - 1);
2442
if (previous is not null && !previous.Hidden && previous.Url != currentNavigation.Url)
2543
return previous;
2644
index--;
27-
} while (index > 0);
45+
} while (index >= 0);
2846

2947
return null;
3048
}
3149

32-
INavigationItem? GetNext(MarkdownFile current)
50+
INavigationItem? GetNext(IDocumentationFile current)
51+
{
52+
var currentNavigation = GetNavigationFor(current);
53+
return GetNext(currentNavigation);
54+
}
55+
56+
private INavigationItem? GetNext(INavigationItem currentNavigation)
3357
{
34-
var currentNavigation = GetCurrent(current);
3558
var index = currentNavigation.NavigationIndex;
3659
do
3760
{
@@ -44,9 +67,9 @@ public interface IPositionalNavigation
4467
return null;
4568
}
4669

47-
INavigationItem GetCurrent(MarkdownFile file) =>
48-
MarkdownNavigationLookup.TryGetValue(file, out var navigation)
49-
? navigation : throw new InvalidOperationException($"Could not find {file.RelativePath} in navigation");
70+
INavigationItem GetNavigationFor(IDocumentationFile file) =>
71+
NavigationDocumentationFileLookup.TryGetValue(file, out var navigation)
72+
? navigation : throw new InvalidOperationException($"Could not find {file.NavigationTitle} in navigation");
5073

5174
INavigationItem[] GetParents(INavigationItem current)
5275
{
@@ -65,6 +88,6 @@ INavigationItem[] GetParents(INavigationItem current)
6588
return [.. parents];
6689
}
6790

68-
INavigationItem[] GetParentsOfMarkdownFile(MarkdownFile file) =>
69-
MarkdownNavigationLookup.TryGetValue(file, out var navigation) ? GetParents(navigation) : [];
91+
INavigationItem[] GetParentsOfMarkdownFile(IDocumentationFile file) =>
92+
NavigationDocumentationFileLookup.TryGetValue(file, out var navigation) ? GetParents(navigation) : [];
7093
}

‎src/Elastic.Documentation.Navigation/NavigationItemExtensions.cs‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
33
// See the LICENSE file in the project root for more information
44

5+
using System.Collections.Frozen;
6+
using System.Runtime.CompilerServices;
57
using Elastic.Documentation.Navigation.Isolated;
8+
using Elastic.Documentation.Navigation.Isolated.Leaf;
69

710
namespace Elastic.Documentation.Navigation;
811

@@ -71,4 +74,55 @@ private static void ProcessNavigationItem(IDocumentationContext context, ref int
7174
break;
7275
}
7376
}
77+
78+
/// <summary>
79+
/// Builds navigation lookups by traversing the navigation tree and populating both the
80+
/// NavigationDocumentationFileLookup and NavigationIndexedByOrder collections.
81+
/// </summary>
82+
/// <param name="rootItem">The root navigation item to start traversing from</param>
83+
/// <param name="navigationDocumentationFileLookup">The ConditionalWeakTable to populate with file-to-navigation mappings</param>
84+
/// <returns>A frozen dictionary mapping navigation indices to navigation items</returns>
85+
public static FrozenDictionary<int, INavigationItem> BuildNavigationLookups(
86+
this INavigationItem rootItem, ConditionalWeakTable<IDocumentationFile, INavigationItem> navigationDocumentationFileLookup
87+
)
88+
{
89+
var navigationByOrder = new Dictionary<int, INavigationItem>();
90+
BuildNavigationLookupsRecursive(rootItem, navigationDocumentationFileLookup, navigationByOrder);
91+
return navigationByOrder.ToFrozenDictionary();
92+
}
93+
94+
/// <summary>
95+
/// Recursively builds both NavigationDocumentationFileLookup and NavigationIndexedByOrder in a single traversal
96+
/// </summary>
97+
private static void BuildNavigationLookupsRecursive(
98+
INavigationItem item,
99+
ConditionalWeakTable<IDocumentationFile, INavigationItem> navigationDocumentationFileLookup,
100+
Dictionary<int, INavigationItem> navigationByOrder)
101+
{
102+
switch (item)
103+
{
104+
// CrossLinkNavigationLeaf is not added to NavigationDocumentationFileLookup or NavigationIndexedByOrder
105+
case CrossLinkNavigationLeaf:
106+
break;
107+
case ILeafNavigationItem<IDocumentationFile> documentationFileLeaf:
108+
_ = navigationDocumentationFileLookup.TryAdd(documentationFileLeaf.Model, documentationFileLeaf);
109+
_ = navigationByOrder.TryAdd(documentationFileLeaf.NavigationIndex, documentationFileLeaf);
110+
break;
111+
case ILeafNavigationItem<INavigationModel> leaf:
112+
_ = navigationByOrder.TryAdd(leaf.NavigationIndex, leaf);
113+
break;
114+
case INodeNavigationItem<IDocumentationFile, INavigationItem> documentationFileNode:
115+
_ = navigationDocumentationFileLookup.TryAdd(documentationFileNode.Index.Model, documentationFileNode);
116+
_ = navigationByOrder.TryAdd(documentationFileNode.NavigationIndex, documentationFileNode);
117+
_ = navigationByOrder.TryAdd(documentationFileNode.Index.NavigationIndex, documentationFileNode.Index);
118+
foreach (var child in documentationFileNode.NavigationItems)
119+
BuildNavigationLookupsRecursive(child, navigationDocumentationFileLookup, navigationByOrder);
120+
break;
121+
case INodeNavigationItem<INavigationModel, INavigationItem> node:
122+
_ = navigationByOrder.TryAdd(node.NavigationIndex, node);
123+
foreach (var child in node.NavigationItems)
124+
BuildNavigationLookupsRecursive(child, navigationDocumentationFileLookup, navigationByOrder);
125+
break;
126+
}
127+
}
74128
}

‎src/Elastic.Markdown/DocumentationGenerator.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using Elastic.Documentation.Configuration.LegacyUrlMappings;
1010
using Elastic.Documentation.Configuration.Versions;
1111
using Elastic.Documentation.Links;
12+
using Elastic.Documentation.Navigation;
1213
using Elastic.Documentation.Serialization;
1314
using Elastic.Documentation.Site.FileProviders;
1415
using Elastic.Documentation.Site.Navigation;
@@ -53,6 +54,7 @@ public class DocumentationGenerator
5354
public DocumentationGenerator(
5455
DocumentationSet docSet,
5556
ILoggerFactory logFactory,
57+
INavigationTraversable? positionalNavigation = null,
5658
INavigationHtmlWriter? navigationHtmlWriter = null,
5759
IDocumentationFileOutputProvider? documentationFileOutputProvider = null,
5860
IMarkdownExporter[]? markdownExporters = null,
@@ -69,7 +71,7 @@ public DocumentationGenerator(
6971
DocumentationSet = docSet;
7072
Context = docSet.Context;
7173
var productVersionInferrer = new ProductVersionInferrerService(DocumentationSet.Context.ProductsConfiguration, DocumentationSet.Context.VersionsConfiguration);
72-
HtmlWriter = new HtmlWriter(DocumentationSet, _writeFileSystem, new DescriptionGenerator(), navigationHtmlWriter, legacyUrlMapper, productVersionInferrer);
74+
HtmlWriter = new HtmlWriter(DocumentationSet, _writeFileSystem, new DescriptionGenerator(), positionalNavigation, navigationHtmlWriter, legacyUrlMapper, productVersionInferrer);
7375
_documentationFileExporter =
7476
docSet.Context.AvailableExporters.Contains(Exporter.Html)
7577
? docSet.EnabledExtensions.FirstOrDefault(e => e.FileExporter != null)?.FileExporter

‎src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using Elastic.Documentation.Configuration;
1010
using Elastic.Documentation.Configuration.Synonyms;
1111
using Elastic.Documentation.Diagnostics;
12+
using Elastic.Documentation.Navigation;
1213
using Elastic.Documentation.Search;
1314
using Elastic.Ingest.Elasticsearch;
1415
using Elastic.Ingest.Elasticsearch.Indices;
@@ -382,8 +383,8 @@ private async ValueTask DoReindex(PostData request, string lexicalWriteAlias, st
382383
public async ValueTask<bool> ExportAsync(MarkdownExportFileContext fileContext, Cancel ctx)
383384
{
384385
var file = fileContext.SourceFile;
385-
IPositionalNavigation navigation = fileContext.DocumentationSet;
386-
var currentNavigation = navigation.GetCurrent(file);
386+
INavigationTraversable navigation = fileContext.DocumentationSet;
387+
var currentNavigation = navigation.GetNavigationFor(file);
387388
var url = currentNavigation.Url;
388389

389390
if (url is "/docs" or "/docs/404")

‎src/Elastic.Markdown/HtmlWriter.cs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ public class HtmlWriter(
2424
DocumentationSet documentationSet,
2525
IFileSystem writeFileSystem,
2626
IDescriptionGenerator descriptionGenerator,
27+
INavigationTraversable? positionalNavigation = null,
2728
INavigationHtmlWriter? navigationHtmlWriter = null,
2829
ILegacyUrlMapper? legacyUrlMapper = null,
2930
IVersionInferrerService? versionInferrerService = null
@@ -37,7 +38,7 @@ public class HtmlWriter(
3738

3839
private StaticFileContentHashProvider StaticFileContentHashProvider { get; } = new(new EmbeddedOrPhysicalFileProvider(documentationSet.Context));
3940
private ILegacyUrlMapper LegacyUrlMapper { get; } = legacyUrlMapper ?? new NoopLegacyUrlMapper();
40-
private IPositionalNavigation PositionalNavigation { get; } = documentationSet;
41+
private INavigationTraversable NavigationTraversable { get; } = positionalNavigation ?? documentationSet;
4142

4243
private IVersionInferrerService VersionInferrerService { get; } = versionInferrerService ?? new NoopVersionInferrer();
4344

@@ -59,18 +60,18 @@ private async Task<RenderResult> RenderLayout(MarkdownFile markdown, MarkdownDoc
5960
{
6061
var html = MarkdownFile.CreateHtml(document);
6162
await DocumentationSet.ResolveDirectoryTree(ctx);
62-
var navigationItem = DocumentationSet.FindNavigationByMarkdown(markdown);
63+
var navigationItem = NavigationTraversable.GetNavigationFor(markdown);
6364

6465
var root = navigationItem.NavigationRoot;
6566

6667
var navigationHtmlRenderResult = DocumentationSet.Context.Configuration.Features.LazyLoadNavigation
6768
? await NavigationHtmlWriter.RenderNavigation(root, navigationItem, 1, ctx)
6869
: await NavigationHtmlWriter.RenderNavigation(root, navigationItem, INavigationHtmlWriter.AllLevels, ctx);
6970

70-
var current = PositionalNavigation.GetCurrent(markdown);
71-
var previous = PositionalNavigation.GetPrevious(markdown);
72-
var next = PositionalNavigation.GetNext(markdown);
73-
var parents = PositionalNavigation.GetParentsOfMarkdownFile(markdown);
71+
var current = NavigationTraversable.GetNavigationFor(markdown);
72+
var previous = NavigationTraversable.GetPrevious(markdown);
73+
var next = NavigationTraversable.GetNext(markdown);
74+
var parents = NavigationTraversable.GetParentsOfMarkdownFile(markdown);
7475

7576
var remote = DocumentationSet.Context.Git.RepositoryName;
7677
var branch = DocumentationSet.Context.Git.Branch;

0 commit comments

Comments
 (0)