-
Notifications
You must be signed in to change notification settings - Fork 32
Refactor markdown and layout rendering by decoupling from GlobalLayoutViewModel. Streamline layout handling, enhance navigation item visibility logic, and rework documentation file structures for improved maintainability.
#1353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…utViewModel`. Streamline layout handling, enhance navigation item visibility logic, and rework documentation file structures for improved maintainability. `Hidden` is a pure navigation property now as its removed from `MarkdownFile`. `GetCurrentNavigation()` is now non nullable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors markdown and layout rendering by decoupling from GlobalLayoutViewModel and streamlining navigation handling. Key changes include:
- Converting the current navigation item to a non-nullable property and moving the hidden state from MarkdownFile to navigation items.
- Updating view files to use a new MarkdownLayoutViewModel and removing legacy layout switches.
- Adjusting navigation and link metadata creation, including enhanced error handling in GetCurrent navigation lookup.
Reviewed Changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Elastic.Markdown/Slices/IndexViewModel.cs | Made CurrentNavigationItem non-nullable to align with new GetCurrentNavigation contract. |
| src/Elastic.Markdown/Slices/Index.cshtml | Updated layout model and added a meta section for products when applicable. |
| src/Elastic.Markdown/Slices/HtmlWriter.cs | Swapped use of markdown.Hidden for current.Hidden in AllowIndexing condition. |
| src/Elastic.Markdown/Myst/FrontMatter/FrontMatterParser.cs | Added required using directive for updated slices. |
| src/Elastic.Markdown/IO/Navigation/DocumentationGroup.cs | Updated FileNavigationItem record to include a Hidden property and adjusted navigation items generation. |
| src/Elastic.Markdown/IO/MarkdownFile.cs | Removed the Hidden property as it is now a navigation concern. |
| src/Elastic.Markdown/IO/DocumentationSet.cs | Updated GetCurrent to be non-nullable with explicit exception on missing navigation; adjusted previous/next logic and link metadata accordingly. |
| src/Elastic.Documentation.Site/_ViewModels.cs | Revised GlobalLayoutViewModel to remove legacy properties and include updated members. |
| src/Elastic.Documentation.Site/_GlobalLayout.cshtml | Removed layout switching logic in favor of a simplified render body and footer section. |
| src/Elastic.Documentation.Site/Navigation/_TocTreeNav.cshtml | Filtered out hidden navigation items before rendering. |
| src/Elastic.Documentation.Site/Navigation/INavigationItem.cs | Added Hidden boolean to the navigation item interface. |
| src/Elastic.Documentation.Site/Layout/_SecondaryNav.cshtml | Updated conditional check to use RenderHamburgerIcon from the new layout model. |
| src/Elastic.Documentation.Site/Layout/_Head.cshtml | Removed legacy product meta tags in favor of new layout model usage. |
| src/Elastic.ApiExplorer (Operations, Landing, Endpoints) | Removed legacy properties and standardized navigation items with a default Hidden property. |
Comments suppressed due to low confidence (4)
src/Elastic.Markdown/IO/DocumentationSet.cs:39
- Confirm that callers of GetCurrent properly handle the potential InvalidOperationException when a navigation item for the given file is not found.
MarkdownNavigationLookup.GetValueOrDefault(file.CrossLink) ?? throw new InvalidOperationException($"Could not find {file.CrossLink} in navigation")
src/Elastic.Markdown/Slices/HtmlWriter.cs:107
- Ensure that the 'current' variable referenced here is properly defined in the enclosing scope so that its Hidden property is reliably accessed at runtime.
AllowIndexing = DocumentationSet.Context.AllowIndexing && (markdown is DetectionRuleFile || !current.Hidden)
src/Elastic.Markdown/IO/Navigation/DocumentationGroup.cs:16
- The addition of the 'Hidden' parameter in FileNavigationItem standardizes navigation visibility; please ensure that all consumers of this record are updated to accommodate the new field.
public record FileNavigationItem(MarkdownFile Model, DocumentationGroup Group, bool Hidden = false)
src/Elastic.Documentation.Site/_GlobalLayout.cshtml:23
- Ensure that all pages define a footer section; otherwise, rendering this section may lead to runtime errors.
@await RenderSectionAsync(GlobalSections.Footer)
| public IReadOnlyDictionary<Uri, TocReference> TocReferences { get; } = | ||
| Children.OfType<TocReference>().ToDictionary(kv => kv.Source, kv => kv); | ||
|
|
||
| public bool IsPhantom { get; init; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a comment what IsPhantom means? 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good callout! I did my best to explain their purpose in 05f89cc
…utViewModel`. Streamline layout handling, enhance navigation item visibility logic, and rework documentation file structures for improved maintainability. (#1353) * Refactor markdown and layout rendering by decoupling from `GlobalLayoutViewModel`. Streamline layout handling, enhance navigation item visibility logic, and rework documentation file structures for improved maintainability. `Hidden` is a pure navigation property now as its removed from `MarkdownFile`. `GetCurrentNavigation()` is now non nullable * Ensure NavigationIndex is a navigation property. * Update advertising of landing-page-path * update landing page path output * Add xmldocs for IsPhantom (cherry picked from commit 8df9149)
#1357) * Refactor markdown and layout rendering by decoupling from `GlobalLayoutViewModel`. Streamline layout handling, enhance navigation item visibility logic, and rework documentation file structures for improved maintainability. (#1353) * Refactor markdown and layout rendering by decoupling from `GlobalLayoutViewModel`. Streamline layout handling, enhance navigation item visibility logic, and rework documentation file structures for improved maintainability. `Hidden` is a pure navigation property now as its removed from `MarkdownFile`. `GetCurrentNavigation()` is now non nullable * Ensure NavigationIndex is a navigation property. * Update advertising of landing-page-path * update landing page path output * Add xmldocs for IsPhantom (cherry picked from commit 8df9149) * Ensure links.json considers all navigation items (cherry picked from commit 40f1332d45593f7c0bbbaf08ac540807ad5b5395) * remove commented line * update test file output order
Hiddenis a pure navigation property now as its removed fromMarkdownFile.NavigationIndexis a pure navigation property now as its removed fromMarkdownFile.GetCurrentNavigation()is now non nullable