Feature/pretty docs with theme #413
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis PR transforms the documentation site by redesigning the homepage with a comprehensive inline layout, introducing a unified CSS stylesheet for enhanced theming and styling, and substantially expanding MkDocs configuration to include theme customization, plugin integration, markdown extensions, and API documentation generation capabilities. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes span multiple files with substantial configuration and styling additions, but they are largely additive and follow consistent patterns. The primary complexity lies in verifying the breadth of mkdocs.yml configuration updates and ensuring CSS styling cohesion across the new homepage design, rather than intricate logic density. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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. Comment |
…ith-theme # Conflicts: # docs/user-guide/migration-guide-v3.md # mkdocs.yml
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
pyproject.toml (1)
97-111: Pin MkDocs core to a stable minor to avoid plugin breakages.Given the expanded plugin set, please pin MkDocs itself in docs extras (e.g., ~=1.6) to ensure compatibility and reproducible builds.
Apply:
docs = [ + "mkdocs>=1.6,<1.7", "mkdocs-material==9.6.21", "mkdocstrings-python==1.18.2", "mkdocs-table-reader-plugin==3.1.0", "mkdocs-gen-files==0.5.0", "mkdocs-include-markdown-plugin==7.1.7", "mkdocs-literate-nav==0.6.2", "markdown-include==0.8.1", "pymdown-extensions==10.16.1", "pygments==2.19.2", "mike==2.1.3", "mkdocs-git-revision-date-localized-plugin==1.4.7", "mkdocs-minify-plugin==0.8.0", ]docs/index.md (2)
8-177: Move inline CSS into docs/stylesheets/extra.css.Centralize styling to avoid duplication and ease maintenance; rely on extra.css loaded via mkdocs.yml.
Apply:
-<style> -/* (moved to docs/stylesheets/extra.css) */ -... all inline CSS ... -</style>Then paste the removed rules at the end of docs/stylesheets/extra.css (keeping selectors identical).
187-195: Add rel="noopener noreferrer" to external links opened in new tabs.Prevents reverse‑tabnabbing and improves security.
- <a href="https://github.com/flixOpt/flixopt" class="md-button">⭐ GitHub</a> + <a href="https://github.com/flixOpt/flixopt" class="md-button" target="_blank" rel="noopener noreferrer">⭐ GitHub</a>-[Visit Repository →](https://github.com/flixOpt/flixopt){target="_blank"} +[Visit Repository →](https://github.com/flixOpt/flixopt){target="_blank" rel="noopener noreferrer"}-[Join Discussion →](https://github.com/flixOpt/flixopt/discussions){target="_blank"} +[Join Discussion →](https://github.com/flixOpt/flixopt/discussions){target="_blank" rel="noopener noreferrer"}-[Learn How →](contribute/){target="_blank"} +[Learn How →](contribute/){target="_blank" rel="noopener noreferrer"}Also applies to: 259-285
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/index.md(1 hunks)docs/stylesheets/extra.css(1 hunks)mkdocs.yml(2 hunks)pyproject.toml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.10)
- GitHub Check: test (3.13)
🔇 Additional comments (2)
docs/stylesheets/extra.css (1)
1-223: Looks solid; scoped, theme‑aware overrides.No blocking issues spotted; styles align with Material’s variables and structure.
mkdocs.yml (1)
42-319: Verify assets exist and plugin packages are declared; prevent build breaks.Before merging, please confirm all referenced files exist and plugin packages are present in pyproject docs extras.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/index.md (1)
128-136: CTA section styling uses inline styles.The call-to-action uses inline
styleattribute for layout and theming. Consider moving this to the extra.css stylesheet (as a.cta-sectionor similar class) for better maintainability and consistency with the rest of the CSS enhancements.Example refactor: Define a CSS class in extra.css and apply it to the div instead of using inline
style./* In docs/stylesheets/extra.css */ .cta-section { text-align: center; margin: 3rem 0; padding: 2rem; background: var(--md-code-bg-color); border-radius: 0.75rem; }Then update the HTML:
-<div style="text-align: center; margin: 3rem 0; padding: 2rem; background: var(--md-code-bg-color); border-radius: 0.75rem;"> +<div class="cta-section">docs/stylesheets/extra.css (2)
237-245: Gradient text uses vendor prefixes; verify browser compatibility & accessibility.Lines 242-244 use
-webkit-background-clip: textand-webkit-text-fill-colorto create gradient text on the hero h1. While this works in modern browsers, older browsers (IE, early Firefox) won't render the gradient and may show no text or fallback poorly.Consider adding a fallback solid color for better robustness:
.hero-section h1 { font-size: 3.5rem; font-weight: 700; margin-bottom: 1rem; + color: var(--flixopt-teal); /* Fallback for older browsers */ background: linear-gradient(135deg, #009688 0%, #00796B 100%); -webkit-background-clip: text; -webkit-text-fill-color: transparent; background-clip: text; }Also, screen readers may struggle with gradient text. Ensure the heading text is semantically clear in the HTML (it is, via
<h1>flixOpt</h1>), but consider testing with a screen reader.
278-282: Transform hover effects may feel jarring on touch devices.The
.feature-card:hoverand.quick-link-card:hoverusetransform: translateY/Xwith easing. On touch devices (tablets, mobile), these hover effects won't trigger, but the CSS will still load and the animation code remains. This is generally acceptable, but consider adding a@media (hover: hover)query to ensure smooth interactions are only applied to devices that support hover.Example:
@media (hover: hover) { .feature-card:hover { transform: translateY(-4px); box-shadow: 0 8px 16px rgba(0, 0, 0, 0.1); } }This prevents unnecessary animation resources on touch devices.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/index.md(1 hunks)docs/stylesheets/extra.css(1 hunks)pyproject.toml(3 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/index.md
[grammar] ~58-~58: There might be a mistake here.
Context: ...a> ## 🏗️ Framework Architecture
Conceptual Usage and IO operations of FlixOpt FlixOpt provides a complete workflow f...
(QB_NEW_EN)
[style] ~120-~120: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1571 characters long)
Context: ...ial-file-document-edit: Recent Updates !!! tip "What's New in v3.0.0" Major im...
(EN_EXCESSIVE_EXCLAMATION)
[grammar] ~124-~124: There might be a mistake here.
Context: ...hangelog/) for detailed version history. ---
Ready to optimize your energy system?
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.11)
- GitHub Check: test (3.10)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
🔇 Additional comments (9)
pyproject.toml (2)
37-37: Enhanced dependency comments improve clarity.The clarifications on version constraints (CalVer annotation, patch-to-minor widening rationale, Python version-specific workaround) are helpful for future maintainers.
Also applies to: 39-39, 51-51
99-111: New docs tooling additions look appropriate.The mkdocs ecosystem plugins (material theme, docstrings, git-revision-date, minify) align with the PR's documentation enhancement objectives and appear to be stable, well-maintained packages.
docs/index.md (3)
1-6: Homepage frontmatter configuration is solid.Hiding navigation and TOC on the home page is the right choice for a focused, hero-driven landing experience. Metadata is correctly structured.
24-56: Quick navigation cards provide clear wayfinding.Six well-organized quick-links with descriptive text and consistent icons/emoji guide new users effectively to key sections (Getting Started, Examples, API Reference, Recipes, Math, Roadmap).
119-125: Recent Updates section references specific version (v3.0.0).The admonition hardcodes "v3.0.0" and assumes a migration guide exists. If this is aspirational or not yet released, consider making it more generic or adding a fallback. Also verify
user-guide/migration-guide-v3.mdexists (included in link verification above).docs/stylesheets/extra.css (4)
6-17: CSS custom properties for theming are well-structured.Root variables for spacing, typography, and colors enable consistent, maintainable styling and make future theme adjustments straightforward.
75-84: Code block shadows are appropriately tuned for dark/light modes.The
.highlightstyling includes theme-aware shadows (0.1 opacity for light, 0.3 for dark), which is a nice UX touch that maintains readability in both modes.
213-223: Print styles appropriately hide non-essential UI elements.Hiding header, sidebar, and footer on print is a solid pattern that ensures printed docs are readable and concise.
375-396: Responsive breakpoints are well-placed.Breakpoints at 76.1875em (~1230px) and 44.9375em (~720px) plus the mobile-specific override at 768px cover common viewport sizes. The responsive rules appropriately adjust hero font sizes, card layouts, and button stacking for smaller screens.
| <a href="api-reference/" class="quick-link-card"> | ||
| <h3>📚 API Reference</h3> | ||
| <p>Detailed documentation of all classes, methods, and parameters</p> | ||
| </a> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify internal link targets exist.
Multiple relative links (getting-started/, examples/, api-reference/, user-guide/recipes/, user-guide/mathematical-notation/, roadmap/, contribute/, changelog/) should be verified to ensure they resolve correctly in the built site. Broken links will degrade the homepage experience.
Run the following script to check that link targets exist in the docs structure:
🏁 Script executed:
#!/bin/bash
# Description: Verify that all internal homepage links target existing pages/directories
# List all referenced link targets from the homepage
links=(
"getting-started"
"examples"
"api-reference"
"user-guide/recipes"
"user-guide/mathematical-notation"
"roadmap"
"contribute"
"changelog"
"user-guide/migration-guide-v3"
)
echo "Checking if link targets exist in docs/..."
for link in "${links[@]}"; do
# Check for either .md file or index.md in directory
if [ -f "docs/${link}.md" ] || [ -f "docs/${link}/index.md" ]; then
echo "✓ ${link}"
else
echo "✗ MISSING: ${link}"
fi
doneLength of output: 228
🏁 Script executed:
#!/bin/bash
# Check actual docs directory structure and find where api-reference and changelog might be
echo "=== Full docs directory structure ==="
find docs -type d -o -type f -name "*.md" | head -50
echo -e "\n=== Looking for 'api' or 'reference' files ==="
find docs -type f \( -name "*api*" -o -name "*reference*" \) 2>/dev/null
echo -e "\n=== Looking for 'changelog' or 'change' files ==="
find docs -type f \( -name "*changelog*" -o -name "*change*" -o -name "*history*" \) 2>/dev/null
echo -e "\n=== Checking docs root files ==="
ls -la docs/Length of output: 2403
🏁 Script executed:
#!/bin/bash
# Explore docs structure to find api-reference and changelog
echo "=== Checking docs directory structure ==="
find docs -type d | sort
echo -e "\n=== Looking for api-reference ==="
find docs -type f -o -type d | grep -i "api"
echo -e "\n=== Looking for changelog ==="
find docs -type f -o -type d | grep -i "change"
echo -e "\n=== All .md files in docs ==="
find docs -name "*.md" | sortLength of output: 1703
Create missing documentation pages for api-reference and changelog.
Verification confirms two broken links on the homepage:
api-reference/– link target does not exist in docschangelog/– link target does not exist in docs
These missing pages will result in 404 errors for users. Create docs/api-reference/index.md and docs/changelog.md (or as directories with index.md) to resolve the broken links.
🤖 Prompt for AI Agents
In docs/index.md around lines 37 to 40, the links to "api-reference/" and
"changelog/" point to non-existent targets causing 404s; create the missing
documentation pages by adding docs/api-reference/index.md (or a
docs/api-reference.md) and docs/changelog.md (or a docs/changelog/index.md) with
basic frontmatter/title and placeholder content that links back to the homepage
so the links resolve and users don’t hit 404s.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
mkdocs.yml (2)
143-145: Verify snippet base path is correct for include-markdown.Line 143–145 sets snippet
base_path: .., which is relative to thedocs/directory. If markdown includes reference files outsidedocs/, confirm paths resolve correctly. Document or hard-code the path if clarity is needed.For clarity, consider using an absolute path or documenting the assumption:
- pymdownx.snippets: - base_path: .. + base_path: . # Resolve relative to docs/ directory check_paths: trueOr add an inline comment explaining the relative path.
86-120: Review feature flags for redundancy and performance impact.Lines 86–120 enable 20+ navigation, TOC, search, content, and header features. Some combinations may be redundant or degrade performance:
navigation.instant+navigation.instant.prefetch+navigation.instant.progresstogether provide instant loading (ensure prefetch doesn't bloat bundle).toc.follow+toc.integratetogether (line 102–103) may create confusing navigation behavior; confirm this is intentional.- All
navigation.*features enabled may slow page transitions; consider disabling less critical ones.No immediate issues, but recommend testing build time and page load performance to ensure acceptable UX.
If performance becomes a concern, disable less-used features:
- - navigation.instant.progress + # - navigation.instant.progress # Disable if bloats bundle - - navigation.expand # Expand navigation by default + # - navigation.expand # Disable if impacts performance
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/overrides/main.html(1 hunks)mkdocs.yml(2 hunks)
🔇 Additional comments (6)
mkdocs.yml (5)
72-74: Font fallbacks are automatically handled by Material theme—no changes needed.Material for MkDocs automatically includes fallback fonts when loading Google Fonts like Inter and Fira Code, using CSS variables with system fallback stacks. Your configuration at lines 72–74 will gracefully degrade if the CDN is unavailable without requiring manual fallback comments in the YAML.
202-208: Script verification passed:scripts/gen_ref_pages.pyis correctly implemented and configured.✓ Script exists at
scripts/gen_ref_pages.py(1984 bytes)
✓ Properly importsmkdocs_gen_fileswith correct API usage
✓ Dependencymkdocs-gen-files==0.5.0declared inpyproject.toml
✓ Logic is sound: processes Python modules, generates markdown with mkdocstrings syntax, handles special cases (init.py → index.md, skips main and private modules), creates API reference index
✓ Includes validation output (prints generated file count)The configuration at lines 202–208 in
mkdocs.ymlis valid and will execute correctly.
210-254: mkdocstrings configuration is appropriate for the codebase.Verification confirms the codebase uses exclusively Google-style docstrings. Pattern matches for "Parameters" were false positives from variable names and type hints (e.g.,
OnOffParameters,on_off_parameters), not actual NumPy-style docstrings. The config settingdocstring_style: googleat line 222 correctly matches the codebase conventions. No style-related issues will cause silent documentation failures.
288-304: Review comment is valid—environment variable configuration gap exists but with graceful degradation.The
!ENV GOOGLE_ANALYTICS_KEYreference in mkdocs.yml is undocumented and not configured in the deploy-docs CI/CD workflow. However, verification shows:
Build will not fail: When environment variables are absent in mkdocs, they return null by default, causing graceful degradation (analytics disabled) rather than build failures. No strict mode is enabled in mkdocs.yml to enforce variable presence.
Configuration gap confirmed: The variable is not defined in CI/CD pipelines, .env files, or documentation. This means analytics will be silently disabled unless the variable is set in the production environment.
Feedback form is intentional: Lines 291–304 configure user feedback intentionally. Privacy compliance for feedback collection should be documented separately.
Developer action: Verify that
GOOGLE_ANALYTICS_KEYis configured in your production environment, or document that analytics is intentionally disabled during local/CI builds. Consider adding a.env.examplefile or build documentation if tracking this variable is important for your deployment workflow.
186-273: All plugin and extension dependencies are properly declared with compatible versions—no action required.Verification confirms:
- All 10 plugins referenced in mkdocs.yml (lines 186–273) have corresponding dependencies in
pyproject.tomlwith pinned versions- mkdocs-material 9.6.21 is compatible with MkDocs 1.6.1
- mkdocstrings-python 1.18.2 is compatible with MkDocs 1.6
- pymdown-extensions 10.16.1 is compatible with the MkDocs setup
All three checks from the review comment have been satisfied.
docs/overrides/main.html (1)
1-14: The widget ID is not a sensitive credential and does not require secretization.The Gurubase widget ID is not a secret credential — it's intended to be embedded in client-side widget integrations. The review conflates the public widget ID with API credentials (x-api-key), which are distinct. The hardcoded widget ID is architecturally correct and aligns with Gurubase's domain-specific, origin-validated security model.
Additionally, the codebase does not establish a pattern of externalizing public configuration to environment variables—no .env files or env var usage exists for template configuration. The proposed diff contradicts the project's architectural patterns.
The code as written is correct. The optional suggestion to add documentation comments about the Gurubase integration's purpose is reasonable but not mandatory.
Likely an incorrect or invalid review comment.
mkdocs.yml
Outdated
| language: en | ||
| custom_dir: docs/overrides |
There was a problem hiding this comment.
🧩 Analysis chain
Confirm docs/overrides directory exists and is properly structured.
Lines 44–45 introduce language: en and custom_dir: docs/overrides, enabling the new override template. Verify the directory structure matches Material theme expectations and that docs/overrides/main.html is the only override needed.
🏁 Script executed:
#!/bin/bash
# Verify overrides directory structure
ls -la docs/overrides/
find docs/overrides -type fLength of output: 191
I need to verify the content of main.html and check for any other references to custom files.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check main.html content
echo "=== main.html content ==="
cat docs/overrides/main.html
echo ""
echo "=== Checking for other override file references in mkdocs.yml ==="
rg -n "(custom_dir|overrides)" mkdocs.yml
Length of output: 597
Found a potential issue. The mkdocs.yml references overrides/.icons but that directory doesn't exist. Let me verify:
---
🏁 Script executed:
```shell
#!/bin/bash
# Check if .icons directory exists anywhere
echo "=== Searching for .icons directory ==="
find . -type d -name ".icons" 2>/dev/null || echo "Not found in standard search"
echo ""
echo "=== Checking line 179 context in mkdocs.yml ==="
sed -n '175,185p' mkdocs.yml
Length of output: 372
Create missing docs/overrides/.icons directory or remove the custom icons configuration.
The docs/overrides/main.html exists and is properly structured, but mkdocs.yml (line 179) configures custom_icons: [overrides/.icons] which references a non-existent directory. Either create the .icons directory with custom icon definitions or remove the custom_icons configuration line to use Material theme's defaults.
🤖 Prompt for AI Agents
In mkdocs.yml around lines 44-45 (and referenced at line 179) the configured
custom_icons path overrides/.icons does not exist; either create the missing
directory at docs/overrides/.icons and add your custom icon definition files
(ensure filenames and icon definitions follow Material theme expectations), or
remove the custom_icons: [overrides/.icons] entry from mkdocs.yml so the theme
uses its default icons; update the repo (create the directory and commit icon
files or edit mkdocs.yml) and run a local build to verify no missing-path
warnings.
| watch: | ||
| - flixopt | ||
| - docs |
There was a problem hiding this comment.
🧩 Analysis chain
Confirm watch paths don't create unintended rebuild loops.
Lines 318–320 add docs to watch paths for hot-reloading during development. Ensure:
- Generated files in
docs/(e.g., fromgen_ref_pages.py) don't trigger infinite rebuilds. - Build artifacts (e.g.,
.cache/) are.gitignore'd to prevent watching them.
This is a best practice addition but verify it doesn't conflict with the gen-files plugin.
🏁 Script executed:
#!/bin/bash
# Check .gitignore for build artifacts
echo "=== Checking .gitignore for build artifacts ==="
grep -E '\.cache|\.gen|\.tmp|build|dist' .gitignore || echo "May need to add build artifact patterns"Length of output: 87
---
🏁 Script executed:
```shell
#!/bin/bash
# 1. Check actual .gitignore contents
echo "=== Full .gitignore contents ==="
cat .gitignore || echo "No .gitignore found"
echo -e "\n=== Search for gen_ref_pages or similar ==="
fd -t f 'gen.*\.py' -o -name '*gen*pages*'
echo -e "\n=== Check mkdocs.yml for gen-files plugin ==="
grep -A 10 'gen.files\|gen-files' mkdocs.yml || echo "gen-files not found in mkdocs.yml"
echo -e "\n=== Check if docs/ directory is in .gitignore ==="
grep -E '^docs/?$|^docs/.*' .gitignore | head -5 || echo "docs/ not explicitly in .gitignore"
Length of output: 718
---
🏁 Script executed:
```shell
#!/bin/bash
# 1. Search for gen_ref_pages.py more broadly
echo "=== Search for gen_ref_pages.py ==="
find . -name 'gen_ref_pages.py' 2>/dev/null || echo "Not found with find"
echo -e "\n=== List scripts directory ==="
ls -la scripts/ 2>/dev/null || echo "scripts/ not found"
echo -e "\n=== Check mkdocs.yml for site_dir and watch config ==="
grep -E 'site_dir|watch:|cache' mkdocs.yml | head -15
echo -e "\n=== Search entire mkdocs.yml around watch section ==="
grep -B 5 -A 10 'watch:' mkdocs.yml
Length of output: 733
---
🏁 Script executed:
```shell
#!/bin/bash
# Inspect gen_ref_pages.py to understand what it generates
echo "=== gen_ref_pages.py contents ==="
cat ./scripts/gen_ref_pages.py
echo -e "\n=== Check if mkdocs.yml has site_dir (default is 'site') ==="
grep 'site_dir' mkdocs.yml || echo "Using default site_dir (site)"
echo -e "\n=== Check if .cache would be created by mkdocs ==="
grep -r 'cache' . --include='*.md' --include='*.py' 2>/dev/null | head -3 || echo "No cache refs found"
Length of output: 2425
Add .cache and other build artifacts to .gitignore.
The original concern is partially valid: mkdocs serve with watch: [flixopt, docs] will create a .cache/ directory for incremental builds, but this is not gitignored. Update .gitignore to include:
.cache/
site/
The risk of infinite rebuild loops from gen-files is mitigated by the mkdocs_gen_files library, which tracks generated files to prevent re-triggering on their own output. The setup is sound, but the missing build artifacts in .gitignore should be addressed.
🤖 Prompt for AI Agents
In mkdocs.yml around lines 318 to 320, the project currently watches flixopt and
docs which causes mkdocs to create a .cache/ directory and build artifacts
(site/) during serve/build; update the repository’s .gitignore to add entries
for .cache/ and site/ (and any other generated build artifact directories) so
these incremental build outputs are not committed, ensuring .cache/ and site/
are ignored by Git.
FBumann
force-pushed
the
feature/pretty-docs-with-theme
branch
from
3026e9c to
336e04d
Compare
Description
Improve the docs visually
Type of Change
Related Issues
Closes #(issue number)
Testing
Checklist
Summary by CodeRabbit