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

[김보민] week11 #368

Conversation

bomin0830
Copy link
Collaborator

@bomin0830 bomin0830 commented Apr 29, 2024

요구사항

netlify
https://sparkling-eclair-f53a5c.netlify.app/Folder

기본

  • 폴더 페이지에 필요한 “폴더 이름 변경”, “폴더 추가”, “폴더 공유”, “폴더 삭제”, “링크 삭제”, “폴더에 추가” 모달을 만들어 주세요.
  • “폴더 공유” 모달의 기능은 클릭시 ‘{호스트 주소}/shared/{currrentFolderId}’ 로 접속 가능한 주소를 공유 또는 클립보드에 복사 가능하게 해주세요.
  • 팀내 2 ~ 3명이 짝을 지어 페어프로그래밍으로 여러 모달을 만들 때 사용할 모달 컴포넌트를 만들어 주세요.
  • 카드에 있는 케밥 버튼을 클릭할 경우 보여야 할 팝오버를 만들어 주세요.

주요 변경사항

스크린샷

멘토에게

  • 폴더 공유 기능과 "폴더에 공유" 모달 기능을 아직 구현하지 못하였습니다
  • 모달 컴포넌트 제작에 대해 더 개선할 부분이 있는지 조언 부탁드립니다.

@bomin0830 bomin0830 requested a review from devym-37 May 1, 2024 02:55
@bomin0830 bomin0830 added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels May 1, 2024
Copy link
Collaborator

@devym-37 devym-37 left a comment

Choose a reason for hiding this comment

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

잘 작성해주셨습니다 👍
말씀해주신 모달 부분은 현재 작성해주신 방법으로도 작성할 수 있을 것 같고, 만약에 하나의 모달만 사용한다면,

{isOpen.share && (
        <ModalLayout
          title="폴더 공유"
          description={name}
          toggleHandler={() => toggleHandler("share")}
        >
          폴더 공유
        </ModalLayout>
      )}
      {isOpen.delete && (
        <ModalLayout
          title="폴더 삭제"
          description={name}
          toggleHandler={() => toggleHandler("delete")}
        >
          <div className="button delete">삭제하기</div>
        </ModalLayout>
      )}
      {isOpen.nameChange && (
        <ModalLayout
          title="폴더 이름 변경"
          toggleHandler={() => toggleHandler("nameChange")}
        >
          <div className="modal-contents">
            <input></input>
            <div className="add button">변경하기</div>
          </div>
        </ModalLayout>
      )}
      
      
      ---
      
      const MODAL_INFO = {
  share: {
    title: "폴더 공유",
    description: name,
    contents: "폴더 공유",
    button: null,
  },
  delete: {
    title: "폴더 삭제",
    description: name,
    contents: null,
    button: <div className="button delete">삭제하기</div>,
  },
  nameChange: {
    title: "폴더 이름 변경",
    description: null,
    contents: (
      <div className="modal-contents">
        <input></input>
        <div className="add button">변경하기</div>
      </div>
    ),
    button: null,
  },
}

<ModalLayout
  title={MODAL_INFO[type].title}
  description={MODAL_INFO[type].description}
  toggleHandler={() => toggleHandler(type)}
>
  {MODAL_INFO[type].contents}
  {MODAL_INFO[type].button}
</ModalLayout>

이렇게도 관리할 수 있을 것 같습니다. 하지만, 만약에 UI가 많이 다르게 되거나 할 경우엔, 보민님이 작성해주신 대로 따로따로 해당 모달을 호출하는 것이 저는 더 나은 방법 같습니다.
고생하셨습니다. 👍

.js pj/utils.js Outdated
}
}

export { addErrorSign, removeErrorSign, checkEmailValid, checkPwdValid, checkError, setEyeOff, setEyeOn };
Copy link
Collaborator

Choose a reason for hiding this comment

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

외부에 사용할 util 함수, 변수 등을 잘 분리해주신 것 같습니다

[state]: !prev[state],
}));
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

혹시 동시에 여러 popup이 open 될 가능성은 없는걸까요 ?

<div className="title-button-wrapper">
<p onClick={() => toggleHandler("share")}>
<img src="images/share.svg" alt="share-button"></img>공유
</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

img 태그는 단일 태그로 작성하실 수 있을 것 같습니다

</ModalLayout>
)}
</>
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

모달 부분에 대해서 말씀해주신 부분은 이 부분일까요 ?

</div>
</footer>
);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

단일 태그 작성시 alt 속성까지 잘 작성해주셨습니다

<>
<div className="sorting-wrapper">
<div className="button-wrapper">
<button onClick={() => onAllClick()}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

<button onClick={() => onAllClick()}>
해당 부분은
이렇게 작성할 수 있습니다

const user = await getData(ApiUrl.sampleUser);
const Folders = await getData(ApiUrl.usersFolders);
const AllLinks = await getData(ApiUrl.usersLinks);

Copy link
Collaborator

Choose a reason for hiding this comment

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

컴포넌트 외부에서 data를 부르시는 이유가 있으실까요 ?

}

useEffect(() => {
const newUrl = `${ApiUrl.usersLinks}?folderId=${selectedId}`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

selectedId 초기값이 undefined인데 folderId가 undefined여도 이상 없는걸까요 ?

src/util/url.js Outdated
sampleUser: `${baseUrl}sample/user`,
usersFolders: `${baseUrl}users/1/folders`,
usersLinks: `${baseUrl}users/1/links`,
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

api url을 따로 관리하신 부분도 좋은 것 같습니다

src/util/util.js Outdated
} else {
return minutes === 1 ? "1 minute ago" : minutes + " minutes ago";
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

urtil/util.js파일명보다는 util폴더 하위이니, 조금 더 명확한 파일명이면 나중에 찾기에도 좋으실 것 같아요. 예를 들어 calculateTime.js 이렇게 될 수도 있을 것 같습니다

@devym-37
Copy link
Collaborator

devym-37 commented May 1, 2024

@bomin0830 보민님 머지가 안되는 것 같은데 확인 해주시면 감사하겠습니다!

@bomin0830 bomin0830 changed the base branch from main to part2-김보민 May 4, 2024 12:40
@bomin0830
Copy link
Collaborator Author

@bomin0830 보민님 머지가 안되는 것 같은데 확인 해주시면 감사하겠습니다!

죄송합니다. main브랜치에 pr올려서 생긴 오류였습니다. 다시 한번만 머지 부탁드리겠습니다.

@devym-37 devym-37 merged commit 4463884 into codeit-bootcamp-frontend:part2-김보민 May 4, 2024
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.

2 participants