feat: update landing page to add blog and better understanding - #40
Conversation
Code Review - PR #40: Blog Addition and Styling UpdatesSummaryThis PR adds a blog feature with an initial post and updates the site styling. The implementation uses Astro content collections with MDX support and follows the terminal-inspired design system well. However, there are some critical issues that should be addressed before merging. 🔴 Critical Issues1. Personal Information Exposure (SECURITY/PRIVACY)File: authorEmail: "cpfeifer@madcactus.org"
authorPhone: "+3179955114"
authorDiscord: "https://discordapp.com/users/301068417685913600"Issue: Personal contact information (phone, email, Discord ID) is exposed in a public repository and will be indexed by search engines. This can lead to:
Recommendation:
🟡 Should Fix Before Merge2. Author Image Path InconsistencyFiles:
Issue: The frontmatter uses
Fix: 3. Link Formatting IssueFile: Issue:
4. Avoid
|
There was a problem hiding this comment.
Pull request overview
This PR updates the Astro landing page to add a Blog entry point and refresh the homepage content/styling to better explain Fluid’s workflow and installation options.
Changes:
- Add a new
/blogindex page that lists posts from theblogcontent collection. - Refresh the landing page copy and UI (installation tabs + updated descriptions).
- Add a Blog link in the shared footer and update the introductory blog post content.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| landing-page/src/pages/index.astro | Updates homepage layout/copy and adds install tab UI + client-side interactions. |
| landing-page/src/pages/blog/index.astro | New blog listing route backed by astro:content collection. |
| landing-page/src/layouts/BaseLayout.astro | Adds /blog link to the global footer nav. |
| landing-page/src/content/blog/introducing-fluid.mdx | Expands/updates the “Introducing Fluid” post messaging. |
| landing-page/package.json | Adds prettier-plugin-astro dependency. |
| landing-page/bun.lock | Lockfile update to include the new Prettier plugin and transitive deps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <script> | ||
| // Tab switching | ||
| const tabs = | ||
| document.querySelectorAll<HTMLButtonElement>(".install-tab"); | ||
| const panels = | ||
| document.querySelectorAll<HTMLElement>(".install-panel"); | ||
|
|
||
| tabs.forEach((tab) => { | ||
| tab.addEventListener("click", () => { | ||
| const target = tab.dataset.tab; | ||
|
|
||
| // Update tab styles | ||
| tabs.forEach((t) => { | ||
| t.classList.remove("text-blue-400", "border-blue-400"); | ||
| t.classList.add("text-neutral-500", "border-transparent"); | ||
| }); | ||
| tab.classList.remove("text-neutral-500", "border-transparent"); | ||
| tab.classList.add("text-blue-400", "border-blue-400"); | ||
|
|
||
| // Show matching panel | ||
| panels.forEach((p) => p.classList.add("hidden")); | ||
| const activePanel = document.getElementById(`panel-${target}`); | ||
| if (activePanel) activePanel.classList.remove("hidden"); | ||
| }); | ||
| }); | ||
|
|
||
| // Copy buttons | ||
| document | ||
| .querySelectorAll<HTMLButtonElement>(".copy-btn") | ||
| .forEach((btn) => { |
There was a problem hiding this comment.
AGENTS.md (landing-page/AGENTS.md:165-167) calls for “vanilla JS” in component <script> tags; this script uses TypeScript-only generic annotations (e.g., querySelectorAll(...)), which deviates from that guideline and may rely on TS transpilation. Consider removing the generic annotations (or switching to JSDoc typing), or explicitly marking/bundling the script as TypeScript if that’s the intent.
| <script> | ||
| // Workflow animation | ||
| function initWorkflowAnimation() { | ||
| const steps = | ||
| document.querySelectorAll<HTMLElement>(".workflow-step"); | ||
| const progress = document.getElementById("workflow-progress"); | ||
| const workflowStep = document.querySelector(".workflow-step"); |
There was a problem hiding this comment.
AGENTS.md (landing-page/AGENTS.md:165-167) suggests using vanilla JS in <script> tags; this inline script also uses TypeScript generic annotations in DOM queries (querySelectorAll(...)). Consider removing these TS-only annotations or moving the logic into a typed module (JSDoc/TS) so the client script stays consistent with the documented approach.
|
|
||
| <style> | ||
| a { | ||
| text-decoration: none !important; | ||
| } | ||
| </style> |
There was a problem hiding this comment.
This page-level style disables underlines for every on the blog index (including header/footer links) and uses !important, which makes future styling changes harder. Consider scoping the rule to just the post cards (e.g., a class selector) or relying on the existing Tailwind no-underline/hover:no-underline utilities.
| <style> | |
| a { | |
| text-decoration: none !important; | |
| } | |
| </style> |
Code Review - PR #40Thanks for the substantial improvements to the landing page! The blog feature and enhanced content significantly improve clarity. However, there are several issues that need to be addressed before merging. 🔴 Critical Issues1. PII Exposure in Blog MetadataFile: The changelog post exposes sensitive personal information in the frontmatter:
Action Required: Remove or replace with business/project contact info before merging. Even if these are not rendered publicly, they will remain in git history. 🟡 Medium Priority Issues2. Image Path Handling - Potential Runtime ErrorFile: Issues:
3. Missing Image AssetsFile: References images that are not included in this PR:
Action Required: Add these images to the PR or remove the references. 🟢 Minor Issues4. Missing Test CoveragePer Missing tests for:
5. Type Safety - Optional ChainingShould use optional chaining since ✅ What is Good
📋 Action ItemsBefore merging:
SummaryThis PR makes significant improvements to landing page clarity and adds a well-implemented blog feature. The main blocker is the PII exposure - please address immediately. Overall verdict: |
Description
Type of Change
Checklist
Release Notes
Labels