fix: solvee the header related issue #249
Conversation
WalkthroughThe changes remove a GitHub Actions workflow step that managed the robots.txt file on the Changes
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
docs/overrides/partials/header.html (2)
6-7: Consider externalizing inline scripts for caching
Embedding JavaScript directly in the template limits caching and minification opportunities. Extract this logic into a separate.jsfile (e.g.,_static/version-selector.js) and load it with:<script defer src="{{ pathto('_static/version-selector.js') }}"></script>within the header block to improve performance and manageability.
13-15: Robust version detection using URL API
The regex check can pick uplatestin query strings or other URL parts. Consider using theURLAPI to parse and inspect path segments explicitly:const url = new URL(window.location.href); const segments = url.pathname.split('/').filter(Boolean); const isLatestVersion = segments.includes('latest');This makes the logic clearer and less error-prone.
docs/overrides/partials/outdated.html (1)
5-7: Avoid hard-coded absolute URLs
Linking directly tohttps://pyretailscience.datasimply.co/latest/couples the template to a specific domain. Use a templated base URL (e.g.,{{ config.site_url }}) or a relative path ({{ pathto('latest/') }}) so the link adapts across environments (staging, production, custom domains).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.github/workflows/gh-page.yml(0 hunks)docs/overrides/partials/header.html(1 hunks)docs/overrides/partials/outdated.html(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/gh-page.yml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Pre-Commit
🔇 Additional comments (1)
docs/overrides/partials/header.html (1)
1-5: Proper use of template inheritance
Extendingbase.htmland encapsulating the header content within{% block header %}, while preserving the parent block with{{ super() }}, is a solid approach that enhances maintainability and ensures shared layout isn’t duplicated.
| var versionSelector = document.querySelector('.md-header__topic .md-ellipsis'); | ||
|
|
||
| if (versionSelector) { | ||
| // Get current version from the URL | ||
| var currentPath = window.location.pathname; | ||
| var isLatestVersion = currentPath.includes('/latest/') || | ||
| (window.location.href.includes('latest') && !window.location.href.match(/\/\d+\.\d+\.\d+\//)); | ||
|
|
||
| // If this is the latest version or has the latest alias, change the displayed text | ||
| if (isLatestVersion) { | ||
| var originalText = versionSelector.textContent; | ||
| versionSelector.textContent = originalText.replace(/\d+\.\d+\.\d+/, 'latest'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use modern const/let instead of var
Switch the var declarations (versionSelector, currentPath, isLatestVersion) to const (or let if reassignment is needed) to leverage block scoping and prevent hoisting surprises.
- var versionSelector = document.querySelector('.md-header__topic .md-ellipsis');
+ const versionSelector = document.querySelector('.md-header__topic .md-ellipsis');
- var currentPath = window.location.pathname;
+ const currentPath = window.location.pathname;
- var isLatestVersion = currentPath.includes('/latest/') ||
- (window.location.href.includes('latest') && !window.location.href.match(/\/\d+\.\d+\.\d+\//));
+ const isLatestVersion = currentPath.includes('/latest/') ||
+ (window.location.href.includes('latest') && !window.location.href.match(/\/\d+\.\d+\.\d+\//));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var versionSelector = document.querySelector('.md-header__topic .md-ellipsis'); | |
| if (versionSelector) { | |
| // Get current version from the URL | |
| var currentPath = window.location.pathname; | |
| var isLatestVersion = currentPath.includes('/latest/') || | |
| (window.location.href.includes('latest') && !window.location.href.match(/\/\d+\.\d+\.\d+\//)); | |
| // If this is the latest version or has the latest alias, change the displayed text | |
| if (isLatestVersion) { | |
| var originalText = versionSelector.textContent; | |
| versionSelector.textContent = originalText.replace(/\d+\.\d+\.\d+/, 'latest'); | |
| const versionSelector = document.querySelector('.md-header__topic .md-ellipsis'); | |
| if (versionSelector) { | |
| // Get current version from the URL | |
| const currentPath = window.location.pathname; | |
| const isLatestVersion = currentPath.includes('/latest/') || | |
| (window.location.href.includes('latest') && !window.location.href.match(/\/\d+\.\d+\.\d+\//)); | |
| // If this is the latest version or has the latest alias, change the displayed text | |
| if (isLatestVersion) { | |
| var originalText = versionSelector.textContent; | |
| versionSelector.textContent = originalText.replace(/\d+\.\d+\.\d+/, 'latest'); |
🤖 Prompt for AI Agents
In docs/overrides/partials/header.html between lines 9 and 20, replace all var
declarations for versionSelector, currentPath, and isLatestVersion with const if
they are not reassigned, or let if reassignment occurs. This change will use
modern block-scoped variable declarations to improve code clarity and prevent
hoisting issues.
| <div class="md-banner md-banner--warning"> | ||
| {% if config.extra.version and config.extra.version.provider == 'mike' %} | ||
| {% if mike and (mike.version.aliases | length == 0 or "latest" not in mike.version.aliases) %} | ||
| <div class="md-banner__inner md-typeset"> | ||
| <strong>You're viewing an older version of the PyRetailScience documentation.</strong> | ||
| <a href="{{ '/' | url }}../latest/"> | ||
| <strong>Click here to go to the latest stable version.</strong> | ||
| </a> | ||
| </div> | ||
| {% endif %} | ||
| {% if not page.is_homepage %} | ||
| <div class="md-banner__inner md-typeset"> | ||
| <strong>You're viewing an older version of the PyRetailScience documentation.</strong> | ||
| <a href="https://pyretailscience.datasimply.co/latest/"> | ||
| <strong>Click here to go to the latest stable version.</strong> | ||
| </a> | ||
| </div> | ||
| {% endif %} |
There was a problem hiding this comment.
Prevent empty banner on the homepage
The <div class="md-banner md-banner--warning"> wrapper is always rendered, but its inner content is conditional on not page.is_homepage, leaving an empty element on the homepage. Move the {% if not page.is_homepage %} to wrap the entire banner block (both opening and closing <div>) to avoid rendering empty containers:
- <div class="md-banner md-banner--warning">
- {% if not page.is_homepage %}
- <div class="md-banner__inner md-typeset">
- …
- </div>
- {% endif %}
- </div>
+ {% if not page.is_homepage %}
+ <div class="md-banner md-banner--warning">
+ <div class="md-banner__inner md-typeset">
+ …
+ </div>
+ </div>
+ {% endif %}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div class="md-banner md-banner--warning"> | |
| {% if config.extra.version and config.extra.version.provider == 'mike' %} | |
| {% if mike and (mike.version.aliases | length == 0 or "latest" not in mike.version.aliases) %} | |
| <div class="md-banner__inner md-typeset"> | |
| <strong>You're viewing an older version of the PyRetailScience documentation.</strong> | |
| <a href="{{ '/' | url }}../latest/"> | |
| <strong>Click here to go to the latest stable version.</strong> | |
| </a> | |
| </div> | |
| {% endif %} | |
| {% if not page.is_homepage %} | |
| <div class="md-banner__inner md-typeset"> | |
| <strong>You're viewing an older version of the PyRetailScience documentation.</strong> | |
| <a href="https://pyretailscience.datasimply.co/latest/"> | |
| <strong>Click here to go to the latest stable version.</strong> | |
| </a> | |
| </div> | |
| {% endif %} | |
| {% if not page.is_homepage %} | |
| <div class="md-banner md-banner--warning"> | |
| <div class="md-banner__inner md-typeset"> | |
| <strong>You're viewing an older version of the PyRetailScience documentation.</strong> | |
| <a href="https://pyretailscience.datasimply.co/latest/"> | |
| <strong>Click here to go to the latest stable version.</strong> | |
| </a> | |
| </div> | |
| </div> | |
| {% endif %} |
🤖 Prompt for AI Agents
In docs/overrides/partials/outdated.html around lines 1 to 9, the md-banner div
is always rendered but its inner content is conditional, causing an empty banner
on the homepage. Move the {% if not page.is_homepage %} condition to wrap the
entire md-banner div block, including both the opening and closing tags, so the
banner is only rendered when not on the homepage, preventing empty containers.
Codecov ReportAll modified and coverable lines are covered by tests ✅ 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
Chores
Refactor
Style