Add config-based toggling for layout and sections #16
Conversation
Introduces cvfolio.config.json for toggling layout elements and page sections without modifying templates. Updates BaseLayout and page components to conditionally render header, footer, theme switcher, and homepage/writing sections based on config. Adds site-config utility for config loading and validation. Updates documentation to explain new configuration options.
|
Hey @ksanjeev284, thanks for you contribution. I don't have chance to review today but I'll check it in Monday. |
There was a problem hiding this comment.
Pull Request Overview
Adds a configuration-driven mechanism (cvfolio.config.json) to toggle layout elements and page sections without editing templates. Key updates include a schema-validated config loader, conditional rendering in BaseLayout, and conditional sections on the homepage and writing pages.
- Introduce site-config loader with Zod validation and sensible defaults
- Conditionally render header, footer, theme switcher, and page sections based on config
- Update docs and include a sample cvfolio.config.json
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/site-config.ts | Adds schema, validation, defaults, and loader for cvfolio.config.json |
| src/layouts/BaseLayout.astro | Conditionally render header, footer, and theme switcher based on config |
| src/pages/index.astro | Conditionally render homepage sections from config |
| src/pages/writing/index.astro | Conditionally render writing page sections from config |
| docs/customization.md | Documents new configuration toggles and defaults |
| cvfolio.config.json | Adds default configuration example |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/lib/site-config.ts
Outdated
| const effective = parsed ?? {}; | ||
|
|
||
| const layout = mergeFlags(defaults.layout, effective.layout); | ||
| const sections = { | ||
| homepage: mergeFlags(defaults.sections.homepage, effective.sections?.homepage), | ||
| writing: mergeFlags(defaults.sections.writing, effective.sections?.writing), |
There was a problem hiding this comment.
TypeScript compile-time issue: effective is inferred as RawSiteConfig | {}, so property access like effective.layout is not safe and will error. Use parsed directly with optional chaining or assert the type. For example: const layout = mergeFlags(defaults.layout, parsed?.layout); const sections = { homepage: mergeFlags(defaults.sections.homepage, parsed?.sections?.homepage), writing: mergeFlags(defaults.sections.writing, parsed?.sections?.writing) };
| const effective = parsed ?? {}; | |
| const layout = mergeFlags(defaults.layout, effective.layout); | |
| const sections = { | |
| homepage: mergeFlags(defaults.sections.homepage, effective.sections?.homepage), | |
| writing: mergeFlags(defaults.sections.writing, effective.sections?.writing), | |
| // Use parsed directly with optional chaining to ensure type safety | |
| const layout = mergeFlags(defaults.layout, parsed?.layout); | |
| const sections = { | |
| homepage: mergeFlags(defaults.sections.homepage, parsed?.sections?.homepage), | |
| writing: mergeFlags(defaults.sections.writing, parsed?.sections?.writing), |
Copilot uses AI. Check for mistakes.
| const jobs = await getCollection('jobs'); | ||
| const sortedJobs = sortJobsByDate(jobs); | ||
| const talks = await getCollection('talks'); | ||
| const { sections } = await loadSiteConfig(); |
There was a problem hiding this comment.
This calls loadSiteConfig in the page while BaseLayout.astro also calls it (line 16 there), causing duplicate JSON import/parse per render. Consider memoizing loadSiteConfig (cache a module-level Promise) or loading once in the page and passing layout down to BaseLayout as a prop to avoid redundant work.
Copilot uses AI. Check for mistakes.
src/layouts/BaseLayout.astro
Outdated
| <body class="bg-background text-foreground pb-12"> | ||
| <Header /> | ||
| {layout.header && <Header />} | ||
| <main class="pt-24"> |
There was a problem hiding this comment.
[nitpick] pt-24 appears to account for the header; when layout.header is false, this leaves unnecessary top padding. Consider making the padding conditional, e.g., <main class={layout.header ? 'pt-24' : ''}>.
| <main class="pt-24"> | |
| <main class={layout.header ? "pt-24" : ""}> |
Copilot uses AI. Check for mistakes.
|
@ksanjeev284 What do you think about to check the recommendations from Copilot? |
sure |
2 participants
Introduces cvfolio.config.json for toggling layout elements and page sections without modifying templates. Updates BaseLayout and page components to conditionally render header, footer, theme switcher, and homepage/writing sections based on config. Adds site-config utility for config loading and validation. Updates documentation to explain new configuration options.
Issue #15