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

[김창희] sprint6 #87

Conversation

changhui-chan
Copy link
Collaborator

@changhui-chan changhui-chan commented Sep 14, 2024

기본

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

베스트 상품
Desktop : 4개 보이기
Tablet : 2개 보이기
Mobile : 1개 보이기

전체 상품
Desktop : 12개 보이기
Tablet : 6개 보이기
Mobile : 4개 보이기

  • 상품 등록 페이지 주소는 “/additem” 입니다.
  • 페이지 주소가 “/additem” 일때 상단네비게이션바의 '중고마켓' 버튼의 색상은 “3692FF”입니다.
  • 상품 이미지는 최대 한개 업로드가 가능합니다.
  • 각 input의 placeholder 값을 정확히 입력해주세요.
  • 이미지를 제외하고 input 에 모든 값을 입력하면 ‘등록' 버튼이 활성화 됩니다.
  • API를 통한 상품 등록은 추후 미션에서 적용합니다.

심화

  • 페이지 네이션 기능을 구현합니다.
  • 이미지 안의 X 버튼을 누르면 이미지가 삭제됩니다.
  • 추가된 태그 안의 X 버튼을 누르면 해당 태그는 삭제됩니다.

주요 변경사항

  • 스프린트 미션 5 진행
  • 스프린트 미션 6 진행
  • 페이지별 커스텀 훅 사용

배포 사이트

https://cheerful-salamander-19dcbf.netlify.app/

멘토에게

  • items 페이지의 각 컴포넌트를 급하게 분리하느라 예상치 못한 에러가 있을 수 있습니다. 아직까지 한 컴포넌트가 담당하는 기능이 많은 거 같아 분리작업은 현재도 진행 중입니다
  • items 페이지의 하트 버튼은 임시로 추가해놔서 의도했던 기능과는 살짝 다릅니다. 시간 나면 같이 구현할 예정입니다.
  • items 페이지의 드롭다운의 빈 화면 클릭 시 false로 바꾸거나 디테일한 동작은 구현이 안 되어 있는 상태라 시간이 나면 같이 구현할 예정입니다.
  • additem, item 페이지에서 탭으로 focus를 하는 동작을 시간 나면 구현하려고 합니다.
  • 아무튼 시간이 필요한 작업들이 많네요.. 드롭다운의 디테일한 동작이라던지 키보드로만 홈페이지를 탐색하는 사용자를 위한 기능 고려와 같이 제가 놓친 구현하면 좋은 동작이나 기능들 있으면 첨언해주시면 감사하겠습니다.

@changhui-chan changhui-chan added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Sep 14, 2024
Copy link
Collaborator

@GANGYIKIM GANGYIKIM left a comment

Choose a reason for hiding this comment

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

창희님 이번주차 스프린트 미션 고생하셨습니다~
기존에 작업하셨던 양이 꽤 되는데 그것도 다 옮기시고 손이 정말 빠르시군요 👍
다음주 스프린트 미션과 분리작업도 화이팅입니다~

  • 아무튼 시간이 필요한 작업들이 많네요.. 드롭다운의 디테일한 동작이라던지 키보드로만 홈페이지를 탐색하는 사용자를 위한 기능 고려와 같이 제가 놓친 구현하면 좋은 동작이나 기능들 있으면 첨언해주시면 감사하겠습니다. : 컴포넌트의 동작은 생각보다 세세한 부분까지 고려해야합니다. 제가 코멘트로 남긴 상품 가격 인풋만해도, 창희님께서 사용자를 위해 ','를 추가하고 나서는 이에 따라 필요한 로직들이 여러개가 발생되지요. dropdown도 그러합니다. esc 키를 눌렀을 때 dropdown이 닫히게 하기, dropdown에 focus가 있을 시 arrowDown, arrowUp key로 option들을 탐색하게 하기, dropdown option에서 key enter 이벤트 발생시 해당 option이 적용되게 하기 등 다양한 기능이 있으므로 한번 고민해보시고 창희님이 생각하시는 좋은 동작에 대해 고민해보시는것을 추천드려요~

Comment on lines +19 to +24
<NavLink
to="/free"
className={
isFreeBoardPath ? `${styles['nav-link']} ${styles['active']}` : styles['nav-link']
}
>
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:
NavLink 태그에서 to 값과 현재 페이지가 일치하는지에 대한 값을 제공합니다.
아래처럼 사용하시는 것을 추천드립니다.

