Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
daine
changed the title
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Astro-based docs site foundation under /docs (leaving the existing 11ty docs unaffected), including initial routing/i18n scaffolding, versioned content support, shared layouts/components, and starter content/assets.
Changes:
- Adds Astro app configuration, TypeScript config, and build/dev helper scripts for route validation and asset syncing.
- Implements dynamic locale + slug routing with a manifest-driven side nav and initial i18n utilities.
- Introduces an initial versioned content structure (manifests/config + version switcher) and starter English/French pages/assets.
Reviewed changes
Copilot reviewed 68 out of 89 changed files in this pull request and generated 33 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/tsconfig.json | Astro TS config + path aliases |
| docs/src/utils/lastModified.ts | Git/fs last-modified utilities |
| docs/src/utils/i18n.ts | Route-key lookup helper |
| docs/src/types/routing.ts | Routing/nav type definitions |
| docs/src/types/content.ts | Content module typing |
| docs/src/styles/pages/splash.css | Splash page styling |
| docs/src/styles/global.css | Global styles/util classes |
| docs/src/scripts/menu.js | Placeholder script |
| docs/src/routing/versioned/version-config.json | Versioned sections config |
| docs/src/routing/versioned/tokens-manifest.json | Tokens manifest stub |
| docs/src/routing/versioned/css-shortcut-manifest.json | CSS shortcuts manifest stub |
| docs/src/routing/versioned/component-manifest.json | Components manifest (version-aware) |
| docs/src/routing/routes.ts | Side nav config builder (manifest-driven) |
| docs/src/routing/route-manifest.json | Side nav route manifest |
| docs/src/pages/index.astro | Splash entry page |
| docs/src/pages/_fr/index.astro | Legacy _fr landing page |
| docs/src/pages/_fr/composants/bouton/overview.mdx | French button overview MDX (legacy path) |
| docs/src/pages/_fr/composants/bouton/index.mdx | French button page MDX (legacy path) |
| docs/src/pages/_fr/composants/bouton/examples.mdx | French button examples MDX (legacy path) |
| docs/src/pages/_fr/composants/bouton/assets/gcds-button-anatomy.svg | French button anatomy asset (legacy path) |
| docs/src/pages/_fr/a-propos.astro | Legacy _fr about page |
| docs/src/pages/_en/index.astro | Legacy _en landing page |
| docs/src/pages/_en/components/button/overview.mdx | English button overview MDX (legacy path) |
| docs/src/pages/_en/components/button/index.mdx | English button page MDX (legacy path) |
| docs/src/pages/_en/components/button/examples.mdx | English button examples MDX (legacy path) |
| docs/src/pages/_en/components/button/assets/gcds-button-anatomy.svg | English button anatomy asset (legacy path) |
| docs/src/pages/_en/about-us.astro | Legacy _en about page |
| docs/src/pages/[locale]/[...slug].astro | Dynamic locale/slug router + static paths |
| docs/src/layouts/VersionedLayout.astro | Versioned docs layout |
| docs/src/layouts/SplashLayout.astro | Splash layout |
| docs/src/layouts/BaseLayout.astro | Base docs layout |
| docs/src/i18n/config.ts | Route i18n config (en/fr) |
| docs/src/content/versioned/components/1.1.0/en/button/overview.mdx | Versioned button overview (1.1.0) |
| docs/src/content/versioned/components/1.1.0/en/button/index.mdx | Versioned button page (1.1.0) |
| docs/src/content/versioned/components/1.1.0/en/button/examples.mdx | Versioned button examples (1.1.0) |
| docs/src/content/versioned/components/1.1.0/en/button/design.mdx | Versioned button design stub (1.1.0) |
| docs/src/content/versioned/components/1.1.0/en/button/code.astro | Versioned button code stub (1.1.0) |
| docs/src/content/versioned/components/1.1.0/en/button/assets/gcds-button-anatomy.svg | Versioned button anatomy asset (1.1.0) |
| docs/src/content/versioned/components/1.0.0/en/button/overview.mdx | Versioned button overview (1.0.0) |
| docs/src/content/versioned/components/1.0.0/en/button/index.mdx | Versioned button page (1.0.0) |
| docs/src/content/versioned/components/1.0.0/en/button/examples.mdx | Versioned button examples (1.0.0) |
| docs/src/content/versioned/components/1.0.0/en/button/design.mdx | Versioned button design stub (1.0.0) |
| docs/src/content/versioned/components/1.0.0/en/button/code.astro | Versioned button code stub (1.0.0) |
| docs/src/content/versioned/components/1.0.0/en/button/assets/gcds-button-anatomy.svg | Versioned button anatomy asset (1.0.0) |
| docs/src/content/pages/fr/index.astro | French home content (canonical) |
| docs/src/content/pages/fr/demarrer/index.mdx | French “Start to use” content |
| docs/src/content/pages/fr/a-propos.astro | French about content |
| docs/src/content/pages/en/start-to-use/index.mdx | English “Start to use” content |
| docs/src/content/pages/en/index.astro | English home content (canonical) |
| docs/src/content/pages/en/about-us.astro | English about content |
| docs/src/components/markdown/ul.astro | MDX <ul> mapping component |
| docs/src/components/markdown/ol.astro | MDX <ol> mapping component |
| docs/src/components/index.ts | Components barrel export |
| docs/src/components/_copyNav.astro | Prototype nav component |
| docs/src/components/VersionSwitcher.astro | Version selector + client redirect logic |
| docs/src/components/Text.astro | <gcds-text> wrapper |
| docs/src/components/Tabs.astro | Tabs UI component |
| docs/src/components/SideNav.astro | Side nav rendering from manifest |
| docs/src/components/HelpUs/i18n.js | HelpUs copy (en/fr) |
| docs/src/components/HelpUs/HelpUs.astro | HelpUs section component |
| docs/src/components/Heading.astro | <gcds-heading> wrapper |
| docs/src/components/Header.astro | Header + language toggle logic |
| docs/src/components/Footer.astro | Footer wrapper |
| docs/src/components/DocLinks.astro | Figma/GitHub links component |
| docs/src/components/DateModified.astro | Date modified wrapper |
| docs/src/components/CodePreviewReact.jsx | React code preview component |
| docs/src/components/CodePreview.css | Code preview styles |
| docs/src/components/CodePreview.astro | Astro code preview component |
| docs/src/components/AnatomyList.astro | Anatomy list component |
| docs/src/components/AnatomyImage.astro | Anatomy image component |
| docs/src/assets/img/home/tools-tokens.png | Home asset |
| docs/src/assets/img/home/tools-templates.png | Home asset |
| docs/src/assets/img/home/tools-css-util.png | Home asset |
| docs/src/assets/img/common/start-to-use/banner-develop.svg | Start-to-use banner asset |
| docs/src/assets/img/common/start-to-use/banner-design.svg | Start-to-use banner asset |
| docs/scripts/validate-routes.mjs | Route/content validation script |
| docs/scripts/copy-gcds-assets.mjs | Copies GCDS assets into public |
| docs/package.json | Docs app dependencies + scripts |
| docs/astro.config.mjs | Astro config (MDX/React/i18n/aliases) |
| docs/README.md | Astro starter README |
| docs/.gitignore | Docs folder ignore rules |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| --- | ||
| layout: '../../../../layouts/BaseLayout.astro' | ||
| title: Démarrer | ||
| description: Commencez à utiliser le Système de design GC pour la conception et le développement. | ||
| --- |
There was a problem hiding this comment.
This MDX page uses layout: ...BaseLayout.astro, but the frontmatter sets title instead of the pageTitle prop that BaseLayout reads. This will likely render an undefined page title in the HTML head. Rename title to pageTitle (or update BaseLayout to accept title).
| "include": [ | ||
| ".astro/types.d.ts", | ||
| "**/*" | ||
| ], | ||
| "exclude": [ | ||
| "dist" | ||
| ], |
There was a problem hiding this comment.
tsconfig.json sets include: ["**/*"] but only excludes dist, which means TypeScript will also include node_modules (the default exclude list is overridden when exclude is provided). This can drastically slow down type-checking and may surface thousands of irrelevant errors. Add node_modules (and likely .astro if not needed) to exclude, or replace "**/*" with the standard Astro include patterns.
| <button | ||
| type="button" | ||
| class={`tab ${"selectedTab" === tab ? "active" : ""}`} | ||
| id={`tab-${tab}`} | ||
| > | ||
| {tab} |
There was a problem hiding this comment.
The active tab class is never applied because the template compares a string literal ("selectedTab" === tab). As a result, no tab is marked active until JS runs, and even then the initial SSR markup is incorrect. Consider either removing this SSR class logic and letting the script manage classes, or computing the initial selected tab in frontmatter and using that variable in the class expression.
| const gitDate = execSync(`git log -1 --format=%cI -- "${filePath}"`, { | ||
| encoding: 'utf-8', | ||
| }).trim(); |
There was a problem hiding this comment.
execSync is invoked with a shell-constructed command string that interpolates filePath. Even though the path is quoted, this is still brittle (paths containing quotes) and can be dangerous if pathname/slug ever ends up influencing the resolved path unexpectedly. Prefer execFileSync('git', ['log','-1','--format=%cI','--', filePath], { encoding: 'utf-8' }) to avoid shell parsing and improve portability.
| const pathname = Astro.url.pathname.replace(/\/$/, ''); | ||
| const parts = pathname.split('/').filter(Boolean); | ||
| const lang = parts[0]; | ||
| --- | ||
|
|
||
| <section> | ||
| <Heading tag="h2">{I18N[lang].heading}</Heading> | ||
|
|
||
| {I18N[lang].text.map((paragraph) => ( | ||
| <Text>{paragraph}</Text> | ||
| ))} |
There was a problem hiding this comment.
lang is derived from the URL but used directly as I18N[lang]. If a user hits a non-supported locale (or the route is missing the locale segment), this will throw at render time. Add a fallback (e.g., default to 'en') or validate lang against the supported locales before indexing into I18N.
| <style> | ||
| .section--hero { | ||
| /* background: url(https://design-system.canada.ca/images/common/home/home-hero.jpg) top center no-repeat; */ | ||
| background-size: auto; | ||
| background-size: cover; | ||
|
|
||
| div { | ||
| background: linear-gradient(to right, var(--gcds-bg-primary), transparent); | ||
| } | ||
|
|
||
| article { | ||
| z-index: 20; | ||
| position: relative; | ||
| padding: 10rem 0; | ||
|
|
||
| --gcds-heading-default-text: #fff; | ||
| } | ||
| } |
There was a problem hiding this comment.
This <style> block uses CSS nesting (div { ... }, article { ... } inside .section--hero). Unless nesting is enabled, these rules will be ignored. Rewrite to non-nested selectors or add a nesting transform.
| <gcds-button button-role="secondary" type="link" href="{{ links.startToUseDesign }}">Configurer une bibliothèque Figma</gcds-button> | ||
|
|
||
| ## Développement | ||
|
|
||
| Que vous construisiez des pages HTML statiques ou des applications dynamiques faisant appel à un cadre JavaScript, Système de design GC peut vous aider. | ||
|
|
||
| - Simplifiez votre livraison numérique. | ||
| - Travaillez dans le cadre que vous souhaitez utiliser. | ||
| - Créez des expériences numériques reconnaissables et dignes de confiance du gouvernement du Canada. | ||
| - Construisez des produits qui <gcds-link href="{{links.accessibility}}">répondent aux normes d'accessibilité du GC ou qui les surpassent</gcds-link>. | ||
|
|
||
| ### Fonctionnement | ||
|
|
||
| Système de design GC propose des composants Web réutilisables et personnalisables. Les composants sont autosuffisants, combinant leur propre HTML, CSS et JavaScript pour l'encapsulation et la réutilisation. | ||
|
|
||
| - Les composants sont agnostiques sur le plan technologique et peuvent être utilisés dans n'importe quel projet Web, quel que soit le cadre sous-jacent. | ||
| - Système de design GC utilise des <gcds-link href="{{links.designTokens}}">unités de style</gcds-link> pour capturer les décisions de marque et de design dans le code pour des éléments de style comme les couleurs, la typographie et l'espacement. | ||
| - Les changements y sont apportés dans le cadre de mises à jour uniques qui englobent les unités de style, les composants et les changements non rétrocompatibles. | ||
| - Système de design GC est indépendant de la Boîte à outils de l'expérience Web (BOEW) et de GCWeb. | ||
|
|
||
| <gcds-button button-role="secondary" type="link" href="{{ links.startToUseDevelop }}">Installer Système de design GC</gcds-button> |
There was a problem hiding this comment.
This MDX content uses Eleventy/Nunjucks-style placeholders like {{ links.startToUseDesign }} / {{links.accessibility}}. In Astro MDX these will render literally and produce broken links. Replace with real URLs or import a links object and reference it via MDX expressions.
| "Quelque chose ne va pas? Signalez-le sur GitHub _en créant un compte. Vous pourrez accéder aux réponses de l'équipe, suivre les progrès réalisés concernant votre problèmes et voir les problèmes signalés par d'autres personnes.", | ||
| ], |
There was a problem hiding this comment.
Spelling/grammar in the French string: "votre problèmes" should be singular ("votre problème").
| btnReport: "Signaler un problème sur Github", | ||
| }, |
There was a problem hiding this comment.
Brand capitalization: use "GitHub" (not "Github") in this button label.
melaniebmn
left a comment
melaniebmn
left a comment
There was a problem hiding this comment.
Excited to get this up and running:) Left some comments with ideas/suggestions.
| path.join(targetDir, "gcds-css-shortcuts.min.css"), | ||
| ); | ||
|
|
||
| console.log(`Synced GCDS assets to ${targetDir}`); |
| .map((entry) => entry.name) | ||
| .sort((a, b) => this.compareSemver(a, b)); | ||
| } catch { | ||
| return []; |
There was a problem hiding this comment.
Can we add a console.warn here for easier debugging?
| itemRules: section.itemVersionRules ?? section.itemRules ?? section.itemAvailability ?? {} | ||
| })); | ||
| } catch { | ||
| return []; |
There was a problem hiding this comment.
Same here: can we add a console.warn for easier debugging?
| const parsed = JSON.parse(content); | ||
| return Array.isArray(parsed?.items) ? parsed.items : []; | ||
| } catch { | ||
| return []; |
There was a problem hiding this comment.
Same here: can we add a console.warn for easier debugging?
| } | ||
|
|
||
| isItemExpectedInVersion(section, item, version) { | ||
| const itemKey = this.localized(item?.slug, 'en') || this.localized(item?.slug, 'fr'); |
There was a problem hiding this comment.
Are we assuming that slugs are identical across locales? If they differ (or one is missing), lookups in itemRules could fail or behave inconsistently.
Would it make sense to use a stable, locale-independent key (e.g. item.id or routeKey), or explicitly enforce/document that slugs must match across locales?
| <div class="splash-footer-row mt-600"> | ||
| <div class="splash-footer-links"> | ||
| <a href="/src/pages/_en/terms-of-use" class="splash-footer-link">Terms of use</a> | ||
| <!--<span aria-hidden="true">✦</span>--> |
| height: auto; | ||
| } | ||
|
|
||
| .splash-copy { |
There was a problem hiding this comment.
This can be removed after the div is removed.
| margin: 0 0 var(--gcds-spacing-100); | ||
| font-size: var(--gcds-font-text-small); | ||
| font-weight: var(--gcds-text-weight-bold); | ||
| /*letter-spacing: var(--gcds-border-width-sm);*/ |
There was a problem hiding this comment.
| /*letter-spacing: var(--gcds-border-width-sm);*/ |
| // https://astro.build/config | ||
| export default defineConfig({ | ||
| i18n: { | ||
| locales: ["_en-CA", "_fr-CA"], |
There was a problem hiding this comment.
Is the underscore required for routing logic, or is it a workaround for something internal?
| import rehypeSlug from "rehype-slug"; | ||
| import { fileURLToPath } from "node:url"; | ||
|
|
||
| // Astro uses remark (for .md syntax) and rehype (for HTML output) instead of |
There was a problem hiding this comment.
Do we still need all this commented out code?
There was a problem hiding this comment.
I want to keep it, especially since it's foundational and not shipped.
Because this is mostly setting things up, I commented it for now, but it's one of the options we'll consider for moving away from markdown-it to remark + rehype. Here's what we had in 11ty
Lines 206 to 207 in 99dcae2
📝 Summary | Résumé
This PR adds a first pass at rebuilding the docs site in Astro under a new folder
/docs. The existing documentation website built with 11ty is unaffected.The main goal is to establish the foundation of an Astro-based docs website. This includes the initial Astro app setup, layouts, routing, i18n, shared components, and content structure needed to begin migrating documentation from the current 11ty implementation.
Versioning is also introduced as part of this first pass through a versioned content and routing structure, including version-specific manifests and a version switcher component.
This work is needed to kick-start Astro as the direction for the next iteration of the docs site and to start setting the foundations for future migration work. It creates a contained implementation that can be reviewed, tested, and iterated on before any broader rollout.
Implementation references:
docs/VersionSwitcher,SideNav,Header,Footer, andDateModified🧩 Related Issues | Cartes liées
🧪 Test instructions | Instructions pour tester la modification
/en/components/button/)/en/components/1.1.0/button/is exactly the same as/en/components/button/)Preview links
English
French
Or, go to:
✍️ Author checklist | Liste de vérification de l'auteur
Choose one (primary change type):
Breaking / impact flag:
Ready for review (all items must be checked):
Note: I'm checking all of these off to the best of my ability, we don't need all of them for now since this code isn't live or deployed yet and will still change.
🧐 Reviewer checklist | Liste de vérification du réviseur
Developer checklist (if applicable)
For complex PRs, in lieu of a simple approval or "LGTM" ✅, include the following with your approval:
Design checklist (if applicable)
For designers, include the following with your approval:
This PR is scoped to a new Astro implementation under
/docs, so it does not replace the existing site or introduce breaking changes to current production routes.Main review risks for this first pass are:
Reviewers should focus on whether the Astro foundation, routing model, content structure, and versioning approach are sound enough to build on in follow-up PRs.
Visible code evidence for that wording: the PR title is “migrate docs site to Astro,” it adds a new docs/