-
Notifications
You must be signed in to change notification settings - Fork 37
リファクタリング #187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
リファクタリング #187
Conversation
4b3eb40
to
89bd1fd
Compare
89bd1fd
to
38dff38
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the UI by extracting common layout patterns into Card
and Heading
components, replaces inline SVGs with @lucide/astro
icons, and adds global meta tags to improve responsiveness and meta generation.
- Introduced
Card
andHeading
components and wired them throughout sections - Replaced raw SVG markup with Lucide icon components for consistency
- Added viewport and generator
<meta>
tags inLayout.astro
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/layouts/Layout.astro | Added <meta name="viewport"> and generator tag |
src/components/Venue.astro | Wrapped in Card & Heading ; replaced SVGs |
src/components/Staff/index.astro | Swapped static heading for Heading component |
src/components/Staff/StaffCard.astro | Wrapped card markup in new Card component |
src/components/Sponsors/index.astro | Introduced Card /Heading ; replaced SVGs |
src/components/Schedule.astro | Used Heading & Card ; replaced calendar SVG |
src/components/OfficialBlog.astro | Added Heading ; swapped SVGs for Lucide icons |
src/components/KeynoteSpeakers.astro | Used Heading & Card ; replaced users SVG |
src/components/Heading.astro | New component for section headings |
src/components/Header.astro | Replaced menu SVGs with Lucide Menu and X icons |
src/components/Footer.astro | Refactored Vim logo to component; replaced mail/Twitter icons |
src/components/Card.astro | New component for card styling |
src/components/AboutVimConf.astro | Swapped static heading for Heading component |
package.json | Added @lucide/astro dependency |
Files not reviewed (1)
- 2025/pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (4)
src/components/Heading.astro:4
- [nitpick] The
level
prop is defined but onlylevel === 2
is implemented. Either implement support for other levels or remove the unused prop to avoid confusion.
level?: 1 | 2 | 3 | 4 | 5 | 6;
src/components/Venue.astro:19
- [nitpick] Decorative icons should include
aria-hidden="true"
or a meaningfularia-label
if conveying information. Addaria-hidden="true"
here since the surrounding text provides the context.
<MapPin class="mt-0.5 h-4 w-4 flex-shrink-0 text-emerald-600" />
src/components/Card.astro:9
- [nitpick] The
Card
component only acceptsclass
but does not forward other attributes (e.g.,id
,aria-*
). Consider spreading...Astro.props
on the wrapper<div>
to make it more flexible.
<div
src/components/Footer.astro:3
- The Lucide export for the X icon is named
X
, notXIcon
. Update the import to{ Mail, X }
or alias correctly.
import { Mail, XIcon } from "@lucide/astro";
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Uh oh!
There was an error while loading. Please reload this page.