-
Notifications
You must be signed in to change notification settings - Fork 8
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
드래그로 카테고리 순서 변경 가능하도록 변경 #1045
base: dev/fe
Are you sure you want to change the base?
드래그로 카테고리 순서 변경 가능하도록 변경 #1045
Conversation
…de-zap into feat/966-add-category-ordinal
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.
잘 작동하는거 같네요! 고생하셨습니다.
저는 date로 ID를 만들어도 큰 이슈 없을 것 같아요. 어차피 프론트에서만 잠시 쓰는 임시 아이디이니까요
자세한 PR description도 고마워요!
const EXPLAIN = [ | ||
{ title: 'ZAP하게 저장', description: '자주 쓰는 나의 코드를 간편하게 저장하세요' }, | ||
{ title: 'ZAP하게 관리', description: '직관적인 분류 시스템으로 체계적으로 관리하세요' }, | ||
{ title: 'ZAP하게 검색', description: '필요한 나의 코드를 빠르게 찾아 사용하세요' }, | ||
]; | ||
|
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 isEqualCategoryList = (arr1: Category[], arr2: Category[]): boolean => { | ||
if (arr1.length !== arr2.length) { | ||
return false; | ||
} | ||
|
||
return arr1.every((category, index) => { | ||
const category2 = arr2[index]; | ||
|
||
return category.id === category2.id && category.name === category2.name && category.ordinal === category2.ordinal; | ||
}); | ||
}; |
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.
id가 같은 경우가 있을 수도 있나요?!
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 handleAddCategory = () => { | ||
const id = Date.now(); | ||
|
||
const ordinal = editedCategoryList.length + 1; | ||
|
||
setEditedCategoryList((prev) => [...prev, { id, name: '', ordinal }]); | ||
setEditingCategoryId(id); | ||
}; |
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.
수고하셨습니다 헤인~!
꼼꼼하게 작업해주신 덕분에 코드 잘 확인했습니다! CategoryEditModal
의 서브 컴포넌트 분리와 상태 관리 방식이 깔끔하네요. 새로운 카테고리 생성 시 ID 관리 부분도 고민이 잘 반영된 것 같아 좋습니다. 현재 3번 방안(Date.now())으로 처리하신 것도 적절해 보이는데, 만약 프로젝트 확장성을 고려한다면 uuid로의 전환도 이후에 충분히 고려할 수 있겠네요. 전 지금도 너무 좋습니다!!
너무 수고 많으셨습니다! 😊💪
헤인 확인하고 머지하면 될거 같아용~ |
⚡️ 관련 이슈
📍주요 변경 사항
1. 카테고리에 순서(ordinal)이 생겼습니다. 이에 따라 Category 인터페이스가 아래와 같이 변경되었습니다.
2. 카테고리 편집 모달에서 관리하는 상태가 아래와 같이 변경되었습니다.
updateCategories
에 모든 카테고리를 다시 넘겨줘야 하기 때문입니다.3. 카테고리 편집 모달에서 사용하는 서브 컴포넌트들 및
useCategoryEditModal
을 파일 분리하였습니다.CategoryEditModal.ts
파일이 너무 길어지고, 한 눈에 CategoryEditModal JSX 부분이 들어오지 않는다고 생각했기 때문입니다.CategoryEditModal
폴더 내부에 그냥 생성하였습니다.CategoryEditModal
내부에서 또 다시components
파일을 만드는 것은 과도하게 depth가 깊어진다고 생각했기 때문입니다. 이외에 더 좋은 위치나 아이디어 있으시다면 답변달아주세요!!4. 네이밍 컨벤션에 따라
categories
를 사용하던 용어를categoryList
로 변경하였습니다.🎸기타
새로운 카테고리 생성시 id 문제에 대한 고민을 나누고자 길게 서술합니다.
현재 새로운 카테고리를 생성하면 클라이언트에서 임의로 id를 생성하여 사용하고 있습니다. (id가 필요한 이유는 새로 만든 카테고리를 삭제/편집하기 위해서입니다.) 그동안은 순서 변경이 없었기 때문에 맨 마지막 카테고리를 가장 마지막에 생성된(id가 큰) 카테고리로 간주하고, 마지막 카테고리 id를 기반으로 임의의 id를 생성하였습니다.
그런데 이제 순서가 변경 가능해져서 맨 마지막 카테고리를 기반으로 id를 생성할 수 없어졌습니다. 현재는 임의로
마지막 카테고리 id + 모든 카테고리의 길이+1
로 id를 생성하는데, 이 로직은 중복된 id가 생성될 가능성이 존재합니다.따라서 해당 로직을 어떻게 변경하면 좋을지 고민했고, 제가 고려했던 방안은 다음과 같습니다.
(1)
id
가 아닌tempId
와 같은 다른 키를 사용한다. => 이 방법을 사용할 경우 새로운 카테고리의 경우 인터페이스를 별개로 만들어야 하고, 타입 관리가 조금 까다로워질 수 있다는 단점이 있습니다. 장점은 새로운 카테고리인지 아닌지 빠르게 검증 가능하고, 기존과 중복된 id 생성에 대한 걱정이 없습니다.(2)
id
를 숫자가 아닌 uuid(문자열)로 사용한다. => 이 방법을 사용할 경우 동일한 카테고리 인터페이스를 사용할 순 있지만, Category의 id 를 number | string 으로 타입을 유니온으로 관리하게 되면 이 또한 까다로워질 수 있다는 단점이 있습니다.(3)
id
를Date.now()
로 생성한 number로 사용한다. => 이 방법을 사용할 경우 uuid를 사용하는 것과 비슷하지만 number로 관리할 수 있다는 장점이 있습니다.따라서 일단 3번 방안으로 구현해두었습니다. 혹시 더 좋은 아이디어 있으시면 코멘트 달아주세요!!
🍗 PR 첫 리뷰 마감 기한
01/26 23:59
ps. 작업이 너무 늦어져서 죄송합니다🥲🙏 PR 마감 기한은 일부로 넉넉하게 해놨는데 각자 편한 시간에 리뷰해주시면 바로바로 반영해서 올릴게요!