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

모임 개설 대상 파트 Chip 으로 변경 #914

Merged
merged 8 commits into from
Oct 9, 2024
Merged

모임 개설 대상 파트 Chip 으로 변경 #914

merged 8 commits into from
Oct 9, 2024

Conversation

j-nary
Copy link
Member

@j-nary j-nary commented Oct 7, 2024

🚩 관련 이슈

📋 작업 내용

  • 모임 개설 대상 파트 Chip 으로 변경
  • Form 데이터 연동 처리
  • 전체파트 클릭 로직 구현 ( 개별 옵션 해제 시 전체 옵션도 해제, 모든 개별 옵션 선택 시 전체 옵션도 활성화 )
  • 반응형 작업

📌 PR Point

  • 무슨 이유로 어떻게 코드를 변경했는지
  • 어떤 위험이나 우려가 발견되었는지
  • 어떤 부분에 리뷰어가 집중해야 하는지
  • 개발하면서 어떤 점이 궁금했는지

📸 스크린샷

2024-10-07.11.27.32.mov

Select 로 구현했을 때와 form에 넘어가는 데이터 구조 동일하게 유지
image

@j-nary j-nary self-assigned this Oct 7, 2024
@j-nary j-nary requested review from borimong and ocahs9 as code owners October 7, 2024 14:30
Copy link

height bot commented Oct 7, 2024

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@@ -135,8 +135,6 @@ export default function PostPage() {
!!comment
);

console.log({ comments });

Copy link
Contributor

Choose a reason for hiding this comment

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

콘솔 제거 고마워~!

@@ -18,12 +18,11 @@ export const generationOptions = [
];

export const parts = [
Copy link
Contributor

Choose a reason for hiding this comment

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

요 options 의 part 는 여기에서만 쓰이는 아이일까? 다른 곳에서도 쓰인다면 그곳에서도 잘 동작하는지 확인이 필요할 거 같애~!

Copy link
Member Author

Choose a reason for hiding this comment

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

edit 이랑 presentation 쪽에서만 쓰여!
edit 쪽 실행시켜봤는데 모임 수정 잘 되는 거 확인했오요!

Comment on lines 20 to 55
const handleClick = (selectedOption: Option) => {
let updatedParts = [...selectedParts];

// 'all' 옵션을 클릭했을 때 처리
if (selectedOption.value === 'all') {
if (selectedParts.some(part => part.value === 'all')) {
// 전체 옵션이 이미 선택되어 있으면 해제
updatedParts = [];
} else {
// 전체 옵션을 선택하면 모든 부분을 선택
updatedParts = parts;
}
} else {
// 개별 옵션을 선택할 때
if (selectedParts.some(part => part.value === selectedOption.value)) {
// 이미 선택된 항목이면 해제
updatedParts = updatedParts.filter(part => part.value !== selectedOption.value);
} else {
// 선택되지 않은 항목이면 추가
updatedParts.push(selectedOption);
}

// 개별 옵션 해제 시 전체 옵션도 해제
if (updatedParts.some(part => part.value === 'all') && updatedParts.length < parts.length) {
updatedParts = updatedParts.filter(part => part.value !== 'all');
}

// 모든 개별 파트가 선택되었으면 'all' 옵션도 활성화
if (updatedParts.length === parts.length - 1) {
updatedParts.push(parts[0]);
}
}

setSelectedParts(updatedParts);
onChange(updatedParts); // 선택된 파트의 value만 전달
};
Copy link
Contributor

Choose a reason for hiding this comment

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

코드가 조금 verbose 한 면이 있어서, 리팩토링을 해보면 좋을 것 같아! 지금 진이는, value 라는 사용자 입력 값을 useEffect 를 이용해서 지켜보면서, setState 를 해주고 있는데, 관점을 바꿔서 사용자 이벤트를 기반으로 리팩토링해보면 좋지 않을까 하는 생각이야~

Copy link
Member Author

Choose a reason for hiding this comment

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

좋은 피드백 고마오 !! 코드리뷰 반영했는데 한 번 확인해줄 수 있어 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

훨씬 깔끔해진 거 같애! 충돌만 잡고 머지해보자~!

@j-nary j-nary merged commit d185a08 into develop Oct 9, 2024
1 check passed
ocahs9 added a commit that referenced this pull request Oct 13, 2024
ocahs9 added a commit that referenced this pull request Oct 13, 2024
* Revert "모임 개설 대상 파트 Chip 으로 변경 (#914)"

This reverts commit d185a08.

* Revert "Input 높이 및 모집 대상 chip 변경 (#924)"

This reverts commit 02ee18e.
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.

모집 대상 파트 chip 구현
2 participants