-
Notifications
You must be signed in to change notification settings - Fork 31
Refactor navigation to be more anemic. #781
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
This allows us to inject navigation using the assembler build (not yet implemented) without having to resolve all markdown files
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 the navigation rendering to a more anemic design by introducing an interface (INavigationHtmlWriter) and updating the related navigation and TOC processing components. Key changes include extracting navigation rendering to a dedicated interface and class, removing the in_nav flag from TOC configurations, and updating the HTMX attribute generation logic.
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/Elastic.Markdown/Slices/HtmlWriter.cs | Extracts navigation rendering into INavigationHtmlWriter and updates dependency injection |
src/Elastic.Markdown/Myst/InlineParsers/DiagnosticLinkInlineParser.cs | Updates link data assignment for navigation information in diagnostic links |
src/Elastic.Markdown/IO/Navigation/DocumentationGroup.cs | Refactors DocumentationGroup to implement INavigation and use topLevelGroup for navigation consistency |
src/Elastic.Markdown/Myst/Renderers/HtmxLinkInlineRenderer.cs | Removes BuildContext dependency and simplifies HTMX attribute generation |
src/Elastic.Markdown/IO/MarkdownFile.cs | Implements INavigationScope and ITableOfContentsScope and initializes RootNavigation |
src/Elastic.Markdown/Helpers/Htmx.cs | Simplifies HTMX helper methods by removing feature flag parameters from attribute construction |
Other cshtml files | Adapt HTMX attribute calls to the new API and remove unused navigation properties |
Comments suppressed due to low confidence (2)
src/Elastic.Markdown/Myst/InlineParsers/DiagnosticLinkInlineParser.cs:197
- [nitpick] Consider using 'nameof(MarkdownFile.RootNavigation)' as the key instead of 'nameof(currentMarkdown.RootNavigation)' to ensure consistency across the codebase.
link.SetData(nameof(currentMarkdown.RootNavigation), currentMarkdown.RootNavigation);
src/Elastic.Markdown/Helpers/Htmx.cs:29
- Wrap the attribute value in quotes (e.g. hx-get="{targetUrl}") to ensure that the generated HTML is valid.
_ = attributes.Append($" hx-get={targetUrl}");
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.
Thanks for cleaning up!
I just noticed this introduced a visual bug. |
This is maybe also related: #788 |
This allows us to inject navigation using the assembler build (not yet implemented) without having to resolve all markdown files.
This still supports the PrimaryNav feature flag for local builds.
cursorful-video-1742410256917.mp4