-
Notifications
You must be signed in to change notification settings - Fork 0
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
KDT5 Team4_4조 토이프로젝트 제출_(양준용, 김다슬, 이용수, 송수연) #10
base: main
Are you sure you want to change the base?
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.
총평
좋았던 점
가지고 있는 정보를 활용하여 시각적으로 다양한 기능을
구현하려고 노력하신 부분이 인상 깊었습니다.
api가 잘 정리되어 있어 좋을 것 같고 폴더 구조도 고민을 하신 것 같아 좋습니다.
아쉬운 점
api 관련 타입 설정이나 예외처리 부분이 일관적이지 않은 것 같습니다.
컴포넌트 내에 불필요한 변수가 들어 있거나
비슷한 로직을 가진 반복되는 컴포넌트가 많이 있는 것 같습니다.
컴포넌트 하나당 들고있는 내용이 많은 것 같습니다.
store를 활용 하는 등 역할을 나누어 보면 좋을 것 같습니다.
불필요한 요청이나 렌더링이 발생하게 되는 부분이 있습니다.
이런 부분은 기간적으로 짧아서 어쩔 수 없는 부분이라 생각합니다.
시간이 되면 몇군데 최적화를 해보시면 좋을 것 같아요.
질 만드셨습니다! 고생하셨습니다!
useEffect(() => { | ||
const time = new Date() | ||
setDay(time.getFullYear() + '년' + (time.getMonth() + 1) + '월' + time.getDate() + '일') | ||
}, []) | ||
|
||
useEffect(() => { | ||
const Timer = setInterval(() => { |
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.
하나의 useEffect 안에서 처리 가능할 것 같아요.
시간을 구하고 set하는 같은 동작을 함수로 만들 수 있을 것 같아요.
const CATEGORYOPTION = [ | ||
{ label: '카테고리', value: '카테고리' }, | ||
{ label: '페이스', value: '페이스' }, | ||
{ label: '립', value: '립' }, | ||
{ label: '아이', value: '아이' }, | ||
] |
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.
컴포넌트 외부에 선언해도 좋을 것 같습니다.
const THUMB_MAX = 1024 ** 2 //1MB | ||
const DETAIL_MAX = 1024 ** 2 * 4 //4MB |
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.
매직 넘버를 만들지 않은 것은 좋으나
위 리뷰와 동일하게 컴포넌트 외부에 가지고 있어도 될 것 같습니다.
Category.length === 0 || | ||
Category[0] === '카테고리' || | ||
Title.length === 0 || | ||
Price === 0 || | ||
Description.length === 0 || | ||
Thumb.length === 0 || | ||
Detail.length === 0 |
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.
따로 validation하는 함수를 만들어
현재 비교 사항이 어떤 것을 위한 확인 인지 명칭을 정하는 것이 어떨까요?
나중에는 if 조건이 왜 어떤 것을 확인하기 위해 왜 이렇게 작성되어있는지
다시 읽어봐야 알 수 있을 것 같아요
import { useState, useEffect } from 'react' | ||
import { addProduct, editProduct, Product } from '../apis/api' | ||
|
||
export const AdminForm = ({ formtype }: { formtype: string }) => { |
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.
컴포넌트 하나에 많은 처리가 있는 것 같습니다.
컴포넌트를 나눌 것은 나누고 state(변화 시 재 렌더링이 필요한 변수)는 store로 관리하는 등
간단하게 만들어 각 컴포넌트의 역할을 파악하기 쉽게 만들
방법을 고민해 보면 좋을 것 같습니다.
const CoolProduct = () => { | ||
const navigate = useNavigate() | ||
|
||
const Product = [ |
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.
내부에 선언하지 않아도 될 것 같습니다.
|
||
return ( | ||
<div> | ||
<GlobalStyle /> |
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.
매 컴포넌트 마다 부르게 작성하신 이유가 있을까요?
|
||
productList?.forEach((product) => (productTotal += product.price)); | ||
total = productTotal + DELIVERY_CHARGE; | ||
|
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.
total의 경우 state나 memo로 관리하는 것이 어떨까 싶습니다.
// 선택한 태그 배열을 전달하여 분류 처리 | ||
useEffect(() => { | ||
onSelectTags(selectedTags); // onSelectTags 콜백 함수 호출 | ||
}, [selectedTags, onSelectTags]); |
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.
onSelectTags를 바라볼 이유가 있을까요?
}) | ||
|
||
// 사용자 : 회원가입 | ||
export const signUp = async (email: string, password: string, displayName: string) => { |
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.
함수에서 반환하는 값의 타입이 있으면 좋을 것 같습니다.
KDT5_Team4
양준용, 김다슬, 이용수, 송수연
📑 프로젝트 주제
거래 API를 활용해 코로나 시기에 오프라인 쇼핑을 하지 못하는 소비자들을 위한
온라인 상으로 퍼스널 컬러를 진단 하고 진단 결과에 따른 제품을 추천 받고 구매할 수 있는
메이크업 제품 판매 쇼핑몰입니다.
📆 프로젝트 기간
2023.05.30 ~ 2023.07.02
🔗 배포 링크
뷰티인사이드
🔧 기술 스택
config
development
enviroment
deployment
design
📁 주요 기능
사용자
관리자
💻 구현 화면
보기
👍팀원&역할 분담
참고 사항
보기
사용자 관리자를 각각 다른 저장소에서 개발해서 제출 업로드는 합쳐서 올립니다.
개별 저장소 링크:
사용자
관리자
관리자 계정
보기
이메일: [email protected]
비밀번호: 1q2w3e4r