Skip to content

Conversation

@kimpra2989
Copy link
Contributor

🚀 풀 리퀘스트 제안

🔍 작업 내용

이미 해당 컴포넌트를 사용하고 있는 것들이 있는 것 같아서 기존 컴포넌트를 그대로 사용할 수 있는 상태에서 기능만 확장했습니다.
스크린샷 2024-11-25 022951

🔧 변경 사항

  1. 데이터가 없는 경우에 스크린샷처럼 tbody 대신 메시지를 보여줄 수 있도록 수정
  2. 테이블 데이터 칸에 ui가 들어가야하는 경우를 위해 기존 TableBodyDataType 확장

@github-actions
Copy link

🚀 Storybook이 배포 됐어요! 확인하러 가기 => https://6729e72ee61d8f57ca4790fb-aolbeqfszq.chromatic.com/

Copy link
Contributor

@ssumanlife ssumanlife left a comment

Choose a reason for hiding this comment

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

확장 기능 좋네요!!👍

}
}

export default useVerticalTable
Copy link
Contributor

Choose a reason for hiding this comment

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

페이지네이션의 페이지를 상태관리로 추가할 예정이라 훅으로 사용하신걸까요?
ex) [currentPage, setCurrentPage] = useState(1)
상태관리를 할 예정이 아니라면 utill쪽이 좋지 않을까요..?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

페이지네이션의 페이지를 상태관리로 추가할 예정이라 훅으로 사용하신걸까요? ex) [currentPage, setCurrentPage] = useState(1) 상태관리를 할 예정이 아니라면 utill쪽이 좋지 않을까요..?

유틸이랑은 성격이 조금 다른 것 같아요.
유틸은 도메인 정보가 없는, 꽤 범용적인 경우에 쓰는 걸로 감을 잡고 있는데 해당 코드는 컴포넌트와의 결합도가 높고 도메인 장보 역시.포함하고 있다보니 일잔훅으로 뺐습니다.

Copy link
Contributor

@ssumanlife ssumanlife Nov 26, 2024

Choose a reason for hiding this comment

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

slice부분은 페이지네이션을 사용하는 곳이라면 많이 쓰일것같은데

const sliceArray = <T>(array: T[], countPerPage: number, currentPage: number) => {
  return array.slice(
    countPerPage * (currentPage - 1),
    countPerPage * (currentPage - 1) + countPerPage
  )
}

slice부분만 utill로 분리하고 hasData는 컴포넌트 내부에서 관리하는건 별로일까요..?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

slice부분은 페이지네이션을 사용하는 곳이라면 많이 쓰일것같은데

const sliceArray = <T>(array: T[], countPerPage: number, currentPage: number) => {
  return array.slice(
    countPerPage * (currentPage - 1),
    countPerPage * (currentPage - 1) + countPerPage
  )
}

slice부분만 utill로 분리하고 hasData는 컴포넌트 내부에서 관리하는건 별로일까요..?

좋은 아이디어 같아서 생각을 조금 더 해봤습니다. 결론부터 말하자면, 작성해주신 slcedArray함수는 @shared/utils에 두고 테이블과, hasData는 컴포넌트 내부에 위치 시켜도 좋을 것 같습니다.
table/utils에 두지 않는 이유는 페이지네이션을 사용하는 컴포넌트에서도 사용하기 때문입니다.
hasData를 훅으로 분리했던 이유는 이름으로 유추가능한 함수로직들을 한 파일에서 관리하여 복잡도를 높이는 게 싫어서였는데, 함수 한 개라면 파일을 분리하는 편이 오히려 프로젝트를 복잡하게 하는 것 같네요.

@nanafromjeju nanafromjeju changed the title [fix] table 컴포넌트의 데이터가 없는 경우 ui 추가, 훅 분리, body로 컴포넌트를 받을 수 있도록 수정 [Fix] table 컴포넌트의 데이터가 없는 경우 ui 추가, 훅 분리, body로 컴포넌트를 받을 수 있도록 수정 Nov 26, 2024
@github-actions
Copy link

🚀 Storybook이 배포 됐어요! 확인하러 가기 => https://6729e72ee61d8f57ca4790fb-qxnxoitgft.chromatic.com/

Copy link
Contributor

@devdeun devdeun left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

Copy link
Contributor

@ssumanlife ssumanlife left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

@kimpra2989 kimpra2989 temporarily deployed to fix/table-111 - FinalProject_team3_FE PR #114 November 29, 2024 05:56 — with Render Destroyed
@kimpra2989 kimpra2989 merged commit c83da2c into develop Nov 29, 2024
1 check passed
@kimpra2989 kimpra2989 deleted the fix/table-111 branch November 29, 2024 05:57
@github-actions
Copy link

🚀 Storybook이 배포 됐어요! 확인하러 가기 => https://6729e72ee61d8f57ca4790fb-jqmnzocuxe.chromatic.com/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 Refactor 코드 리팩토링

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants