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

[Feat/Fix] 모달 레이아웃 & 뷰 퍼블리싱 #316

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

cindy-chaewon
Copy link
Contributor

@cindy-chaewon cindy-chaewon commented Nov 5, 2024

해당 이슈 번호

closed #281


체크리스트

  • 🔀 PR 제목의 형식을 잘 작성했나요? e.g. [feat] PR을 등록한다.
  • 💯 테스트는 잘 통과했나요?
  • 🏗️ 빌드는 성공했나요?
  • 🧹 불필요한 코드는 제거했나요?
  • ✅ 컨벤션을 지켰나요?
  • 💭 이슈는 등록했나요?
  • 🏷️ 라벨은 등록했나요?
  • 💻 git rebase를 사용했나요?
  • 🙇‍♂️ 리뷰어를 지정했나요?
  • ✨ 저는 공통 모달 컴포넌트 (ModalFooter, ModalBody, ModalWrapper, ModalHeader)은 common으로, 나머지는 shared로 분리했습니다.

📌 내가 알게 된 부분


💎 PR Point

우선 기존의 모달 뷰 였던 워크스페이스 생성하기, 타임블록 생성하기 뷰는 자세하게 안 보셔도 됩니다!! (안 봐도 됨...ㅎㅎ)( 기존의 뷰 그대로에 모달 합성 컴포넌트만 적용 함)
그리고 새로 만든 태그 모달들(ActivityTagModal, MemberTagModal), 초대 모달(InviteModal), 휴지통(DeletedModal) 모달! --> 사실 이 부분도요 대충 쓱 훑어 주시기만 해도 됩니다! 말그대로 단순! 뷰 퍼블리싱이었고 피그마에서 이 세 모달들을 보시면 아시겠지만 아이템이랑 input 위치 생김새가 굉장히 유사해요...!!

그래서 어떤 부분을 중심적으로 보시고 리뷰를 남겨주시면 좋냐...!! ==> 바로 모달 대격변한 모달 레이아웃 입니다!!
이번에 바뀐 모달은 아래 사진과 같이 세 부분으로 나누어져 있고, 모달 헤더, 모달 푸터 같은 경우는 아이콘, 텍스트, 버튼 안의 텍스트 내용만 달라지고 컴포넌트들의 구성요소들을 그대로 인것을 볼 수 있습니다!
image

이번에 모달 레이아웃을 바꾸면서 제일 큰 목표가 재사용성 있는 뷰 만들기 였는데요,
저는 이 반복되는 모달 레이아웃을 재사용성 있게 사용하고자 합성 컴포넌트 를 도입하였습니다.

  1. 모달 바디
import { ReactNode } from 'react';

import Flex from '@/common/component/Flex/Flex';

interface ModalBodyProps {
  children: ReactNode;
}

const ModalBody = ({ children }: ModalBodyProps) => (
  <Flex styles={{ direction: 'column', justify: 'flex-start', align: 'center', paddingTop: '2rem', grow: '1' }}>
    {children}
  </Flex>
);

export default ModalBody;

제일 별 거 없습니다. 모달 바디안에 children 으로 컨텐츠 뷰를 주입하는 형식이기에 틀만 맞춰줬습니다.

  1. 모달 헤더
    image
  • icon
  • title
  • infoText

getHeaderContent 함수??

  • contentType, step, totalSteps와 같은 매개변수를 받아 해당 모달의 유형과 진행 단계에 따라 icon, title, infoText 데이터를 반환하는 유틸 함수입니다.
  • ModalHeader 컴포넌트는 getHeaderContent에서 반환된 데이터를 사용해 UI를 구성하게됩니다!
  • switch 문을 사용해 각 contentType에 맞는 데이터를 설정하고, 조건문을 통해 단계에 따른 텍스트와 아이콘을 동적으로 구성하였습니다.

<예외 구현 포인트>

switch (contentType) {
    case 'create-workspace':
      return {
        icon:
          step === totalSteps ? (
            <SuccessIcon width={40} height={40} />
          ) : step && totalSteps ? (
            `${step}/${totalSteps}`
          ) : null,
        title:
          step === 1 || step === 2
            ? '새로운 워크스페이스 생성하기'
            : step === 3
              ? '동아리 프로필 이미지 등록'
              : '워크스페이스 생성완료',
        infoText:
          step === 1
            ? '워크스페이스의 이름을 입력해주세요'
            : step === 2
              ? '팀 카테고리를 선택해주세요'
              : step === 3
                ? '우리 동아리 프로필에 표시할 이미지를 등록해주세요'
                : '이제 워크스페이스를 사용할 수 있습니다.',
      };

워크 스페이스 생성 보시면 아이콘이 "완료" 모달 제외하고는 1/4 의 스텝 텍스트 표시입니다. 이를 예외처리 해주었습니다.

  1. 모달 푸터
    getFooterContent 함수??
  • contentType, step, buttonClick, closeModal, isButtonActive 등의 매개변수를 받아 모달의 진행 상황에 맞는 버튼 데이터를 반환해주는 유틸 함수입니다.
  • 각 모달 유형(contentType)에 대해 switch 문을 사용하여 버튼 배열을 설정하고, 조건에 맞는 버튼만 반환해줍니다!
  • ModalFooter 컴포넌트는 getFooterContent에서 반환된 버튼의 텍스트(text), 클릭 이벤트(onClick), 스타일(variant), 활성화 여부(disabled)를 포함한 객체 배열 데이터를 기반으로 Button 컴포넌트를 렌더링합니다!

구현 예시

switch (contentType) {
    case 'create-workspace':
      return [
        step >= 3 ? { text: '건너뛰기', onClick: buttonClick, variant: 'outline' } : false,
        {
          text: step === 4 ? '확인' : '다음으로',
          onClick: buttonClick,
          variant: 'primary',
          disabled: !isButtonActive,
        },
      ].filter(Boolean) as FooterButton[];

워크 스페이스 생성하기 모달에서 스텝 3전, 완료 부분은 버튼이 "한개", 그 외는 두개
--> step >= 3이 아닌 경우, '건너뛰기' 버튼이 false로 설정
--> step === 4일 때는 '확인' 버튼으로, 그 외에는 '다음으로' 버튼으로 설정

  1. 적용
const WorkSpaceName = ({ isVisible }: WorkSpaceNameProps) => {
  const [inputValue, setInputValue] = useState('');
  const { setFormData, nextStep } = useWorkSpaceContext();

  const handleNext = () => {
    setFormData({ name: inputValue });
    nextStep();
  };

  const isButtonActive = inputValue.trim().length > 0;

  return (
    <>
      <Modal.Header step={1} totalSteps={4} />
      <Modal.Body>
        <Input
          placeholder="팀, 동아리, 조직 이름 입력"
          value={inputValue}
          onChange={handleInputChange}
          css={inputWrapperStyle}
        />
      </Modal.Body>
      <Modal.Footer step={1} buttonClick={handleNext} isButtonActive={isButtonActive} />
    </>
  );
};

export default WorkSpaceName;

요런 식으로 합성 컴포넌트 조립해서 사용하면 됩니다!!

📌스크린샷 (선택)

모달 UI 스크린샷

Copy link

github-actions bot commented Nov 5, 2024

🚀 Storybook 확인하기 🚀

Copy link
Contributor

@wuzoo wuzoo left a comment

Choose a reason for hiding this comment

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

고생하셨어요 !! 다음부터는 작업량 최대한 분리해서 pr 올리기 ....

리뷰 확인해주세용 ~

Copy link
Contributor

Choose a reason for hiding this comment

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

기존 common 모달은 그대로 두고, 채원님이 지금처럼 합성 컴포넌트로 구현한 모달을 shared에 놓고 활용하는게 맞는 것 같아요 !
특정 기능에 따른 모달 컨텐츠를 제공하고, 혹은 각 단계의 step을 표현해주기도 하니, common에 있을 친구는 아닌 것 같습니다 !

Comment on lines +10 to +32
case 'create-workspace':
return {
icon:
step === totalSteps ? (
<SuccessIcon width={40} height={40} />
) : step && totalSteps ? (
`${step}/${totalSteps}`
) : null,
title:
step === 1 || step === 2
? '새로운 워크스페이스 생성하기'
: step === 3
? '동아리 프로필 이미지 등록'
: '워크스페이스 생성완료',
infoText:
step === 1
? '워크스페이스의 이름을 입력해주세요'
: step === 2
? '팀 카테고리를 선택해주세요'
: step === 3
? '우리 동아리 프로필에 표시할 이미지를 등록해주세요'
: '이제 워크스페이스를 사용할 수 있습니다.',
};
Copy link
Contributor

Choose a reason for hiding this comment

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

step까지 유틸로 분리해서 처리하는 것보다는 저번에 제가 말씀드린 것처럼 step에 따라 올바른 단계의 자식 요소를 렌더링하는 funnel 컨테이너 컴포넌트를 만들고, 해당 컴포넌트 안에서 각 모달들한테 step prop을 넘겨주면서 그냥 그 컴포넌트 안에서 처리해도 될 것 같아요.

이거 퍼널 구조 레퍼런스 확인해보시면 좋을 것 같습니다. 토스에서 제공하는 라이브러리인 use-funnel도 있어용

<Funnel>
  <Funnel.Step step={1}>
    < 번째 모달 />
  </Funnel.Step>
  <Funnel.Step step={2}>
    < 번째 모달 />
  </Funnel.Step>
</Funnel>

식으로 말이죠. 즉 step을 유틸에서 처리할 필요 없이, 그냥 컴포넌트에게 step을 넘겨서 알아서 처리하게 말이죠 !

추가적으로 그렇다면 아래의 create-block이나 delete와 같은 것들은, 그냥 constant 파일에서 객체 형태로 빼줄 수도 있을 것 같아요

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. 각 모달에 대한 뷰만 퍼블리싱 해놓는다
  2. 단계에 대한 로직은 funnel 구조에게 그냥 위임해버린다
  3. 단계에 따른 icon 같은 것들은 그냥 상수 파일로 분리시킬 수 있다.

정도로 요약 가능하겠네요 !

Copy link
Member

@namdaeun namdaeun left a comment

Choose a reason for hiding this comment

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

합성 컴포넌트 구조로 모달 구현한 거 너무너무 좋네요 고생 많았슴니다 채원이 !!🚀👍🏻

const { icon, title, infoText } = getHeaderContent(contentType!, step, totalSteps);

return (
<Flex styles={{ direction: 'row', justify: 'flex-start', align: 'center', gap: '1.2rem' }}>
Copy link
Member

Choose a reason for hiding this comment

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

modalHeader니까 header 태그 사용해주면 어떨까요 ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋습니다!!

@@ -23,7 +19,6 @@ interface BlockModalProps {

const BlockModal = ({ isVisible }: BlockModalProps) => {
const [selectedIcon, setSelectedIcon] = useState<number>(-1);
Copy link
Member

Choose a reason for hiding this comment

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

자동으로 타입 유추가 되니까 useState의 경우 타입 명시를 안해도 괜찮을 것 같아요 !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵! 반영완료 했습니다!

Comment on lines 109 to 110
<Modal.Body>
<Flex tag={'section'} css={flexStyle}>
Copy link
Member

Choose a reason for hiding this comment

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

Modal.Body 컴포넌트 자체를 section 태그로 지정하는 건 어떠신가요 ?
section태그 안에서 div로 레이아웃을 조정해주는 게 더 자연스러울 것 같다고 생각했어요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋은거 같아용! section 태그로 지정완료 했습니다!

저장
</Button>
</Flex>
<div className="scroll" css={scrollStyle}>
Copy link
Member

Choose a reason for hiding this comment

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

여기 className를 따로 지정한 이유가 있을까요 ??

그리고 commonStyle에 scrollStyle이 이미 존재하는데 해당 스타일을 재사용할 수 있는지 궁금합니다 !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

앗 스크롤 스타일 때문에 className 추가해놓았는데, 사용하지 않았네용.. 지웠습니다!!
재사용 가능해서 적용완료 했습니다!

const modalData = useModalData();

const itemType = modalData?.itemType;
const title = itemType ? DELETED_TITLE[itemType.toUpperCase() as keyof typeof DELETED_TITLE] : '';
Copy link
Member

Choose a reason for hiding this comment

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

const title = itemType && DELETED_TITLE[itemType.toUpperCase() as keyof typeof DELETED_TITLE]

else일 떄 빈 문자열을 리턴하는 것보다 and 연산자를 통해 필요한 경우만 값을 지정하는 건 어떨까요 ? 에러가 날런가요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋습니다! 에러 안나서 반영완료 했습니다!

onDelete: () => void;
}

const MemberItem = ({ title, onDelete }: MemberItemProps) => {
Copy link
Member

Choose a reason for hiding this comment

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

헷갈릴 수 있다고 생각해서 폴더명과 파일명을 통일하는 게 좋을 것 같다고 생각해요!!
InviteModal 안의 컴포넌트들이니 Member 혹은 MemberItem으로 이름 지어도 Invite에 대한 의미는 전달될 것 같습니다

아니면 InviteItem이 혹시 더 생길 가능성이 있다면 InviteItem에 Member 폴더를 생성해서 그 안에 요 파일을 배치할 수도 있겠네요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

다은님 의견 동의합니다..! 조금 헷갈릴 수 있는거 같아요! 파일명 변경 완료했습니다!

Copy link
Contributor

@Bowoon1216 Bowoon1216 left a comment

Choose a reason for hiding this comment

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

모달의 대격변이네요!! 피알도 정성스럽고 넘 수고하셨어요 💯

@@ -72,7 +72,7 @@ export const childrenStyle = css({
display: 'flex',
alignItems: 'center',

padding: '0.4rem',
//padding: '0.4rem',
Copy link
Contributor

Choose a reason for hiding this comment

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

요건 사용하지 않는거면 삭제해줘도 좋을 것 같네용 !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵!

Comment on lines +6 to +10
export const Modal = Object.assign(ModalWrapper, {
Header: ModalHeader,
Body: ModalBody,
Footer: ModalFooter,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

이런 메소드가 있었다니 신기하네요 !! 알아갑니다 ❤️

Copy link
Contributor

@rtttr1 rtttr1 left a comment

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.

사라진 친구에게 많은 정성을 쏟아 주셨군요..!

<div className="scroll" css={scrollStyle}>
{files.map((file) => (
<BlockItem
key={file.name}
Copy link
Contributor

Choose a reason for hiding this comment

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

파일 이름은 중복될수도 있지 않을까요?!?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵...! 그래서 key={${file.name}-${file.lastModified}} 이렇게 변경했습니다!

앗 그리고 타임픽커 날짜 받아오는 기능 추가 완료했으니 한번 확인 부탁드립니당1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

모달 레이아웃 & 적용
5 participants