Suggested change
<NavLink
to="/free"
className={
isFreeBoardPath ? `${styles['nav-link']} ${styles['active']}` : styles['nav-link']
}
>
<NavLink
to="/free"
className={({ isActive })
isActive ? `${styles['nav-link']} ${styles['active']}` : styles['nav-link']
}
>

*react-router-dom NavLink
https://reactrouter.com/en/main/components/nav-link

import styles from "./AddProduct.module.css";

const AddProduct = ({
productImage,
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
사용하지 않는 속성은 지워주시는게 가독성에 더 좋습니다.

Suggested change
productImage,

Comment on lines +6 to +9
productTitle,
productDescription,
productPrice,
productTags,
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
속성값을 모두 받는 것보다 아래처럼 disabled 값을 받아서 전달만 해주시면 좋겟습니다.
해당 컴포넌트에서 이러한 속성을 모두 알 필요가 없으니까요~

Suggested change
productTitle,
productDescription,
productPrice,
productTags,
isDisabled


const AddProductImage = ({ productImage, setProductImage }) => {
const imageContentRef = useRef(null);
const errorMessageRef = useRef(null);
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:
react에서 useRef는 특수한 경우 사용되는 escape hatch 입니다.
이러한 경우는 useState를 사용하시는 것을 추천드립니다.

https://ko.react.dev/learn/referencing-values-with-refs#differences-between-refs-and-state

<p>이미지 등록</p>
</div>
</label>
<input className={styles['image-upload-input']} onChange={handleFileRead} type="file" id="file" accept="image/*" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
accept 속성은 유저가 파일선택시 가능한 파일 확장자를 "제안"하는 역할을 합니다.
여기서 중요한점은 제한하는 것이 아니라는 것입니다. 유저는 해당 속성외의 파일을 업로드할 수 있습니다.
따라서 accept로 제안한 확장자 이외의 것을 업로드할 시 onChange같은 함수에서 이를 확인해
업로드하지 않는 로직이 필요합니다.

const ProductTagList = ({ productTags, setProductTags}) => {

const deleteTag = (tag) => {
setProductTags(productTags.filter((e) => e !== tag));
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:
e는 일반적으로 event를 의미하는 것이니 다른 이름으로 하시는것이 가독성에 더 좋습니다.

const deleteTag = (selectedTag) => {
    setProductTags(productTags.filter((tag) => tag !== selectedTag));

@@ -0,0 +1,22 @@
import { useState } from "react";

export const useAddItemSharedData = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
data보다 state가 더 적절한 이름인 것 같습니다.

Suggested change
export const useAddItemSharedData = () => {
export const useAddItemState= () => {

<SearchIcon className={styles['search-icon']} />
<input className={styles['search-input']} type="text" placeholder="검색할 상품을 입력해주세요" />
</div>
<Link to='/additem'><button className={styles['search-button']}>상품 등록하기</button></Link>
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
Link태그안에 button태그가 있기보다 아래처럼 하시는 것을 추천드립니다.

Suggested change
<Link to='/additem'><button className={styles['search-button']}>상품 등록하기</button></Link>
<Link to='/additem' className={styles['search-button']}>상품 등록하기</Link>

{
isDropdown &&
<ul className={styles['search-dropdown-list']}>
<li onClick={() => { setSortProduct('최신순'); setIsDropdown(!isDropdown) }}>최신순</li>
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
li보다 clickable tag로 사용하시길 추천드립니다.

Suggested change
<li onClick={() => { setSortProduct('최신순'); setIsDropdown(!isDropdown) }}>최신순</li>
<li>
<button type='button'
value='최신순'
// 해당 함수 좋아요와 공통으로 사용가능
onClick={(e) => {
setSortProduct(e.target.value);
setIsDropdown(!isDropdown)
}}>최신순</button>
</li>

Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:
페이지네이션이 tablet,moblie에서 20 page에 있다고 가정해봅시다.
그러다가 pc 사이즈로 화면 사이즈가 커지게 되면 해당 사이즈에서는 20 page가 없기 때문에 빈 화면이 노출됩니다.
이와 관련된 로직도 추가해주시면 좋을 것 같고, 컴포넌트 작성후 동작에 대한 테스트를 해보시기를 추천드립니다~

@GANGYIKIM GANGYIKIM merged commit f2a31be into codeit-bootcamp-frontend:React-김창희 Sep 19, 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