Skip to content

Navigation 1st principles hardening #3870

Description

@Mpdreamz

The navigation core is genuinely principled — INavigationItem / ILeafNavigationItem / INodeNavigationItem / IRootNavigationItem form a clean recursive tree, INavigationHomeProvider buys O(1) re-homing with lazily-computed URLs, and NavigationRenderModel resolves the tree once into a dumb render model the templates consume without logic. The periphery is where it frays, in four recurring patterns. None of this is in scope for the current branch — tracking here so the investigation survives.

1. Render mode encoded as three orthogonal booleans

NavigationRenderModel.Create takes isUsingNavigationDropdown, isPrimaryNavEnabled, isGlobalAssemblyBuild — eight combinations for three real modes. Each conditional inside (CreateRootIndex, CreateBackLinks) re-derives the mode from a different pair. One mode enum would make any production-affecting flag flip obvious rather than a one-token diff.

Related: IsUsingNavigationDropdown is both an INavigationItem property and a Create parameter, and GlobalNavigationHtmlWriter previously hardcoded a literal that contradicted the root's own value.

2. ~15 hand-rolled recursive walkers

The same tree is walked by ~15 independent recursive visitors, each re-deriving its own leaf/node/hidden/crosslink policy, despite INavigationTraversable existing. There is already a behavioural divergence:

  • SitemapBuilder.cs:89-100 filters on Hidden where the rest of the pipeline uses ExcludeFromIndexing, so listing pages that are deliberately "hidden but indexed" are silently missing from the sitemap.
  • CodexNavigation.cs:48-54 documents why a second root over shared nodes cannot reuse UpdateNavigationIndex at all — mutable NavigationIndex on a shared node is the leak.

3. docs-content privileged in code rather than configuration

docs-content is special-cased in at least five places:

  • SiteNavigation.cs:59-71 — root-leaf hoisting
  • SiteNavigation.cs:224-240path_prefix-required exemption (error path invents a bad-mapping-… URL segment)
  • SiteNavigationFile.cs — URI scheme default
  • AssembleSources.cs — implicit special casing
  • HtmlWriter.cs — indexing rule

Additionally, SiteNavigationFile.cs:158-257 and :259-332 are ~70 lines of copy-paste YAML converters that share the docs-content:// implicit scheme.

4. Mutation-after-construction as the extension mechanism

IAssignableChildrenNavigation, IAssignableIslandNavigation, SectionNavigation.Url's internal set, Index = null! / Id = null! sentinels, public Parent and NavigationIndex setters — each forces a defensive check downstream.

SiteNavigation implements IAssignableChildrenNavigation and then throws from SetNavigationItems (:184-185) — the interface contract is a lie for one implementation.


Feature-specific follow-ups (from the top-nav branch)

  • Aliased section URLs. SectionNavigation.Url = first child's URL (SiteNavigation.cs:96-99) is the root cause of both the GetParents() URL-dedupe drop and the "← Guides goes to get-started" limitation. A synthetic section landing page gives the node its own URL and removes both special cases.
  • Sections matched by display title. SectionTopNavBuilder.cs:34-41 keys plain tocs by Identifier but sections by Title with OrdinalIgnoreCase — renaming a section in YAML silently drops its tab. Match on identity (section://…).
  • Dead code. TopNavDropdownItem and TopNavLinkItem.SectionIds are never constructed anywhere in src/ — only in tests. The dropdown branch at _SecondaryNav.cshtml, secondary-nav.ts, and secondary-nav-dropdown.css are unreachable in real builds. Either wire a dropdown: concept into the schema or delete all of it.
  • SiteNavigation.TopLevelItems is marked //TODO Obsolete? yet is an input to SectionTopNavBuilder and GlobalNavigationHtmlWriter. Resolve before adding a third consumer.
  • String-namespace type test. LlmsNavigationEnhancer.cs:96 dispatches on item.GetType().FullName?.StartsWith("Elastic.ApiExplorer.") to avoid an assembly reference — reflection-shaped dispatch in an AOT codebase. A marker interface on INavigationItem removes it.

Localized messes worth their own cleanups

  • GlobalNavigationPathProvider.OutputFile (:59-111) — a dozen hardcoded repo/path prefixes under a //TODO clean up
  • ConfigurationFileProvider.CreateNavigationFile (:140-212) — regex line-surgery on navigation.yml plus an inlined hardcoded docs-builder:// block

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions