Conversation
Deploying funasitien-github-io with
|
| Latest commit: |
7219553
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://edbb4132.funasitien-github-io.pages.dev |
There was a problem hiding this comment.
Pull request overview
This PR appears to move the site into a new “beta” layout/theme by replacing the old main/home/docs layouts and legacy components with a new default layout, new navigation/dock components, and new/rewritten pages (blog/archives/legal pages).
Changes:
- Introduces a new
src/layouts/default.astro+src/layouts/head.astrolayout system and updates pages to use it. - Adds new pages/components for Blog/Archives and markdown-based legal pages (ToS, Privacy) plus a new markdown layout.
- Updates styling/theme configuration (
src/assets/app.css) and bumps dependencies inpackage.json.
Reviewed changes
Copilot reviewed 40 out of 139 changed files in this pull request and generated 21 comments.
Show a summary per file
| File | Description |
|---|---|
| src/layouts/default.astro | New main layout; adds Discord status fetch + theme toggle + side navigation/dock |
| src/layouts/head.astro | New shared <head> partial for meta tags, theme init, transitions |
| src/assets/app.css | Reworks DaisyUI theme definitions and typography styles |
| src/pages/index.astro | Rewrites home page content for new design |
| src/pages/projets.astro | Rebuilds projects page using new Card/CardSection components |
| src/pages/contact.astro | Rewrites contact page content/layout |
| src/pages/blog.astro | Adds new blog page scaffold |
| src/pages/archives.astro | Adds new archives page scaffold |
| src/pages/tos.mdx | Adds Terms of Service page (MDX) |
| src/pages/privacy.mdx | Adds Privacy Policy page (MDX) |
| src/layouts/markdown.astro | Adds markdown wrapper layout around default.astro |
| src/components/Navigation.astro | Adds sidebar navigation list |
| src/components/PageList.astro | Adds navigation row component |
| src/components/Dock.astro | Adds mobile dock navigation |
| src/components/CardSection.astro | Adds section wrapper for card grids |
| src/components/Card.astro | Adds generic card component |
| src/components/BlogCard.astro | Adds blog card component |
| package.json | Updates versions and adds/removes dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <h3 class="text-xl font-bold pb-4">Me contacter</h3> | ||
| <a class="btn">hello@funa.dev</a> | ||
| </div> | ||
| <div class="flex flex-col gap-4"> | ||
| <h3 class="text-xl font-bold pb-4">Mes Résaux</h3> | ||
| <a class="btn">Discord</a> | ||
| <a class="btn">Instagram</a> | ||
| <a class="btn">Bluesky</a> | ||
| <a class="btn">Linkdin</a> | ||
| </div> |
There was a problem hiding this comment.
These are styled as buttons but are <a> elements without href, and there are typos in the headings/labels (e.g. "Résaux", "Linkdin"). Use proper href values (mailto:, external URLs) or switch to <button>, and correct the visible text.
| <div class="p-8 flex flex-col"> | ||
| <Card title="Nouveau Portfolio!" description="" image="" slug="" /> | ||
| </div> |
There was a problem hiding this comment.
BlogCard.astro does not accept/use slug, so passing slug="" here has no effect and there is currently no link to the post. Either update BlogCard to render a link using slug (or a full URL), or remove the unused prop and add navigation another way.
| const { title, icon = "simple-icons:html5", description } = Astro.props; | ||
| // If title is a string starting with http or https, treat it as an image URL | ||
| const iconIsImage = typeof icon === "string" && /^https?:\/\//i.test(icon); | ||
| --- | ||
| <div class="card bg-base-200 border-2 border-neutral-content rounded-xl"> | ||
| <div class="card-body"> | ||
| <h2 class="card-title inline-flex items-center gap-1"> | ||
| {iconIsImage ? ( | ||
| <img src={icon} alt="title image" class="h-6 w-auto mr-2 rounded" /> | ||
| ) : ( | ||
| <Icon name={icon} class="text-2xl mr-2" /> | ||
| )} | ||
| { title } | ||
| </h2> | ||
| <p>{description}</p> |
There was a problem hiding this comment.
Card is used with a date prop in several pages, but this component doesn't accept/render date, so that information is silently dropped. Either add date to the component and display it, or stop passing it from callers.
| // If title is a string starting with http or https, treat it as an image URL | ||
| const iconIsImage = typeof icon === "string" && /^https?:\/\//i.test(icon); | ||
| --- | ||
| <div class="card bg-base-200 border-2 border-neutral-content rounded-xl"> | ||
| <div class="card-body"> | ||
| <h2 class="card-title inline-flex items-center gap-1"> | ||
| {iconIsImage ? ( | ||
| <img src={icon} alt="title image" class="h-6 w-auto mr-2 rounded" /> | ||
| ) : ( |
There was a problem hiding this comment.
The comment says "If title is a string…" but the logic actually checks icon (not title). Also the <img> alt text is hard-coded to "title image"; for accessibility it should describe the specific icon (e.g., derived from title).
| <a | ||
| class="flex items-center rounded-xl border-2 transition-all duration-300 ease-in-out bg-base-100 border-neutral-content" | ||
| > | ||
| <div class="relative ml-4 mr-1"> |
There was a problem hiding this comment.
This <a> element has no href, which makes it non-interactive and hurts accessibility/semantics. If it's just a profile card, use a <div>; if it should link somewhere, add a real destination (href) and appropriate aria-label/focus styles.
| <Card title="Smart" date="2023-Aujourd'hui" description="Un Cloudsystem abstrait développé en Java"> | ||
| <a class="btn">Github</a> | ||
| <a class="btn btn-info">Documentation</a> | ||
| </Card> | ||
| <Card title="Vaatigames" icon="https://vaatigames.ovh/img/vaati2.png" date="2024-Aujourd'hui" description="Une association d'hébergement cloud gratuit" > | ||
| <a class="btn bg-purple-300 dark:bg-purple-700">Présentation</a> | ||
| </Card> | ||
| <Card title="DEMOCRAFT" date="2023-Aujourd'hui" description="Le premier serveur mini-jeux crossplay en europe."> | ||
| <a class="btn btn-error">Site officiel</a> | ||
| </Card> | ||
| <Card title="MOODS" date="2023-Aujourd'hui" description="Streaming de mudique open source"> | ||
| <a class="btn">Github</a> | ||
| </Card> |
There was a problem hiding this comment.
Several buttons are rendered as <a class="btn">…</a> without href. This is invalid for links and harms keyboard/screen-reader behavior. Use <button> for actions, or add real href targets (and target/rel when linking off-site).
| If you are redirect to this pae, this means that you are using a service provided by Funasitien. By using our website and services, you agree to comply with and be bound by the following terms and conditions: | ||
|
|
There was a problem hiding this comment.
Typo in user-facing legal text: "pae" should be "page" (and the sentence currently reads ungrammatically as "If you are redirect …"). Please correct this wording so the Terms page looks professional.
| // Import fetch with custom SSL handling | ||
| const fetch = (await import('node-fetch')).default; | ||
| const https = await import('https'); | ||
|
|
There was a problem hiding this comment.
This layout imports node-fetch, but node-fetch is listed under devDependencies in package.json. If this code runs in production (SSR/build), installs that omit devDependencies will fail at runtime; move node-fetch to dependencies or use the Node runtime fetch instead.
| const response = await fetch("https://api.lanyard.rest/v1/users/574169911782277135", { | ||
| agent: httpsAgent | ||
| }); | ||
|
|
||
| //if (!response.ok) throw new Error(`HTTP error! status: ${response.status}`); | ||
|
|
||
| let data = await response.json(); | ||
| data = data["data"]; | ||
| status = data["discord_status"]; | ||
| const activities = data["activities"]; | ||
|
|
There was a problem hiding this comment.
The layout performs an external API call during render to populate Discord status. This adds a network dependency to every page render/build and can cause slow renders or failures if the API is down; consider caching the result (with a timeout/fallback), moving it behind an API route with caching, or fetching client-side so the layout render stays deterministic.
| <div class="p-8"> | ||
| <div class="bg-[url('/banners/white.png')] h-64 bg-cover bg-center rounded-lg border-2 border-neutral-content p-4" /> | ||
| </div> | ||
| <div class="mt-8 text-center"> |
There was a problem hiding this comment.
Same markup issue as in projets.astro: the banner <div /> is self-closing but there is an extra </div> right after it, causing mismatched tags. Please fix the structure so the page renders valid HTML.
No description provided.