kaxil commented on PR #1443:
URL: https://github.com/apache/airflow-site/pull/1443#issuecomment-4042543791
Thanks for working on this! The Hugo and Docsy version bumps are welcome,
but I think the migration from Docsy v0.12.0 → v0.14.3 is incomplete and
introduces a few issues that should be addressed in a follow-up.
### `sidebar-tree.html` — band-aid instead of proper fix
The new Docsy `sidebar.html` now always passes a dict (`{context,
cacheSidebar, sidebarRoot, sidebarRootID}`) instead of the page directly. The
change here:
```go
{{ $context := cond (isset . "context") .context . }}
```
has two problems:
1. **The `cond` backward-compat guard is dead code.** Since this same PR
upgrades Docsy, `sidebar.html` always passes a dict — `isset . "context"` is
always true. The fallback to `.` never triggers. This should just be `{{
$context := .context }}`.
2. **The custom override silently drops new arguments.** `cacheSidebar`,
`sidebarRoot`, and `sidebarRootID` are all ignored. The new `sidebar.html`
injects a `<script>` and adds `d-none` to `#td-sidebar-menu` expecting
`sidebar-tree.html` to cooperate with cache-aware active-state rendering — this
override doesn't.
More broadly, this custom partial is based on the old Docsy v0.12.0
structure and is now severely out of sync with the upstream 186-line template
(missing foldable nav, search, sidebar root support, configurable truncation,
etc.). Worth evaluating whether this override is still needed or if the
upstream template can be used directly.
### `$enable-dark-mode: true !default;` — incorrect fix
```scss
$enable-dark-mode: true !default;
@import "td/code-dark";
```
- **`!default` is meaningless** — nothing defines this variable before this
line, so `!default` has no effect.
- **The comment is misleading** — it says "we don't import bootstrap in
Docsy's main.scss" but Bootstrap IS imported in Docsy's `td/_main.scss` (line
10). The actual issue is that `main-custom.scss` is a separate SCSS entry point
that bypasses Docsy's pipeline.
- **`true` generates unnecessary CSS** — it outputs
`[data-bs-theme='light']` and `[data-bs-theme='dark']` selectors. If the site
doesn't have a functioning dark mode toggle, this is dead CSS.
The root issue is that `td/_code-dark.scss` in Docsy v0.14.x now expects
Bootstrap variables to be in scope (it's wrapped in `@if $enable-dark-mode`).
Importing it directly from `main-custom.scss` breaks that contract. The proper
fix is to either route through Docsy's SCSS pipeline or import Bootstrap's
variables first.
### General
Jumping 3 minor versions (v0.12.0 → v0.14.3) is a significant change. Docsy
v0.13.0 and v0.14.0 both had breaking changes to SCSS structure, sidebar
partials, and the dark mode system. This PR patches two compilation errors but
doesn't fully address the migration. Would be good to audit the [Docsy
changelog](https://www.docsy.dev/blog/) for other breaking changes and open a
follow-up PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
