-
Notifications
You must be signed in to change notification settings - Fork 0
client のリファクタリング
#192
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
client のリファクタリング
#192
Conversation
…a list format for better code reuse refactor(Arranger, Category, Exhibition, Material, Season): replace individual image rendering functions with WorksImages component to reduce code duplication and improve maintainability close #185
feat(Index.tsx): add loading state for exhibitions to improve user experience
…ork.tsx, MaterialWork.tsx, SeasonWork.tsx): replace fragment shorthand with section tags for better semantic structure and accessibility
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
Refactors page components by extracting common image listing into a shared WorksImages component, normalizing naming conventions, removing redundant fragments, and adding a loading state to the Index.tsx page.
- Extracted inline image list renderers into
WorksImagesand updated all pages to use it - Removed unnecessary fragment wrappers (
<>...</>) around<main>elements - Renamed variables to camelCase, updated imports, and added a loading fallback in
Index.tsx
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/pages/SeasonWork.tsx | Removed redundant fragment wrapper |
| client/src/pages/Season.tsx | Replaced inline SeasonImages with shared WorksImages |
| client/src/pages/MaterialWork.tsx | Removed redundant fragment wrapper |
| client/src/pages/Material.tsx | Replaced inline MaterialImages with shared WorksImages |
| client/src/pages/Index.tsx | Renamed date variables, added loading state before render |
| client/src/pages/ExhibitionWork.tsx | Removed redundant fragment wrapper |
| client/src/pages/Exhibition.tsx | Replaced inline ExhibitionImages with shared WorksImages |
| client/src/pages/CategoryWork.tsx | Renamed works_url to worksUrl, removed redundant fragment |
| client/src/pages/Category.tsx | Replaced inline CategoryImages with shared WorksImages |
| client/src/pages/ArrangerWork.tsx | Removed redundant fragment wrapper |
| client/src/pages/Arranger.tsx | Added type annotation on arrangerWorks, replaced inline images |
| client/src/components/WorksImages.tsx | Introduced shared component for listing work thumbnails |
| client/src/components/WorkImages.tsx | Removed unnecessary fragment wrapper in WorkImages |
Comments suppressed due to low confidence (2)
client/src/pages/Index.tsx:48
- The new loading branch (
if (!exhibitions)) introduces conditional UI that should be covered by a unit or integration test to ensure the loading state renders correctly.
if (!exhibitions) {
client/src/components/WorksImages.tsx:2
- Importing Link from 'react-router' may not work in a browser environment; typically Link comes from 'react-router-dom'.
import { Link } from "react-router";
| {works.map((work, index) => ( | ||
| <li key={index}> |
Copilot
AI
Jun 28, 2025
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.
Using the array index as the React key can cause rendering issues; consider using a unique identifier like work.id for the key prop.
| {works.map((work, index) => ( | |
| <li key={index}> | |
| {works.map((work) => ( | |
| <li key={work.id}> |
| <Link to={`work/${work.id}`}> | ||
| <img | ||
| className="works-image-list__image" | ||
| src={`${import.meta.env.VITE_API_BASE_URL}/images/${work.image_ids[0]}`} |
Copilot
AI
Jun 28, 2025
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.
Accessing work.image_ids[0] without first checking for its existence can lead to runtime errors if image_ids is empty or undefined; add a guard or fallback.
| src={`${import.meta.env.VITE_API_BASE_URL}/images/${work.image_ids[0]}`} | |
| src={ | |
| work.image_ids && work.image_ids.length > 0 | |
| ? `${import.meta.env.VITE_API_BASE_URL}/images/${work.image_ids[0]}` | |
| : `${import.meta.env.VITE_API_BASE_URL}/images/placeholder.png` | |
| } |
Index.tsxに非同期処理の待機時の状態を追加close #185