Skip to content
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

[김강우] sprint5 #156

Conversation

hvrain
Copy link
Collaborator

@hvrain hvrain commented Jun 21, 2024

요구사항

기본

중고마켓
  • 중고마켓 페이지 주소는 “/items” 입니다.
  • 페이지 주소가 “/items” 일때 상단네비게이션바의 '중고마켓' 버튼의 색상은 “3692FF”입니다.
  • 상단 네비게이션 바는 이전 미션에서 구현한 랜딩 페이지와 동일한 스타일로 만들어 주세요.
  • 상품 데이터 정보는 https://panda-market-api.vercel.app/docs/#/ 에 명세된 GET 메소드 “/products” 를 사용해주세요.
  • '상품 등록하기' 버튼을 누르면 “/additem” 로 이동합니다. ( 빈 페이지 )
  • 전체 상품에서 드롭 다운으로 “최신 순” 또는 “좋아요 순”을 선택해서 정렬을 할 수 있습니다.
반응형

베스트 상품

  • Desktop : 4개 보이기
  • Tablet : 2개 보이기
  • Mobile : 1개 보이기

전체 상품

  • Desktop : 12개 보이기 (저는 figma의 디자인에 따라 10개 보이기로 했습니다.)
  • Tablet : 6개 보이기
  • Mobile : 4개 보이기

심화

  • 페이지 네이션 기능을 구현합니다.

주요 변경사항

  • Basic-김강우 => React-김강우 migration 완료
  • Items 컴포넌트 구현, 그에 필요한 컴포넌트도 구현

스크린샷

PC

items_pc

Tablet

items_tablet

Mobile

items_mobile

멘토에게

  • 몇가지 컴포넌트를 compound components로 구현했습니다. 부모 컴포넌트에서 자식의 모양을 커스텀 할 수 있어서 좋다고 생각했는데, 좋은 방법인가요?
  • 아직 컴포넌트를 완전히 분리하지 못했습니다. 그래서 컴포넌트가 복잡할 수 있습니다. 특히 Items 부분이 복잡합니다.
  • 매운맛으로 부탁드립니다.

@hvrain hvrain requested a review from 201steve June 21, 2024 14:28
@hvrain hvrain self-assigned this Jun 21, 2024
@hvrain hvrain added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Jun 21, 2024
@201steve
Copy link
Collaborator

몇가지 컴포넌트를 compound components로 구현했습니다. 부모 컴포넌트에서 자식의 모양을 커스텀 할 수 있어서 좋다고 생각했는데, 좋은 방법인가요?

  • 컴파운드 컴포넌트 패턴은 좋은 패턴입니다. 나중에 MUI, Antdesign, chakraUI같은 ui 컴포넌트 라이브러리를 사용할 기회가 있으시면 컴파운드 컴포넌트를 쉽게 접하실 수 있을꺼에요.
  • 다만, 우려되는건 아직 state,useEffect,props 사용과, 컴포넌트 추상화가 익숙치 않으신것같아서 디자인 패턴을 익히는 우선순위보다 react를 잘 쓰는게 우선순위가 높지않을까 싶습니다.

아직 컴포넌트를 완전히 분리하지 못했습니다. 그래서 컴포넌트가 복잡할 수 있습니다. 특히 Items 부분이 복잡합니다.

  • 만약 복잡하다고 느끼셨다면 구체적인 코드를 추상적인 컴포넌트로 만들어보세요!

매운맛으로 부탁드립니다.

  • 꽤나 매웠을것같습니다...아니면 다행이구요 :-) 수고하셨습니디ㅏ!

src/index.js Show resolved Hide resolved
src/index.jsx Show resolved Hide resolved
src/App.js Show resolved Hide resolved
src/App.jsx Show resolved Hide resolved
onChange={({ target }) => setValue(target.value)}
/>
<span
className={isValid ? "hidden" : ""}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BEM 방식으로 className convention을 정하신것같네요!
className도 일관성있게 BEM 방식으로 쓰셔야 혼동이 없을것같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basic 스프린트를 BEM으로 했는데, 마이그레이션을 한 후 일관되게 하지 못했습니다. 다만 sprint6에서는 styled-component로 바꿀거라, BEM을 삭제해야할 것 같네요

src/utils/valid.js Show resolved Hide resolved
src/components/Auth.jsx Show resolved Hide resolved
src/components/Auth.jsx Show resolved Hide resolved
Comment on lines +6 to +14
const [email, setEmail] = useState("");
const [emailValid, setEmailValid] = useState(false);
const [nickname, setNickname] = useState("");
const [nicknameValid, setNicknameValid] = useState(false);
const [password, setPassword] = useState("");
const [passwordValid, setPasswordValid] = useState(false);
const [confirmPassword, setConfirmPassword] = useState("");
const [confirmPasswordValid, setConfirmPasswordValid] = useState(false);
const [isValid, setIsValid] = useState(false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이런 값이 유효한지를 판단하는 조건은
state로 관리 하는것보다

state의 값을 기반으로 한 조건 으로 관리할 수 있습니다.

ex)

const validNickname = (nickname: string): boolean => {
 return nickname.length >= 3; // 예시 조건
};

const validPassword = (password: string): boolean => {
 return password.length >= 8; // 예시 조건
};

state인 nickName, password를 받아서 검사하는 함수인데, 반환 타입은 boolean이 됩니다. state가 없어도 되죠.

const nickNameValid = nickname.length >= 3; 
const passwordValid = password.length >= 8; 

꼭 함수가 아니어도 비교연산자는 항상 boolean 타입을 반환하니까 state의 값을 비교 연산자로 계산하면 state로 관리 하지않아도 됩니다.

그래서 모든 유효성 검사 값이 true가 되면 isValid도 true가 되겠죠. setIsValid 없이도 계산된 값들로 비교해서 결과를 얻을 수 있습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

마이그레이션하면서 이전 값을 최대한 유지하면서 수정해서 생긴 오류네요.. 평소에 react로 만들면 어떤 코드가 더 나을지 생각을 해야할 것 같습니다.

Comment on lines +16 to +24
useEffect(() => {
setConfirmPasswordValid(password === confirmPassword);
}, [password, confirmPassword]);

useEffect(() => {
setIsValid(
emailValid && nicknameValid && passwordValid && confirmPasswordValid
);
}, [emailValid, nicknameValid, passwordValid, confirmPasswordValid]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

유효성 검사 state를 전부 변수처리하면 이 코드들도 간략하게 변수로 바꿀 수 있습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

첫 번째 useEffect는 변수처리해도 될 것 같은데, 두 번째 useEffect는 focusout(onBlur)에 반응하는 함수라 useEffect로 나둬야 될 것 같습니다. 맞을까요?

.gitignore Show resolved Hide resolved
@201steve 201steve merged commit 9a5f71e into codeit-bootcamp-frontend:React-김강우 Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants