-
Notifications
You must be signed in to change notification settings - Fork 24
[1단계 미션] 정창우 미션 제출합니다 #37
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
Conversation
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.
Hello @ChangwooJ, 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 by ChangwooJ is a submission for a React refactoring mission. The original HTML and CSS code for a restaurant list website is converted into React components with modular CSS. The main components created are Header, Body, and Aside, with further division within them. The Body section includes CategorySortFilter and RestaurantList components. The Aside section includes AddRestaurantModal and RestaurantInfoModal components. The pull request also includes updates to package-lock.json and package.json to add necessary dependencies and a global CSS file and module CSS files for styling.
Highlights
- Component Restructuring: The original HTML structure is divided into React components: Header, Body (containing CategorySortFilter and RestaurantList), and Aside (containing AddRestaurantModal and RestaurantInfoModal).
- Modular CSS Implementation: CSS modules are used for styling, providing component-level CSS scoping to avoid naming conflicts and improve maintainability.
- Dependency Updates: package-lock.json and package.json are updated to include necessary dependencies such as typescript-plugin-css-modules, and other supporting libraries.
- Global CSS: A global CSS file (App.css) is added to define global styles, such as basic resets, typography, and color variables.
Changelog
Click here to see the changelog
- package-lock.json
- Added typescript-plugin-css-modules as a dev dependency.
- Added @adobe/css-tools as a dev dependency.
- Added @parcel/watcher and related platform-specific packages as dev dependencies.
- Added @types/postcss-modules-local-by-default and @types/postcss-modules-scope as dev dependencies.
- Added braces, chokidar, copy-anything, cssesc, detect-libc, dotenv, errno, fill-range, graceful-fs, iconv-lite, icss-utils, image-size, immutable, is-number, is-what, less, lilconfig, lodash.camelcase, make-dir, micromatch, mime, needle, node-addon-api, parse-node-version, pify, picomatch, postcss-load-config, postcss-modules-extract-imports, postcss-modules-local-by-default, postcss-modules-scope, postcss-selector-parser, postcss-value-parser, prr, readdirp, reserved-words, safer-buffer, sass, sax, source-map, strip-bom, stylus, to-regex-range, tsconfig-paths, tslib, util-deprecate, and yaml as dev dependencies.
- Updated versions of various dependencies.
- package.json
- Added typescript-plugin-css-modules as a dev dependency.
- src/App.css
- Created a global CSS file to define base styles, color variables, and typography.
- src/App.jsx
- Modified to include Header, Body, and Aside components.
- src/component/aside/AddRestaurantModal.tsx
- Created a new component for the 'Add Restaurant' modal, including form elements for category, name, and description.
- src/component/aside/RestaurantInfoModal.tsx
- Created a new component for the 'Restaurant Info' modal, displaying restaurant name and description.
- src/component/body/CategorySortFilter.jsx
- Created a new component for category filtering.
- src/component/body/RestaurantList.jsx
- Created a new component to display the list of restaurants with category icons, names, and descriptions.
- src/css/Body.module.css
- Created a CSS module for styling the Body components, including filter and restaurant list sections.
- src/css/Header.module.css
- Created a CSS module for styling the Header component, including title and add button.
- src/css/Modal.module.css
- Created a CSS module for styling the modal components, including backdrop, container, form items, and buttons.
- src/global.d.ts
- Created a global type definition file to support CSS modules.
- src/pages/Aside.jsx
- Created a page component to render RestaurantInfoModal and AddRestaurantModal components.
- src/pages/Body.jsx
- Created a page component to render CategorySortFilter and RestaurantList components.
- src/pages/Header.jsx
- Created a page component to render the header section.
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.
Did you know?
React was initially developed by Jordan Walke, a software engineer at Facebook, and was first deployed on Facebook's newsfeed in 2011.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
The pull request introduces a refactored React application displaying a list of restaurants, separating concerns into components and applying CSS modules. The structure seems well-organized, and the use of CSS modules is a good practice. The author has also asked some good questions about folder structure and CSS module usage, demonstrating a desire to learn and improve. Overall, this is a good start, but there are some areas that could be improved, as highlighted in the review comments.
Merge Readiness
The pull request is not yet ready for merging. There are some medium and high severity issues that need to be addressed before merging. I am unable to directly approve the pull request, and recommend that others review and approve this code before merging. I recommend that the pull request not be merged until the issues I've raised are addressed.
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.
@ChangwooJ 👋🏻
반갑습니다, 창우님! 저는 @greedy-team 에서 프론트엔드 외부 리뷰어를 맡고 있는 김의천이라고 합니다. 커뮤니티에서는 흔히 요술토끼 또는 wizardrabbit이라는 닉네임으로 불리기도 합니다. 실명과 닉네임 중 어느 쪽으로든 편하신 쪽으로 불러 주셔도 좋습니다. 반갑습니다!
보아하니 React 경험이 이미 있으셨나 보군요. 미션이 시시하셨을 수도 있겠군요. 하지만 여전히 배워볼만한 부분을 찾아볼 수도 있으니 저와 함께 더 탐구를 해 봅시다!
2단계 리뷰로 이동하기 ➡
1️⃣ 답변
폴더 구조?
저는 일단 기초가 될 페이지를 따로 pages폴더를 만들어 묶고 각 페이지에 해당하는 기능 별 폴더를 components 폴더 안에서 나눠서 제공했습니다.
혹은 기본적인 페이지이니 pages에 전부 구성해야했나 싶기도 합니다.
보통은 어떤 식으로 폴더를 구성하는지에 대한 조언을 구하고 싶습니다. 매번 이렇게 만들 때마다 고민이 많이 되는 것 같아요..
많은 프로젝트에서 채용하는 몇 가지 폴더를 제외하고는 폴더 구조가 엄격하게 정해져 있지는 않아요. 그 몇 가지 폴더명을 적어드릴게요.
components/: 여러 컴포넌트들을 저장하는 폴더에요. 재사용이 가능하도록 설계되었든 그렇지 않든, 페이지 하나를 다루는 큰 단위가 아니라면 대부분의 경우 이곳에 두기에 적합해요. 참고로 폴더 이름에는 복수형을 사용하는 경우가 많아요.images/또는assets/: 이미지만을 저장한다면 전자, 다른 리소스들도 포함한다면 후자가 많이 쓰이는 방식같아요. 요즘은 후자를 사용하는 케이스가 많아지고 있어요.pages/: 재사용에 적합하지 않은 페이지 단위의 컴포넌트라면 이곳에 저장하기 적합해요.hooks/: 재사용에 용이한 여러 hook들을 저장하기에 적합해요.utils/: 범용성이 높은 유틸 함수들, 객체들을 저장하기에 적합해요. 그냥 함수랑 유틸 함수의 차이는 무엇일까요? 유틸 함수가 지니는 추가적인 특징은 이 프로젝트에만 국한되지 않으며 다른 프로젝트에 적용되어도 손색없는 범용성이 높은 함수라는 것이에요.- 숫자야구 라운드 결과를 반환하는 함수는 숫자야구 게임 기능과 긴밀하게 연관된 함수로, 범용적이지 않고 특정 기능을 위해 사용되도록 설계된 예시라고 생각해요.
- 정수 두 개를 받아 그 범위 내의 랜덤값을 반환하는 함수는 특정 기능에 꼭 치우쳐있지 않고 범용적이며 이 함수를 필요로 하는 어떤 기능에도 활용할 수 있도록 설계된 예시라고 생각해요.
constants/: 프로젝트에서 관리하는 상수들을 저장하고 관리하기에 적합해요.
이 이름들을 제외하고는 상황에 따라 다른 폴더명을 추가하셔도 무방해 보입니다. 또한 사용하시는 프레임워크 및 기술에 따라 크게 달라지기도 합니다.
- 저는 React 프로젝트 위에 WXT라는 Chrome 확장 프로그램 프레임워크를 사용하게 되었는데 추천하는 폴더 구조가 있어
src/폴더 자체를 사용하지 않고entrypoints/라는 폴더를 추가로 사용하게 되었어요.
여전히 감이 잘 안 잡히신다면 여러 오픈소스들의 폴더 구조를 보시는 것으로 인사이트를 얻어가실 수도 있다고 생각합니다. ✅
module.css 의 사용 방식?
Header.jsx에 보면 css 적용방식이 styles.(classname)으로 되어 있습니다. 이 경우 '-'이 포함되게 되면 js가 이를 인식해서 문제가 되었습니다. 이에 대한 해결 방식 중에 저는 styles[(classname)]으로 해결하였는데 올바른 방식일까요?
네, 올바른 방식입니다.
module.css의 스타일을 컴포넌트에 적용하려면 객체의 프로퍼티 값에 접근한 값을 사용해야 하는데, styles.(classname) 와 styles[(classname)] 모두 적합한 방법이기 때문에 대체가 충분히 가능합니다.
객체의 프로퍼티 값에 접근하는 방법은 크게 두 가지입니다.
1️⃣ 마침표/점 표기법
styles.foo- 정적으로 접근. 동적인 값을 넣어야 할 경우에는 적합하지 않음
- 키 값이 식별자일 때만 가능. 식별자는 영어 대소문자, 숫자,
$,_만으로 이루어져 있어야 하며 숫자로 시작할 수 없음.
2️⃣ 대괄호 표기법
styles["foo"]- 동적인 값에 대응 가능하므로 이러한 상황에서 적합
- 키 값이 식별자의 네이밍 규칙을 따르지 않아도 괜찮음
이 글도 추천드립니다!
결론적으로 프로퍼티에 접근하기 위한 접근자를 올바르게 사용하셨고, 작성하신 프로젝트에서는 css 속성명에 -(hyphen)이 포함되기 때문에 식별자 이름 규칙을 따르지 않으므로 두 번째 방법을 사용해야 하며, 적합한 방법입니다.
html 태그 선택자는 module.css에서 방법이 없나요?
태그 선택자로 구성된 아래와 같은 css 스타일링 코드는 module.css 방식에서는 적용되지 않는다고 하는 것 같습니다.
css파일은 수정하지 않는 것이 목표인것 같아서 수정하지 않았는데, 별다른 방법이 있을까요?
으음, 결론적으로 module.css에서 태그 선택자를 사용하셔도 스타일은 정상 적용됩니다. 다만 태그 선택자만 단독으로 사용하는 경우 는 module.css를 사용할 이유를 꽤 많이 줄인다고 생각합니다. 태그 선택자와 클래스 선택자를 같이 사용하는 경우는 괜찮습니다.
창우님은 CSS Module을 사용하신 이유가 무엇인가요? 사용 이유에 대해서 고민해 보시면 태그 선택자를 왜 단독으로 이용하지는 않는지에 대한 답변을 내리실 수 있을 것이라 생각합니다.
2️⃣ 리뷰
역시 말씀해주신대로 React 사용 경험이 있으신 만큼 능숙하게 요구사항들을 잘 이행해주신 것 같습니다.
- 마이그레이션 전과 후의 로직이 같도록 잘 마이그레이션 해 주셨고
className,htmlFor등 사용해야 하는 JSX 문법의 속성명도 잘 변경해주셨고- 함수형 컴포넌트 및 파일 구조도 생각을 가지고 잘 나눠주신 것 같습니다.
그렇기 때문에 이 단계에서 크게 드릴 코멘트는 별로 없었습니다. 👍🏻 몇 개의 질문 코멘트들이 있습니다.
요구사항 외에는 코드의 포매팅이 좀 더 깔끔하게 이루어지면 좋겠다는 생각이 들었습니다. 코드에 인덴트가 2인 곳도 있고, 4인 곳도 있었고, 세미콜론의 사용 여부가 통일되지 않았더라고요. 컴포넌트를 사용하게 되는 경우 특히 이 depth가 상당히 깊어지기 때문에 그만큼 인덴트 통일의 가능성은 커지게 된다고 생각합니다.
손으로 직접 인덴트를 맞출 수도 있지만, 이제 구조가 복잡하기 때문에 일일이 그렇게 하시기는 쉽지 않으실 거에요. 어찌저찌 했더라도 이후 실수를 할 수도 있고요.
Prettier 라이브러리를 사용하시고, 설정해 보시는 것은 어떠세요? 미리 정해놓은 일관된 포매팅 스타일로 프로젝트를 관리할 수 있고, 때로는 복잡한 로직이 들어가 중구난방이 된 코드를 단 한 번의 조작으로 깔끔하게 탈바꿈시킬 수 있을 거에요.
3️⃣ 질문
다음 리뷰 요청 시, 아래의 질문에 대해 의견을 공유해주실 수 있으실까요?
- Q. CSS Module를 본 프로젝트에 도입하신 이유가 무엇인가요? 어떤 장점을 누리고 싶으신가요?
2단계 리뷰로 이동하기 ➡
src/App.jsx
Outdated
| <Header></Header> | ||
| <Body></Body> | ||
| <Aside></Aside> |
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.
의견에 따라 갈리는 부분이지만, children이 없는 태그라면 self-closing tag를 사용해 보시는 것에 대해서는 어떻게 생각하시나요!
여기에 더해, <input> 등의 태그의 경우 HTML 태그를 닫지 않아도 문제 없이 사용할 수가 있었잖아요? JSX 문법을 쓰는 현 상황에서는 <Header>와 같이 children이 없는 태그는 닫지 않고 쓸 수는 없는 것인가요?
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.
저도 그것이 좋다고 생각합니다!
구체적인 이유가 있는 것은 아니지만 그렇게 하는 편이 가독성에 좋다고 생각해요.
여기에 더해,
<input>등의 태그의 경우 HTML 태그를 닫지 않아도 문제 없이 사용할 수가 있었잖아요? JSX 문법을 쓰는 현 상황에서는<Header>와 같이 children이 없는 태그는 닫지 않고 쓸 수는 없는 것인가요?
음.. </>태그는 저는 끝을 알려주는 것이라고 알고 있습니다. 그러니까 자바스크립트는 끝이 어디인지 정확히 지정해주어야 하는데.. 솔직히 말씀드리면 input 태그가 닫지 않아도 사용할 수 있는지 몰랐습니다.
일단, JSX문법, 그러니까 곧 JavaScript문법에서는 해당 태그의 끝이 어디인지 알려주어야 하기 때문에 닫지 않고 사용할 수는 없습니다. JSX가 어디서 시작했는지는 알지만 어디서 끝나는지는 모르게 되니 안에 어떤 내용이 들어가는 지도 모르게 되어 오류가 나는 것이죠!
그리고 찾아보니, input이나 img 같은 태그는 Void Elements라고 불립니다.
Void Elements는 내용을 가질 수 없는 태그 입니다. 그렇기 때문에 으로 사용하여야 할 이유도 없으며, 그렇게 해서도 안됩니다.
이 외에 특별한 이유는 찾지 못했지만, 그냥 그렇게 작동되도록 만들어진 것..?으로 보입니다.
찾다보니 알게된 사실인데, 이런 Void Elements들도 JSX에서는 클로징 태그가 필요하다고 합니다. 위에서 말한 이유와 같은 이유로 끝이 어딘지 명시해야하기 때문이라고 합니다.
src/pages/Aside.jsx
Outdated
| const Aside = () => { | ||
| return ( | ||
| <> | ||
| <RestaurantInfoModal></RestaurantInfoModal> | ||
| <AddRestaurantModal></AddRestaurantModal> | ||
| </> | ||
| ) | ||
| } |
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.
<>...</>는 무엇이고 무슨 역할을 하는 걸까요?- 이 코드에 쓰인
<>...</>는 생략이 가능한가요, 아니면 생략할 수 없을까요? 이유는 무엇이라고 생각하시나요?
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.
<></>는 리액트에서 반드시 하나의 요소만은 반환해야하는 룰을 지키기 위함입니다.
React.Fragment로 사용하기도 했지만, 굳이 그렇게 하지 않아도 되는 것 같아, <></>만을 표기하여 부모요소로 묶고 있습니다.
생략하게 된다면, 그 내부의 자식 요소가 하나라면 괜찮지만 지금처럼 두가지 이상이라면 오류를 반환하게 됩니다.
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.
그렇습니다. 지금 이 상황에서는 <>...</>는 생략할 수 없습니다.
내용들을 다시 정리해 보겠습니다.
<>...</>,<Fragment>...</Fragment>모두 Fragment라 부릅니다.<>...</>는 shorthand(축약)된 버전입니다.- 요소를 하나만 반환할 수 있는 것은 JSX의 규칙입니다.
- Fragment는 DOM에 영향을 주지 않으면서도, 여러 자식 요소들을 그룹으로 묶기 위해 사용합니다.
React 공식문서를 잘 읽어보시면 내용이 정리가 잘 될 것이라 생각합니다. 앞으로도 모르는 개념이 나올 때마다 공식 문서를 보실 것을 추천드립니다.
src/pages/Header.jsx
Outdated
| const Header = () => { | ||
| return ( | ||
| <header className={styles.gnb}> | ||
| <h1 className={`text-title ${styles.gnb__title}`}>점심 뭐 먹지</h1> |
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.
오 module.css에서 styles를 import하고 className에 적용하셨더라고요. text-title처럼 클래스명을 그대로 적는 것과는 무엇이 다른걸까요?
개발자 도구를 켜고 이 부분의 스타일이 어떻게 변경되었는지도 한 번 살펴봐 보시겠어요? 무언가 신기한 일이 있어났을 수도요?
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.
전체 리뷰에 대한 답글에 포함되어 있지만, module.css는 다른 컴포넌트에서 사용시에 중복을 방지하여 충돌을 막는 역할을 합니다. 개발자 도구를 살펴보면 이를 위해 클래스명이 class="_gnb_168g4_1"와 같이 고유한 이름을 가지게 됩니다
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.
제대로 확인해보셨군요! 그렇습니다. CSS Module이 하는 일이 무엇인지를 생각하시면 이렇게 되는 이유가 이해가 가실 것이라 생각합니다.
| <h2 className={`${styles["modal-title"]} ${styles["text-title"]}`}>새로운 음식점</h2> | ||
| <form> | ||
| <div className={`${styles["form-item"]} ${styles["form-item--required"]}`}> | ||
| <label htmlFor={`${styles["category"]} ${styles["text-caption"]}`}>카테고리</label> |
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.
htmlFor(React 마이그레이션 전은for)은 어떨 때 사용할까요? 사용할 경우 어떤 효과를 볼 수 있을까요?htmlFor의 사용법이 잘못된 것으로 보이는데,htmlFor에는 실제로 어떤 값이 들어가야 할까요? 의도대로 작동하도록 수정해 보실 수 있으신가요?
미션 원본 코드를 그대로 가져오셨다는 것은 저도 알고 있습니다. 그렇지만 짚을만한 좋은 주제라 생각하며 이후 모달의 폼도 관리하실 텐데 그 때쯤이면 다른 분이 리뷰어를 맡고 계시겠죠? 그래서 저는 이번에 이야기해보자 합니다.
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.
htmlFor는 label같은 태그가 어떤 요소를 가리키는지를 명시하는 역할을 합니다.
이번 미션을 수행하면서 먼저 컴포넌트를 분리하고 이후에 css작업을 하다보니 문제가 생긴 것 같습니다.
확실히 for를 통해 특정 요소를 가리켜야하는데, 갑자기 css스타일링을 위한 내용이 이 안에 들어가게 되었네요.
이 라벨은 바로 아래에 있는 id="category"를 가리켜야합니다.
그러므로 여기서 의미하는 category는 classname도 아닐뿐더러, category 태그의 스타일이 들어가는 곳도 아닙니다.
그런데 여기서 htmlFor안에 두가지 네임이 들어가있는 것은 이상하게 보입니다.
일단 text-caption은 라벨의 요소 명시를 위한 내용이 아니라 label자체의 내용인 "카테고리"를 스타일링 하기 위한 class로 보입니다.
따라서 제 생각엔 htmlFor="category" className={styles[text-caption]} 으로 수정되어야 할 것 같습니다.
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.
말씀하신 바가 맞습니다! 잘 조사해주신 것 같아요. 고치신 부분은 확인했지만, 모달에는 총 세 개의 <label>이 있으니 나머지 <label>도 잘못된 htmlFor값 확인 후 수정을 부탁드리며, 수정 이후 <label>을 클릭해 봤을 때 대응되는 요소로 포커싱이 잘 가는지도 확인해 보시기 바랍니다!
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.
이 파일은 module.css로 바꾸지 않은 이유가 있으신가요?
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.
App.css의 내용을 보면 전체에 해당하는 내용을 담고 있습니다.
이는 특정 컴포넌트에서만 적용될 이유가 없기도 하며, 특히 Colors와 같이 어떤 값을 특정 상수로 치환해놓은 정보를 담고 있기에 모듈화 시킬 필요가 없다고 판단했습니다.
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.
이해가 잘 되는 이유네요, App.css에 있는 스타일들은 전역으로 적용되는 의도를 지닌 스타일들이 모여있으므로 .module.css를 붙이지 않는 것이 합리적일 것이라 생각합니다.
|
리뷰 감사합니다, 의천님! 먼저 module.css에 국한하여 선택자를 연결하는 과정에 집중했었는데, 말씀을 듣고나니 그렇게 생각할 것이 아니라는 것을 알게되었습니다.
이를 토대로 생각해보면, 태그 선택자를 중간에 단독으로 사용할 수도 있겠지만, 그렇게 하지 않는 이유는 css module을 사용하는 의도와 맞지 않는 방식이기 때문입니다.
앗 그렇네요.. ESLINT와 Prettier 이용한 코드 컨벤션 자동 적용을 해보겠습니다! |
wzrabbit
left a comment
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.
@ChangwooJ 👋🏻
1단계 미션 수행하시고 변경사항도 반영하시느라 고생 많으셨습니다. 포매팅이 진행되니 한결 코드 보기가 편해졌군요.
이를 토대로 생각해보면, 태그 선택자를 중간에 단독으로 사용할 수도 있겠지만, 그렇게 하지 않는 이유는 css module을 사용하는 의도와 맞지 않는 방식이기 때문입니다.
그렇습니다. css module을 왜 사용할까요? 바로 css의 스타일을 특정 범위에서만 적용 하기 위함입니다. 원치 않는 스타일의 적용을 막고, 클래스/ID명의 충돌을 막음으로써 컴포넌트를 더욱 컴포넌트답게 설계할 수 있고, 스타일로 인한 부작용을 줄일 수 있습니다.
태그 선택자만을 단독으로 사용했다는 것 자체가 그 태그를 사용하는 모든 요소에 대응되는 스타일을 적용하겠다는 의미이고, 이는 특정 범위에서만 스타일을 적용하기 위함인 목적에 어긋나는 상황이 됩니다. 이 경우에는 전역으로 스타일을 적용하도록 의도된 App.css 에 적용하는 것이 더 목적에 맞을 거라 생각합니다.
| "preview": "vite preview", | ||
| "server": "npx json-server db.json" | ||
| "server": "npx json-server db.json", | ||
| "format": "prettier --write .", |
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.
개인적으로 Prettier는 vscode의 Format on save 옵션과 같이 썼을 때 특히 편하더라고요
코드를 저장만 하면 바로 포매팅이 진행되는 거에요
관심이 있으시다면 한 번 적용해 보셔도 좋을 것 같아요 ☑️
src/pages/Aside.jsx
Outdated
| const Aside = () => { | ||
| return ( | ||
| <> | ||
| <RestaurantInfoModal></RestaurantInfoModal> | ||
| <AddRestaurantModal></AddRestaurantModal> | ||
| </> | ||
| ) | ||
| } |
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.
그렇습니다. 지금 이 상황에서는 <>...</>는 생략할 수 없습니다.
내용들을 다시 정리해 보겠습니다.
<>...</>,<Fragment>...</Fragment>모두 Fragment라 부릅니다.<>...</>는 shorthand(축약)된 버전입니다.- 요소를 하나만 반환할 수 있는 것은 JSX의 규칙입니다.
- Fragment는 DOM에 영향을 주지 않으면서도, 여러 자식 요소들을 그룹으로 묶기 위해 사용합니다.
React 공식문서를 잘 읽어보시면 내용이 정리가 잘 될 것이라 생각합니다. 앞으로도 모르는 개념이 나올 때마다 공식 문서를 보실 것을 추천드립니다.
src/pages/Header.jsx
Outdated
| const Header = () => { | ||
| return ( | ||
| <header className={styles.gnb}> | ||
| <h1 className={`text-title ${styles.gnb__title}`}>점심 뭐 먹지</h1> |
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.
제대로 확인해보셨군요! 그렇습니다. CSS Module이 하는 일이 무엇인지를 생각하시면 이렇게 되는 이유가 이해가 가실 것이라 생각합니다.
| <h2 className={`${styles["modal-title"]} ${styles["text-title"]}`}>새로운 음식점</h2> | ||
| <form> | ||
| <div className={`${styles["form-item"]} ${styles["form-item--required"]}`}> | ||
| <label htmlFor={`${styles["category"]} ${styles["text-caption"]}`}>카테고리</label> |
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.
말씀하신 바가 맞습니다! 잘 조사해주신 것 같아요. 고치신 부분은 확인했지만, 모달에는 총 세 개의 <label>이 있으니 나머지 <label>도 잘못된 htmlFor값 확인 후 수정을 부탁드리며, 수정 이후 <label>을 클릭해 봤을 때 대응되는 요소로 포커싱이 잘 가는지도 확인해 보시기 바랍니다!
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.
이해가 잘 되는 이유네요, App.css에 있는 스타일들은 전역으로 적용되는 의도를 지닌 스타일들이 모여있으므로 .module.css를 붙이지 않는 것이 합리적일 것이라 생각합니다.
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.
이 설정 파일도 single quote로 포매팅이 되면 좋을 것 같아보이는군요
안녕하세요, wzrabbit 리뷰어님!
저는 그리디 2기 프론트엔드로 들어온 정창우라고 합니다. 3주차 미션으로 처음 뵙게 되었네요 잘부탁드립니다!
리액트를 처음 접해보는 것은 아니지만, 꼼꼼히 배우지는 않아서 모자란 부분이 아주 많다고 생각합니다. 그래서 모자란 부분을 많이 채우려고 노력하고 있습니다. 제 코드를 리뷰해주시면서 아쉽거나 보완했으면 하는 부분이 있으면 지적해주시면 감사하겠습니다!
이번주 미션은 음식점 리스트를 보여주는 사이트의 코드를 리팩토링 하는 과정이었습니다. html과 css로 작성된 파일을 리액트의 방식대로 컴포넌트로 분리하고, module.css를 사용하여 css를 적용하는 과정이었습니다.
저는 html파일에 크게 Header, Body, Aside 세 가지로 기능이 분리되어 있다고 생각해서 먼저 세 개의 페이지로 나누고, 그 안의 구성요소를 컴포넌트로 분리하여 작성했습니다.
Header는 틀별히 분리할 필요 없다고 판단하여 그대로 두었고, Body에는 카테고리/정렬 필터 기능에 해당하는 코드를 CategorySortFilter.jsx 파일로 음식점 목록 기능에 해당하는 코드를 RestaurantList.jsx 파일로 분리하였습니다.
마지막으로 Aside에는 음식점 추가 모달에 해당하는 코드를 AddRestaurantModal.jsx 파일로, 음식점 정보 모달에 해당하는 코드를 RestaurantInfoModal.jsx 파일로 분리하였습니다.
그리고 각 컴포넌트에 적용시킬 css파일을 module.css로 제공하여 적용하였습니다.
아래는 구현된 페이지 이미지입니다.
마지막으로 리팩토링 하면서 궁금했던 점이 있습니다!
폴더 구조?
저는 일단 기초가 될 페이지를 따로 pages폴더를 만들어 묶고 각 페이지에 해당하는 기능 별 폴더를 components 폴더 안에서 나눠서 제공했습니다.
혹은 기본적인 페이지이니 pages에 전부 구성해야했나 싶기도 합니다.
보통은 어떤 식으로 폴더를 구성하는지에 대한 조언을 구하고 싶습니다. 매번 이렇게 만들 때마다 고민이 많이 되는 것 같아요..
module.css 의 사용 방식?
Header.jsx에 보면 css 적용방식이 styles.(classname)으로 되어 있습니다. 이 경우 '-'이 포함되게 되면 js가 이를 인식해서 문제가 되었습니다. 이에 대한 해결 방식 중에 저는 styles[(classname)]으로 해결하였는데 올바른 방식일까요?
html 태그 선택자는 module.css에서 방법이 없나요?
태그 선택자로 구성된 아래와 같은 css 스타일링 코드는 module.css 방식에서는 적용되지 않는다고 하는 것 같습니다.
css파일은 수정하지 않는 것이 목표인것 같아서 수정하지 않았는데, 별다른 방법이 있을까요?
css 코드의 선택자를 없애고 .form-item-label와 같이 이름을 바꾸는 방법 외에는 모르겠습니다..
아래는 Directory(Component) Structure 입니다.