[1단계 미션] 조정원 미션 제출합니다 #49
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @garden-j, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request addresses the first stage mission by separating the existing App.jsx file into three distinct components: Header, CategoryFilter, and RestaurantList. This aims to improve code organization and readability by adhering to the principle of component-based architecture in React.
Highlights
- Component Separation: The primary change involves splitting the monolithic
App.jsxinto smaller, reusable components, enhancing maintainability and scalability. - CSS Styling: A new
App.cssfile has been added to manage the styles for the application, promoting a separation of concerns between the structure (JSX) and presentation (CSS). - Functional Components: The newly created components (
Header,CategoryFilter,RestaurantList) are implemented as functional components, leveraging React's hooks for managing state and side effects (though no hooks are used in this specific PR).
Changelog
Click here to see the changelog
- 01-first-component/README.md
- Minor formatting changes to the README file, adding hyphens to list items for better readability.
- src/App.css
- Creation of a new CSS file (
App.css) to define the styles for the application. - Includes styling for the header, category filter, restaurant list, and modal components.
- Defines color palettes and typography using CSS variables for consistency.
- Creation of a new CSS file (
- src/App.jsx
- Refactoring of the original
App.jsxto import and render the newHeader,CategoryFilter, andRestaurantListcomponents. - Comments out modal-related code, indicating potential future work or removal.
- Refactoring of the original
- src/components/CategoryFilter.jsx
- Creation of a new
CategoryFiltercomponent responsible for rendering the category filter dropdown. - Includes options for filtering restaurants by category (e.g., 한식, 중식, 일식).
- Creation of a new
- src/components/Header.jsx
- Creation of a new
Headercomponent to display the application's header section. - Includes the title and a button for adding new restaurants.
- Creation of a new
- src/components/RestaurantList.jsx
- Creation of a new
RestaurantListcomponent to render a list of restaurants. - Includes hardcoded restaurant data with categories, names, and descriptions.
- Creation of a new
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A component's born,
From chaos, order drawn,
React's gentle hand.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request introduces a good component separation, enhancing the project's structure and readability. The changes are well-organized, and the new components seem to be correctly implemented. However, there are a few areas that could be improved for better maintainability and code clarity.
Summary of Findings
- Inconsistent class naming: In
RestaurantList.jsx, some class names useclassinstead ofclassName, which is incorrect in React. This needs to be fixed to ensure proper styling and functionality. - Missing alt text: The
altattribute is missing in someimgtags withinRestaurantList.jsx. Providing descriptivealttext is crucial for accessibility. - Hardcoded data: The restaurant data is hardcoded in
RestaurantList.jsx. Consider using props or a data file to make the component more reusable and dynamic.
Merge Readiness
The pull request is almost ready for merging. Addressing the issues related to class naming, missing alt text, and hardcoded data will significantly improve the code quality. I am unable to approve this pull request, and recommend that others review and approve this code before merging. At a minimum, the high severity issues should be addressed before merging.
| <img | ||
| src="./category-chinese.png" | ||
| alt="중식" | ||
| class="category-icon" |
There was a problem hiding this comment.
Using class instead of className here. In React, you should use className to define CSS classes. This is a high severity issue because it will prevent the styles from being applied correctly.
className="category-icon"
There was a problem hiding this comment.
요 제미니 말처럼 class 대신 className을 쓰는 게 맞겠네요!
| <img | ||
| src="./category-asian.png" | ||
| alt="아시안" | ||
| class="category-icon" |
There was a problem hiding this comment.
Using class instead of className here. In React, you should use className to define CSS classes. This is a high severity issue because it will prevent the styles from being applied correctly.
className="category-icon"
| <img | ||
| src="./category-chinese.png" | ||
| alt="중식" | ||
| class="category-icon" |
There was a problem hiding this comment.
The alt attribute is missing for this image. It's important to provide descriptive alt text for accessibility. What would be a good description of this image?
className="category-icon"
alt="중식 카테고리 아이콘"
| <img | ||
| src="./category-asian.png" | ||
| alt="아시안" | ||
| class="category-icon" |
There was a problem hiding this comment.
The alt attribute is missing for this image. It's important to provide descriptive alt text for accessibility. What would be a good description of this image?
className="category-icon"
alt="아시안 음식 카테고리 아이콘"
|
|
||
| <li className="restaurant"> | ||
| <div className="restaurant__category"> | ||
| <img src="./category-etc.png" alt="기타" class="category-icon" /> |
There was a problem hiding this comment.
The alt attribute is missing for this image. It's important to provide descriptive alt text for accessibility. What would be a good description of this image?
alt="기타 음식 카테고리 아이콘" className="category-icon" />
| <li className="restaurant"> | ||
| <div className="restaurant__category"> | ||
| <img | ||
| src="./category-korean.png" | ||
| alt="한식" | ||
| className="category-icon" | ||
| /> | ||
| </div> | ||
| <div className="restaurant__info"> | ||
| <h3 className="restaurant__name text-subtitle">피양콩할마니</h3> | ||
| <p className="restaurant__description text-body"> | ||
| 평양 출신의 할머니가 수십 년간 운영해온 비지 전문점 피양콩 할마니. | ||
| 두부를 빼지 않은 되비지를 맛볼 수 있는 곳으로, ‘피양’은 평안도 | ||
| 사투리로 ‘평양’을 의미한다. 딸과 함께 운영하는 이곳에선 맷돌로 | ||
| 직접 간 콩만을 사용하며, 일체의 조미료를 넣지 않은 건강식을 | ||
| 선보인다. 콩비지와 피양 만두가 이곳의 대표 메뉴지만, 할머니가 옛날 | ||
| 방식을 고수하며 만들어내는 비지전골 또한 이 집의 역사를 느낄 수 | ||
| 있는 특별한 메뉴다. 반찬은 손님들이 먹고 싶은 만큼 덜어 먹을 수 | ||
| 있게 준비돼 있다. | ||
| </p> | ||
| </div> | ||
| </li> | ||
| <li className="restaurant"> | ||
| <div className="restaurant__category"> | ||
| <img | ||
| src="./category-chinese.png" | ||
| alt="중식" | ||
| class="category-icon" | ||
| /> | ||
| </div> | ||
| <div class="restaurant__info"> | ||
| <h3 className="restaurant__name text-subtitle">친친</h3> | ||
| <p className="restaurant__description text-body"> | ||
| Since 2004 편리한 교통과 주차, 그리고 관록만큼 깊은 맛과 정성으로 | ||
| 정통 중식의 세계를 펼쳐갑니다 | ||
| </p> | ||
| </div> | ||
| </li> | ||
|
|
||
| <li className="restaurant"> | ||
| <div className="restaurant__category"> | ||
| <img | ||
| src="./category-japanese.png" | ||
| alt="일식" | ||
| className="category-icon" | ||
| /> | ||
| </div> | ||
| <div className="restaurant__info"> | ||
| <h3 className="restaurant__name text-subtitle">잇쇼우</h3> | ||
| <p className="restaurant__description text-body"> | ||
| 잇쇼우는 정통 자가제면 사누끼 우동이 대표메뉴입니다. 기술은 정성을 | ||
| 이길 수 없다는 신념으로 모든 음식에 최선을 다하는 잇쇼우는 고객 | ||
| 한분 한분께 최선을 다하겠습니다 | ||
| </p> | ||
| </div> | ||
| </li> | ||
|
|
||
| <li className="restaurant"> | ||
| <div className="restaurant__category"> | ||
| <img | ||
| src="./category-western.png" | ||
| alt="양식" | ||
| className="category-icon" | ||
| /> | ||
| </div> | ||
| <div className="restaurant__info"> | ||
| <h3 className="restaurant__name text-subtitle">이태리키친</h3> | ||
| <p className="restaurant__description text-body"> | ||
| 늘 변화를 추구하는 이태리키친입니다. | ||
| </p> | ||
| </div> | ||
| </li> | ||
|
|
||
| <li className="restaurant"> | ||
| <div className="restaurant__category"> | ||
| <img | ||
| src="./category-asian.png" | ||
| alt="아시안" | ||
| class="category-icon" | ||
| /> | ||
| </div> | ||
| <div className="restaurant__info"> | ||
| <h3 className="restaurant__name text-subtitle">호아빈 삼성점</h3> | ||
| <p className="restaurant__description text-body"> | ||
| 푸짐한 양에 국물이 일품인 쌀국수 | ||
| </p> | ||
| </div> | ||
| </li> | ||
|
|
||
| <li className="restaurant"> | ||
| <div className="restaurant__category"> | ||
| <img src="./category-etc.png" alt="기타" class="category-icon" /> | ||
| </div> | ||
| <div className="restaurant__info"> | ||
| <h3 className="restaurant__name text-subtitle"> | ||
| 도스타코스 선릉점 | ||
| </h3> | ||
| <p className="restaurant__description text-body"> | ||
| 멕시칸 캐주얼 그릴 | ||
| </p> | ||
| </div> | ||
| </li> | ||
| </ul> |
There was a problem hiding this comment.
The restaurant data is currently hardcoded. For better reusability and maintainability, consider passing this data as props or fetching it from an external data source. This would allow you to easily update the restaurant list without modifying the component itself.
const RestaurantList = ({ restaurants }) => {
return (
<section className="restaurant-list-container">
<ul className="restaurant-list">
{restaurants.map((restaurant) => (
<li className="restaurant" key={restaurant.name}>
<div className="restaurant__category">
<img
src={restaurant.categoryImage}
alt={restaurant.category}
className="category-icon"
/>
</div>
<div className="restaurant__info">
<h3 className="restaurant__name text-subtitle">{restaurant.name}</h3>
<p className="restaurant__description text-body">{restaurant.description}</p>
</div>
</li>
))}
</ul>
</section>
);
};
export default RestaurantList;
| <img | ||
| src="./category-chinese.png" | ||
| alt="중식" | ||
| class="category-icon" |
There was a problem hiding this comment.
요 제미니 말처럼 class 대신 className을 쓰는 게 맞겠네요!
| <img | ||
| src="./category-asian.png" | ||
| alt="아시안" | ||
| class="category-icon" |
There was a problem hiding this comment.
아까 위에랑 똑같은 실수네용 class -> className
|
|
||
| <li className="restaurant"> | ||
| <div className="restaurant__category"> | ||
| <img src="./category-etc.png" alt="기타" class="category-icon" /> |
There was a problem hiding this comment.
ai가 알아서 다 해주네요.. 아까와 같은 class -> className, alt 속성에 관련해서 이미지에 더 자세히 설명하는 게 좋을 것 같다고 하는데 이것까진 저희가 신경쓰지 않아도 될 것 같네용
| <RestaurantList /> | ||
| </main> | ||
|
|
||
| {/* <aside> |
There was a problem hiding this comment.
전 컴포넌트로 옮기고 아예 없애버렸는데 이렇게 주석처리 하는 게 컴포넌트 분리 전을 보여주니 전/후 비교하기 좋겠네요!
2 participants
Header, CategoryFilter, RestaurantList 3개의 component를 분리했습니다.