Open
Conversation
Reviewer's GuideReplaces the Bootstrap-based Jekyll top navigation with a custom Kruize-branded header that matches the home page, and adds corresponding CSS plus improved styling for the search box and results dropdown. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new header hardcodes the primary nav links and drops the previous
site.data[page.topnav]-driven menu logic, which removes configurability and active-state handling; consider preserving the data-driven structure while updating the visual style. - Inline styles on the search container and input in
topnav.htmlcould be moved intocustomstyles.cssto keep presentation concerns in CSS and make future styling changes easier. - The
.kruize-header-active .navbarrule seems unused now that the Bootstrap navbar markup has been removed; consider deleting or repurposing this selector to avoid dead CSS.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new header hardcodes the primary nav links and drops the previous `site.data[page.topnav]`-driven menu logic, which removes configurability and active-state handling; consider preserving the data-driven structure while updating the visual style.
- Inline styles on the search container and input in `topnav.html` could be moved into `customstyles.css` to keep presentation concerns in CSS and make future styling changes easier.
- The `.kruize-header-active .navbar` rule seems unused now that the Bootstrap navbar markup has been removed; consider deleting or repurposing this selector to avoid dead CSS.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+21
to
+23
| <div style="padding: 10px 20px; background: #f8f8f8; border-bottom: 1px solid #ddd;"> | ||
| <div id="search-demo-container" style="max-width: 300px;"> | ||
| <input type="text" id="search-input" placeholder="{{site.data.strings.search_placeholder_text}}" style="width: 100%; padding: 8px; border: 1px solid #ccc; border-radius: 4px;"> |
Contributor
There was a problem hiding this comment.
Can we move these inline css to a separate css file for better maintainability. Also please ensure we are following the same pattern across the code base
Comment on lines
+5
to
+12
| /* Kruize Home Page Header Styles */ | ||
| .kruize-header { | ||
| background-color: #f4f4f4; | ||
| color: #004E98; | ||
| display: flex; | ||
| justify-content: space-between; | ||
| align-items: center; | ||
| padding: 15px 20px; |
Contributor
There was a problem hiding this comment.
We should have a separate css file that contains css styles related to the kruize Home Page Header , it should not be present in the docs folder with docs css changes .
Comment on lines
+20
to
+28
| .kruize-header .nav { | ||
| display: flex; | ||
| align-items: center; | ||
| gap: 20px; | ||
| } | ||
|
|
||
| .kruize-header .nav a { | ||
| color: #004E98; | ||
| text-decoration: none; |
Contributor
There was a problem hiding this comment.
There is duplication of css style in this customstyles.css and main style.css both are defining the same properties. Is this duplication needed? If not consider having a single css file so that its easy to understand which css file is for which section/page of the website
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
changing jekyll banner to match home page
Summary by Sourcery
Align the documentation site’s top navigation and search layout with the Kruize home page header design.
New Features:
Enhancements: