docs: introduce Mintlify-inspired Material theme#2701
Conversation
|
Overall readability score: 53.98 (🔴 -0.23)
View detailed metrics🟢 - Shows an increase in readability
Averages:
View metric targets
|
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 177 |
| Duplication | 0 |
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Code Review
This pull request introduces a major redesign of the Codacy documentation theme, replacing the legacy submodule with a custom, plain-CSS theme layer. It updates dependencies to mkdocs-material v9.7.6, adds custom templates, stylesheets, and asset delivery, and introduces a Python build hook to inject image metadata. The review feedback highlights a critical DOM-based XSS vulnerability in the 404 page redirect script, accessibility issues with keyboard tab-trapping on fixed-position elements in the search and navigation drawers, and a potential file descriptor leak in the SVG parsing logic of the image metadata hook.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| (function () { | ||
| if (window.location.pathname.startsWith('/hc/requests/') || window.location.pathname.startsWith('/hc/en-us/requests')) { | ||
| var destination = 'https://codacy.zendesk.com' + window.location.pathname; | ||
| document.querySelector('#docs-404-message').innerHTML = 'You should be redirected to <a href="' + destination + '">' + destination + '</a> shortly.'; |
There was a problem hiding this comment.
Using innerHTML to insert window.location.pathname (via destination) creates a DOM-based Cross-Site Scripting (XSS) vulnerability. An attacker could craft a malicious URL containing HTML/JavaScript (e.g., <script> tags or inline event handlers) which would execute in the context of the user's session when rendered on the 404 page.\n\nTo resolve this securely, avoid innerHTML and construct the DOM elements dynamically using textContent and href properties.
var messageEl = document.querySelector('#docs-404-message');\n messageEl.textContent = 'You should be redirected to ';\n var linkEl = document.createElement('a');\n linkEl.href = destination;\n linkEl.textContent = destination;\n messageEl.appendChild(linkEl);\n messageEl.appendChild(document.createTextNode(' shortly.'));
| var searchToggle = document.querySelector("#__search"); | ||
| if (event.key === "Tab" && searchToggle && searchToggle.checked) { | ||
| var dialog = document.querySelector("#docs-search"); | ||
| var focusable = dialog ? Array.from(dialog.querySelectorAll("input, button:not([disabled]), a[href], [tabindex]:not([tabindex='-1'])")).filter(function (element) { return element.offsetParent !== null; }) : []; |
There was a problem hiding this comment.
The search dialog is styled with position: fixed (defined in theme/stylesheets/theme.css). In many modern browsers, elements that are position: fixed or reside inside a position: fixed container will return null for offsetParent. As a result, the filter element.offsetParent !== null will evaluate to false for all elements inside the search dialog, breaking the keyboard tab-trapping accessibility feature entirely.\n\nTo fix this, use a more robust visibility check such as checking the element's dimensions or using getClientRects().length > 0.
var focusable = dialog ? Array.from(dialog.querySelectorAll(\"input, button:not([disabled]), a[href], [tabindex]:not([tabindex='-1'])\")).filter(function (element) { return element.offsetWidth > 0 || element.offsetHeight > 0 || element.getClientRects().length > 0; }) : [];| var drawerToggle = document.querySelector("#__drawer"); | ||
| if (event.key === "Tab" && drawerToggle && drawerToggle.checked) { | ||
| var drawer = document.querySelector("#docs-navigation"); | ||
| var drawerFocusable = drawer ? Array.from(drawer.querySelectorAll("a[href], label[tabindex='0'], button:not([disabled]), [tabindex]:not([tabindex='-1'])")).filter(function (element) { return element.offsetParent !== null; }) : []; |
There was a problem hiding this comment.
Similar to the search dialog, the mobile navigation drawer may be styled with fixed or absolute positioning. Using element.offsetParent !== null can fail to identify visible focusable elements in these layouts. Replacing it with a dimension or client rect check ensures robust keyboard tab-trapping.
var drawerFocusable = drawer ? Array.from(drawer.querySelectorAll(\"a[href], label[tabindex='0'], button:not([disabled]), [tabindex]:not([tabindex='-1'])\")).filter(function (element) { return element.offsetWidth > 0 || element.offsetHeight > 0 || element.getClientRects().length > 0; }) : [];| if width and height: | ||
| dimensions = (str(width), str(height)) | ||
| elif suffix == ".svg": | ||
| _, root = next(ElementTree.iterparse(path, events=("start",))) |
There was a problem hiding this comment.
Using next(ElementTree.iterparse(...)) without fully consuming or closing the generator can leave the underlying file descriptor open. On large documentation sites with many SVG files, this can lead to a resource leak and potentially trigger an OSError: [Errno 24] Too many open files error.\n\nSince SVG files are typically small, reading the file contents into memory and parsing them with ElementTree.fromstring is safer, faster, and automatically manages the file lifecycle.
| _, root = next(ElementTree.iterparse(path, events=("start",))) | |
| root = ElementTree.fromstring(path.read_bytes()) |
Pin the disclosure chevron to the glyph size so collapsible rows no longer render ~2px taller than standalone links (and no longer shift as ion-icon hydrates). The theme override now matches Material's selector specificity and clears the flex min-height that kept the icon from shrinking. Also drop two dead nav rules: an immediately-overridden margin-top and a section selector that is never emitted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The main grid was redefined at three breakpoints, each in three variants (normal, error, no-TOC), and the variants disagreed on justify-content: doc pages centered the [rail | article | outline] block while no-TOC landing pages left-aligned it. That flip moved the primary rail ~60px sideways when navigating between page types, and forced a second special-cased divider formula. Define the shell once: a centered three-column grid whose track widths come from four CSS custom properties. The breakpoints now only retune those tokens. The rail and the article's left edge are identical on every page; a page without a TOC simply leaves the outline column empty instead of re-aligning the grid. The error page remains the one deliberate exception (no rail, single centered column). Net effect: no horizontal shift between pages with and without a table of contents, and the per-breakpoint layout rules collapse to one line each. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The header bar, the main grid, and the footer were each sized and padded independently (three different max-widths and three different left gutters), and the main grid centered a fixed-width block rather than filling its container. So the primary rail and the outline drifted in and out of the header's and footer's edges as the viewport changed — the logo, sidebar, and footer-left never shared a line, and neither did the theme toggle, outline, and footer-right. Introduce one shared page frame — same max-width, same auto side margins, same left/right gutter — used by the header inner, the main inner, and the footer inner at every breakpoint. The main grid now fills that frame (rail | 1fr article | outline), so the rail pins to the left gutter and the outline to the right gutter; the article measure follows the frame (~48rem at the 90rem cap). Below the desktop breakpoint the sidebars fold away and the article fills the same frame, keeping it aligned with the menu and footer. Only the gutter (2rem desktop, 1rem below) and the desktop grid tracks change per breakpoint. Also: drop the logo's leftover Material margin so its box sits on the gutter, remove the fixed 38rem content cap (the frame sets the measure now), and keep the error page as the one centered, rail-less exception. Verified logo/rail/footer-left and toggle/outline/footer-right align at 480, 1100, 1250, 1400, and 1560px, on TOC, no-TOC, and error pages. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Vendor Ionicons (runtime + used SVGs) locally; drop the unpkg CDN so the docs render fully offline (Self-hosted/air-gapped) and carry no third-party script dependency. Repoint the header scripts and CSS icon masks. - Replace the regex-based admonition-icon injection with Material's native, offline icons recolored to the Codacy palette; simplify image_metadata.py to its image-dimension role only. - Guard the persistent header/search listeners so navigation.instant no longer stacks duplicate handlers (matches the existing drawer guard). - Wire the footer to config.copyright as the single source of truth. - Preload the primary Inter subset; stop forcing mid-token code wrapping. - Split theme.css into concern-scoped, comment-free stylesheets loaded in cascade order; flush the desktop nav rail; give the header a translucent frosted-glass background. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Closing this draft in favor of a fresh squashed pull request. |
|
Superseded by #2702 |
Hi yo, im redoing the theme of the docs.
The docs right now are in need of a lot of love. They look outdated and there's very little attention to how they are structured and organized. Because of this, i would like to start this as a series of changes:
I am not putting the theme within a submodule intentionally. Putting it as a submodule make it hard to make changes on the docs, and since the documentation of the self-hosted has not been updated in the last 2 years, i would rather focus on a quick, and safe solution that allows us to iterate quickly.
This theme was inspired by Mintlify and completely done with Codex.
Yes, its not perfect. Yes, smells like AI spirit. But, it's one step in the right direction.
Doing this will maintain all the functionality the docs have ATM. And reduces migration efforts to other tools / frameworks. TY 👋
TESTS:
CANT TEST: