Skip to content

Remove AstroWind from Material docs while preserving Starlight and homepage design - #13490

Open
NigelBreslaw wants to merge 13 commits into
masterfrom
nigel/material-remove-astrowind
Open

NigelBreslaw wants to merge 13 commits into
masterfrom
nigel/material-remove-astrowind

Conversation

@NigelBreslaw

@NigelBreslaw NigelBreslaw commented Sep 18, 2026

Copy link
Copy Markdown
Member

The material site was based on AstroWind and causes non stop security alerts from outdated dependencies. This strips everything out so it is just a boring normal Astro site and can be updated with all the docs sites instead.

  • Replace the AstroWind homepage with a plain Astro page, local CSS and native mobile navigation; preserve its layout, typography, images, icons and theme preference.
  • Delete the vendored AstroWind integration, unused pages and helpers, Tailwind configuration and compression integration. Remove 20 direct dependencies without adding any.

Use the Starlight splash page component for base-aware hero actions. Preserve the landing content and downloads while removing the AstroWind widgets, browser scripts, and Tailwind dependencies.
Keep Astro 7.3 and Starlight with shared Slint documentation components. Remove redundant Sharp and MDX type declarations, obsolete Knip exclusions, and Tailwind parser configuration. Make remaining root-relative content links respect the deployment base.

Validate root and /material/ builds, documentation content parity, lint, formatting, spelling, Knip, and 24 Chromium/WebKit browser checks. Firefox qualification is limited by a local profile-folder launch failure.

ChangeLog: Simplify the Material documentation site by removing AstroWind while retaining Starlight.
Restore the announcement bar, navigation, gradient heading, large tablet image, feature rows, download panel, component cards, and footer using a plain Astro page and local CSS. Keep the Inter font and existing SVG icons as static assets with the font license.

Keep theme preferences compatible with Starlight and make the mobile menu work without JavaScript. Verify both deployment paths, 28 Chromium/WebKit checks, documentation content parity, lint, formatting, spelling, and Knip.
Extend the gradient paint area around glyphs and keep the span inline so the heading line box retains its height. Verify the local rendering and desktop/mobile homepage checks in Chromium and WebKit.
Initialize the homepage theme without unused Starlight picker support. Remove inherited TypeScript defaults, redundant CSS and obsolete archive exclusions.
Keep the APK and web gallery actions on the current host and deployment base path. Update the homepage checks to verify both links.
Replace the obsolete AstroWind vendor exclusion with the exact third-party font license path. Preserve the upstream license text.
The Material homepage's vendored WOFF2 font made CI autofix fail because the checker only recognized WOFF. Treat both binary formats alike; font licensing remains covered by its OFL sidecar.

Validation: CI=1 mise run --force ci:autofix:fix passes, including the license header check, with no unrelated changes.
// SPDX-License-Identifier: MIT
import { test, expect } from "@playwright/test";

test.describe("homepage", () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like to me that these tests are run in the CI. Am I missing something?

lazyImagesRehypePlugin,
rehypeExternalLinksSlint,
],
processor: unified({ rehypePlugins: [rehypeExternalLinksSlint] }),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this could be replaced with slintStarlightMarkdownRehypeExternalLinksOnly() for consistency with the other Astro sites in the repo?

