-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: 검색 기능을 구현한다. #144
feat: 검색 기능을 구현한다. #144
Conversation
- useDebounceCallback 사용
- 면접 기록 및 지원현황의 검색 컴포넌트 통합
- react query를 사용하여 구현 - 면접기록에 사용하는 데이터 형식 정의 (searchInterviewData) - 지원현황에 사용하는 데이터 형식 정의 (searchApplicantData)
- navbar 클릭 시 url이 바뀜에 따라 검색창이 초기화되는 문제 해결
- 검색 결과에 대한 pagination 적용을 위해 수정
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 searchInterviewData = search?.answers.map((value) => ({ | ||
id: value.id, | ||
title: `[${value.field}] ${value.name}`, | ||
subElements: [ | ||
value.field1.split('"').join(""), | ||
value.field2.split('"').join(""), | ||
`${value.grade.split('"').join("")} ${value.semester | ||
.split('"') | ||
.join("")}`, | ||
], | ||
})); | ||
|
||
const searchApplicantData = search?.answers.map((value) => ({ | ||
id: value.id, | ||
title: `[${value.field}] ${value.name}`, | ||
subElements: [ | ||
value.field1.split('"').join(""), | ||
value.field2.split('"').join(""), | ||
`${value.grade.split('"').join("")} ${value.semester | ||
.split('"') | ||
.join("")}`, | ||
new Date(Number(value.uploadDate)).toLocaleString("ko-KR", { | ||
dateStyle: "short", | ||
}), | ||
], | ||
})); |
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.
하나의 로직으로 만들어볼 수 있을까요??
callback에서 만드는 데이터에서, Date가 추가되는 부분만 다른 것 같은데, 어떻게 재사용 가능하게 분리해 낼 수 있을까요?
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.
수정했습니다... !! 650cacb
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.
좋네용!! 몇개만 고쳐주시면 좋을꺼 같아요!
url={`/applicant/${generation}?type=${type}&order=${order}`} | ||
url={`/applicant/${generation}?search=${searchKeyword}&type=${type}&order=${order}`} |
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.
사실 이것도 좋지만,,, use client라면, 다른 형태 중 하나인 URL search 를 쓰는 것도 추후에 좋을 수도 있어요!
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 searchParams = useSearchParams();
const searchKeyword = searchParams.get("search") || "";
현재 이렇게 검색어를 불러오고 있는데, 제시해주신 방식과 이 방식이 어떤 차이가 있어서 제안해주신 건지 궁금합니다..!
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.
아 get은 충분히 좋지만, 이제 set을 같이하면 좋을것 같았어요! 저렇게 작성을 하면 url에 항상 searchKeyword, type, order을 같이 넣어야 하는 불편함이 있지 않을까 싶었어요!!
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.
const search = searchParams.get("search") || "";
const type = searchParams.get("type") ?? "list";
const order = searchParams.get("order") ?? ORDER_MENU.INTERVIEW[0].type;
const queryParams = { search, type, order };
const createQueryString = useCallback(
(name: string[], value: string[]) => {
const params = new URLSearchParams(searchParams.toString());
name.forEach((el, index) => params.set(el, value[index] || ""));
return params.toString();
},
[searchParams]
);
createQueryString(Object.keys(queryParams), Object.values(queryParams));
그렇다면 이렇게 set하는 함수인 createQueryString
을 만들어서 요 리턴 값을 url
부분에 박아보려고 하는데 어떤가요? 다른 곳에서도 사용될거라서 훅으로 빼볼까 합니다..
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.
굿굿!
- pathname을 다루는 부분에 useTransition 적용 - use-debounce에서 useDebounce 라이브러리로 변경 - Keyword에서 term으로 변수명 통일 - form 태그 삭제
908ffed
to
093ceef
Compare
093ceef
to
376e547
Compare
startTransition(() => { | ||
if (debouncedSearchTerm) { | ||
params.set("search", debouncedSearchTerm); | ||
} else { | ||
params.delete("search"); | ||
} | ||
|
||
replace(`${pathname}?${params.toString()}`); | ||
}); | ||
}, [debouncedSearchTerm, pathname]); |
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.
수고 많으셨습니다!
const searchInterviewData = search?.answers.map((value) => ({ | ||
id: value.id, | ||
title: `[${value.field}] ${value.name}`, | ||
subElements: [ | ||
value.field1.split('"').join(""), | ||
value.field2.split('"').join(""), | ||
`${value.grade.split('"').join("")} ${value.semester | ||
.split('"') | ||
.join("")}`, | ||
], | ||
})); | ||
|
||
const searchApplicantData = search?.answers.map((value) => ({ | ||
id: value.id, | ||
title: `[${value.field}] ${value.name}`, | ||
subElements: [ | ||
value.field1.split('"').join(""), | ||
value.field2.split('"').join(""), | ||
`${value.grade.split('"').join("")} ${value.semester | ||
.split('"') | ||
.join("")}`, | ||
new Date(Number(value.uploadDate)).toLocaleString("ko-KR", { | ||
dateStyle: "short", | ||
}), | ||
], | ||
})); |
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.
동의합니다. 하나의 값으로 뺄 수 있을 것 같아요!
- 쿼리스트링에 파라미터를 get, set 하는 커스텀 훅 생성
주요 변경사항
components/common/Search.tsx
useSearchQuery
(커스텀훅)에서 맞춰주고자 했습니다.리뷰어에게...
늦어져서 죄송합니다...
개선할 부분이 있다면 피드백 주세요
관련 이슈
closes #30
체크리스트
reviewers
설정label
설정milestone
설정