-
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
[Feat] 지원자페이지 구현 #1337
The head ref may contain hidden characters: "Feat/1254-\uC9C0\uC6D0\uC790\uD398\uC774\uC9C0-\uAD6C\uD604"
[Feat] 지원자페이지 구현 #1337
Conversation
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.
작업 내용 확인했습니다. 개선할 수 있을 것 같은 부분이 몇 군데 있어 보여서 댓글 남겨두었는데, 일단은 작업 내용을 테섭에 올리는게 우선일 것 같아 approve 합니다! mockInstance 제거하면서 댓글도 같이 확인 부탁드려요
labelId='demo-multiple-checkbox-label' | ||
id='demo-multiple-checkbox' |
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.
아이디에 demo 붙어 있는데, 임시로 조치한 부분이 아니면 제거하는 것이 좋을 것 같습니다.
expandComponent: React.ReactNode; | ||
} | ||
|
||
const ExpandableTableRow: React.FC<ExpandableTableRowProps> = ({ |
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.
FC에는 기본적으로 optional children props가 있습니다. 따라서 위에 ExpandableTableRowProps에서 children을 재정의할 필요는 없었을 것 같아요. (옵셔널 -> 필수로 바꾸려던게 아니라면)
근데 위에 링크한 글처럼, React.FC에는 이것 말고도 여러 문제가 있어서 전반적으로 사용하지 말자는 것 같더라구요.
특별한 이유가 없다면 사용하지 않는 것이 좋을 것 같습니다.
...otherProps | ||
}) => { | ||
const [isExpanded, setIsExpanded] = useState(false); | ||
const childrenCount = React.Children.count(children); |
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.
Children 사용하셨는데 https://react.dev/reference/react/Children 공식문서 보니까 레거시로 구분되어 있습니다. children을 만드는 부분을 보니 배열에 map을 돌리고 있는 것 같은데 React.Children.count 대신 props로 children 개수를 전달하는 방식으로 바꿀 수도 있을 것 같습니다.
setAnswers( | ||
recruitUserData.reduce((acc, recruit) => { | ||
recruit.form.forEach((formItem) => { | ||
if (formItem.inputType !== 'TEXT') { |
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.
filter로 써도 좋을 것 같아요
<div className={styles.info}> | ||
{(content?.toString() || '').slice(0, maxLen)}... | ||
</div> | ||
{/* <div className={`${styles.hoverInfo}`}>{content}</div> */} |
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.
고생하셨습니다!!
📌 개요
💻 작업사항
기본화면
상세보기
가로스크롤
객관식필터
멀티선택
객관식필터
✅ 변경로직