{ text: "Demo", href: base + "wasm/index.html" },
{ text: "Slint Home", href: "https://slint.dev" },
];
const social = [

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we have a central config for this so that it doesn't have to be duplicated? slintStarlightSocial or so?

@tronical

Copy link
Copy Markdown
Member

@claude Review please (There is no aspect of public API in this PR; but code-reuse and CI coverage is particularly valuable; check licenses as well)

@claude

claude Bot commented Sep 19, 2026

Copy link
Copy Markdown

Claude encountered an error after 3m 3s —— View job


Review

  • Gather context (diff, changed files, repo docs)
  • Check public API impact
  • Check code reuse (shared Starlight helpers vs. duplicated code)
  • Check CI coverage for new/changed tests
  • Check licenses (Inter font, vendored files)
  • Post review

Public API: no impact. This PR only touches the Material docs website (ui-libraries/material/docs/) — build config, pages, styles, and vendored files. No .slint files, no api/ sources, and no callbacks/properties changed. Nothing here breaks compatibility for Slint users.

Confirmed: the new Playwright tests never run in CI

.github/workflows/material.yaml has two jobs that touch ui-libraries/material/docs:

  • material_tests only runs cargo test -p material-gallery (Rust, not the docs site).
  • deploy runs pnpm install and pnpm build, but never pnpm test / playwright test.

Compare with docs/astro, which has the equivalent pnpm exec playwright install --with-deps + pnpm exec playwright test steps in .github/workflows/build_docs.yaml. Material's playwright.config.ts even reuses docs/common/src/testing/playwright-starlight-base.ts, the same helper docs/astro uses, so the two projects are set up the same way — but only docs/astro actually runs the suite.

The 180+ new lines in tests/smoke-test.spec.ts (homepage rendering, no-JS fallback, mobile nav, etc.) are good tests, but right now they only run if someone invokes pnpm test locally. This should be wired into material.yaml (a step alongside/after pnpm build in the deploy job, or a new job) before this PR is merged, otherwise a future homepage regression won't be caught.

Code reuse: two places duplicate a helper instead of using it

  1. astro.config.ts:14,23-24 — builds the markdown pipeline by hand:

    import { unified } from "@astrojs/markdown-remark";
    ...
    markdown: {
        processor: unified({ rehypePlugins: [rehypeExternalLinksSlint] }),
    },

    docs/cpp, docs/nodejs, and docs/python all just do markdown: slintStarlightMarkdownRehypeExternalLinksOnly() (defined in docs/common/src/utils/starlight-site-defaults.ts), which sets up the same rehypeExternalLinksSlint plugin and also turns on gfm: true (needed for Markdown tables in .mdx, see the comment on that helper). The custom unified(...) version skips gfm: true, so it's not just more code, it's missing a behavior the other sites have. Switching to slintStarlightMarkdownRehypeExternalLinksOnly() fixes both.

  2. src/pages/index.astro:33-60 — hand-rolls a social array (GitHub, X, Bluesky, Mastodon, LinkedIn, YouTube) that's almost an exact copy of slintStarlightSocial, already exported from docs/common/src/utils/starlight-social.ts for exactly this purpose. The copy has already drifted from the source: "Github" vs. the canonical "GitHub", and a different GitHub URL (pointing at ui-libraries/material instead of the repo root — that difference may be intentional for this page). Reusing slintStarlightSocial here (overriding just the GitHub entry if the subfolder link is wanted) avoids a second copy that can silently drift, e.g. if a social account changes.

Licenses: look correct

  • The vendored Inter font is REUSE-compliant: src/assets/fonts/inter-latin-wght-normal.woff2.license carries SPDX-FileCopyrightText: The Inter Project Authors / SPDX-License-Identifier: OFL-1.1, and LICENSES/OFL-1.1.txt already exists at the repo root, so the SPDX identifier resolves.
  • xtask/src/license_headers_check.rs (0033768) correctly extends the existing \.woff$NoLicense rule to \.woff2?$, matching how other vendored fonts in the repo (e.g. tools/lsp/ui/assets/Inter-VariableFont.ttf.license) are handled.
  • cspell.json (d667865) swaps the stale vendor/** exclusion for the specific public/fonts/Inter-LICENSE.txt path, so spell-check doesn't choke on the license prose.
  • Minor, non-blocking observation: public/fonts/Inter-LICENSE.txt is a full copy of the OFL text served publicly, separate from LICENSES/OFL-1.1.txt. That's a reasonable way to ship the license alongside the redistributed font, but it's a second copy of the same text that won't update itself if LICENSES/OFL-1.1.txt ever changes. Not something to fix now, just worth knowing.
  • Old AstroWind vendor code and its dependencies are fully removed, so no more outdated-dependency alerts from that vendored tree, consistent with the PR's stated goal.

Summary

No public API risk. The main thing I'd want addressed before merge is wiring the new Playwright suite into material.yaml so it actually runs in CI — the tests exist but currently can't catch a regression. The two duplicated-helper spots (astro.config.ts markdown pipeline and the social array in index.astro) are worth cleaning up for consistency with the other docs sites, and the gfm: true gap is a real (if small) behavior difference. Licensing work looks solid.

@tronical

Copy link
Copy Markdown
Member

@claude Review again please (There is no aspect of public API in this PR; but code-reuse and CI coverage is particularly valuable; check licenses as well

@claude

claude Bot commented Sep 19, 2026

Copy link
Copy Markdown

Claude finished @tronical's task in 1m 14s —— View job


Review

  • Gather context (diff since last review, new commits)
  • Check public API impact
  • Check if CI coverage feedback was addressed
  • Check if code-reuse feedback was addressed
  • Check licenses again
  • Post updated review

Public API: still no impact. The five commits added since the last review (c015180..0033768) only touch ui-libraries/material/docs/ (homepage CSS, theme init, gallery links) plus cspell.json and xtask/src/license_headers_check.rs. No .slint files, no api/ sources.

CI coverage: not yet addressed

The new Playwright tests in tests/smoke-test.spec.ts still aren't run in .github/workflows/material.yaml. The deploy job runs pnpm install and pnpm build but never pnpm test, and there's no separate test job either. Compare with docs/astro, whose build_docs.yaml runs pnpm exec playwright install --with-deps then pnpm exec playwright test. Right now a homepage regression (e.g. the mobile-nav or no-JS fallback the new tests check) would only be caught if someone runs the tests locally.

Fix this →

Code reuse: also not yet addressed

  1. ui-libraries/material/docs/astro.config.ts:14,23-24 still builds its own markdown pipeline with unified({ rehypePlugins: [rehypeExternalLinksSlint] }) instead of calling slintStarlightMarkdownRehypeExternalLinksOnly() from docs/common/src/utils/starlight-site-defaults.ts, which the other Astro sites (docs/cpp, docs/nodejs, docs/python) use. That shared helper also turns on gfm: true, which the hand-written version skips — so Markdown tables may not render the same way here as on the other doc sites.

  2. ui-libraries/material/docs/src/pages/index.astro:33-60 still has its own social array instead of slintStarlightSocial (docs/common/src/utils/starlight-social.ts:13-31). The copy has drifted further from the shared one:

    • "Github" here vs. the canonical "GitHub".
    • The GitHub link points at .../tree/master/ui-libraries/material instead of the repo root.
    • "Linkedin" (canonical) vs. "LinkedIn" here — a difference only visible as alt/label text.

    If the Material-specific GitHub URL is intentional, that can still be done by spreading slintStarlightSocial and overriding just that one entry, rather than hand-copying all six links.

Fix this →

Licenses: still look correct

  • The vendored Inter font stays REUSE-compliant: src/assets/fonts/inter-latin-wght-normal.woff2.license and public/fonts/Inter-LICENSE.txt.license both carry the same SPDX-FileCopyrightText: 2016 The Inter Project Authors / SPDX-License-Identifier: OFL-1.1, matching LICENSES/OFL-1.1.txt.
  • xtask/src/license_headers_check.rs (0033768) now treats .woff and .woff2 the same way for the no-license-header rule, which is needed since the font is now vendored as .woff2.
  • cspell.json (d667865) points spell-check at the specific public/fonts/Inter-LICENSE.txt path instead of a broad vendor/** exclusion.
  • No new dependencies were added; the old AstroWind vendor tree and its outdated dependencies are gone, which is the PR's stated goal.

Summary

No public API risk. The two things flagged in the first review — wiring the Playwright suite into material.yaml, and replacing the duplicated markdown/social config with the shared docs/common helpers — are still open in the latest commits. I'd want those addressed before merge, since right now a homepage regression won't be caught by CI, and the social/markdown copies have already drifted from the shared source ("Github" vs "GitHub", missing gfm: true). Licensing continues to look solid.

Run the Material Playwright suite in CI and retain its report. Reuse the shared markdown and social configuration while preserving Astro 7.3's processor API, the Material repository link, and the homepage icon order.
Knip does not infer the workflow job's default working directory. Set it on both Playwright steps so CI attributes the binary to the Material docs package.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants