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

Feature/#235 스터디 수정 모달 구현 #246

Merged
merged 2 commits into from
Jun 8, 2024

Conversation

yeonddori
Copy link
Collaborator

관련 이슈

작업 요약

  • 스터디 수정 모달을 생성 모달과 합쳐서 구현하였습니다.

작업 상세 설명

  • api 연결을 위해서 teamId와 studyId를 추가했습니다.

리뷰 요구 사항

  • 리뷰 예상 시간: 5분

미리 보기

2024-06-07190331-ezgif com-video-to-gif-converter

@yeonddori yeonddori added 🎨 디자인 CSS 등 사용자 UI 디자인 변경 ✨ 기능 개발 새로운 기능을 구현하는 데 필요한 작업 또는 변경 사항 labels Jun 7, 2024
@yeonddori yeonddori self-assigned this Jun 7, 2024
Copy link
Member

@pipisebastian pipisebastian left a comment

Choose a reason for hiding this comment

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

아직 리뷰 덜 했습니당!

@@ -30,6 +31,8 @@ const Page = () => {
const [assetArray, setAssetArray] = useState<StudyAssetCardProps[]>([]);
const [assetLength, setAssetLength] = useState<number>(0);

const [isCreateModalOpen, setIsCreateModalOpen] = useState<boolean>(false);
Copy link
Member

Choose a reason for hiding this comment

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

isCreateModalOpen -> isCreateStudyModalOpen로 study 붙히는건 어떨까요? 좀더 명확하면 좋을 것 같습니당!

</Flex>
</Flex>
<StudyModal teamId={params.teamId} isOpen={isCreateModalOpen} setIsModalOpen={setIsCreateModalOpen} />
</>
Copy link
Member

Choose a reason for hiding this comment

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

이건 배포 이후에 해도 될 것 같은데,

개인적으로 스터디 탭 버튼 + 스터디 카드 + 네비게이션 + 스터디 모달까지
해당 team page에서 말고, 아예 파일 빼서 하는것도 좋을 것 같습니다! 지금 봐서는 teamId만 props로 넘겨주면 될 것 같아서요!

Copy link
Member

Choose a reason for hiding this comment

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

그리고 저도 team 페이지 작업하고 있었기 때문에,, 조금 충돌이 날지두요 캬햐! 그치만 딱히 중요한건 안 겹쳐서 괜찮을 것 같습니당!

@@ -0,0 +1,6 @@
export interface StudyModalProps {
teamId?: number;
Copy link
Member

Choose a reason for hiding this comment

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

혹시 teamId가 옵셔널인 이유가 있을까요? 스터디 생성/수정 모두 teamId는 들어가야하는거 아녔나용??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

생성에만 teamId가 들어가고, 나머지는 모두 studyId만 필요로 하길래 옵션으로 걸어놨습니다!

onClose={() => setIsModalOpen(false)}
title="스터디 생성"
onClose={onClose}
title={isEditMode ? '스터디 수정' : '스터디 생성'}
Copy link
Member

Choose a reason for hiding this comment

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

`스터디 ${isEditMode ? '수정' : '생성'}` 

요렇게도 할 수 있을 것 같아요!

- isCreateModalOpen -> isCreateStudyModalOpen
- title '스터디' 중복 따로 빼서 할당

- #235
@yeonddori yeonddori requested a review from pipisebastian June 8, 2024 05:04
Copy link
Member

@pipisebastian pipisebastian left a comment

Choose a reason for hiding this comment

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

나중에 api 연결하면서 수정해야할 것들이 조금 있을 것 같아요!
그치만 일단 수정 모달 구현이니까.. 나중에 api 연결할때 바꾸시죵..홋홋홋...!

@@ -11,7 +11,7 @@ import Selector from '@/components/Selector';
import StyledDatePicker from '@/components/StyledDatePicker';
import CROP from '@/constants/crop';

import { CreateStudyModalProps } from './types';
import { StudyModalProps } from './types';
Copy link
Member

Choose a reason for hiding this comment

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

지금 containers/study/study/Modal 폴더안에 비어있는 DeleteStudyModal, TerminateStudyModal 이 있는 것 같습니당!

@@ -21,7 +21,8 @@ const AlertContent = ({ message }: { message: string }) => {
);
};

const CreateStudyModal = ({ isOpen, setIsModalOpen }: CreateStudyModalProps) => {
const StudyModal = ({ teamId, studyId, isOpen, setIsModalOpen }: StudyModalProps) => {
const isEditMode = Boolean(studyId);
Copy link
Member

Choose a reason for hiding this comment

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

스터디 수정 모달에서는, 맨처음에 원래 스터디 정보가 입력되서 나와야할 것 같아요!
그래서 스터디 정보 get api가 필요한데, 이 모달창이 어차피 스터디 페이지 -> 수정 버튼 눌렀을때 뜨는 모달이니까,
스터디 페이지에서 먼저 스터디 정보 get api가 쓰일거란 말이죠! 그래서 그 정보를 모달 props로 그대로 가져오는건 어떨까요? 사실 팀 페이지에서 해당 방식으로 사용했습니다! (팀 페이지 풀리퀘)

그렇게 되면 studyId말고 study 정보가 있는지 없는지로 구별하면 될 것 같아요!

Copy link
Member

Choose a reason for hiding this comment

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

만약 sutdy 정보 props명이 studyInfo라고 한다면,
스터디 생성 모달에서는 studyInfo가 어차피 null로 들어갈거니까, 지금 isEditModestudyInfo로 바꾸면 조건문 다 똑같이 동작할 것 같습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오오 감사합니다! 참고해서 api 연결하겠습니다!

Copy link
Collaborator

@llddang llddang left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!!

@yeonddori yeonddori merged commit 908603c into develop Jun 8, 2024
1 check passed
@yeonddori yeonddori deleted the Feature/#235-스터디_수정_모달_구현 branch June 8, 2024 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎨 디자인 CSS 등 사용자 UI 디자인 변경 ✨ 기능 개발 새로운 기능을 구현하는 데 필요한 작업 또는 변경 사항
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[FEAT] 스터디 수정 모달 구현
3 participants