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.

Sign up for GitHub

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

Jump to bottom

Feat/공통 모달 스토어 및 북마크 모달 추가 #49

Merged
merged 16 commits into from
Jun 6, 2024
Merged

Feat/공통 모달 스토어 및 북마크 모달 추가 #49

merged 16 commits into from
Jun 6, 2024

Conversation

rai-95
Copy link
Contributor

@rai-95 rai-95 commented Jun 2, 2024

📖 관련 문서

#47

스크린샷 2024-06-02 오후 5 46 21

✍🏻 변경사항:

  • createPortal을 사용하여 modal layout 구현했습니다

  • modalStore로 모달 관리하도록 추가했습니다

    • modalName으로 모달 관리할 수 있도록 설정했습니다!

  • 북마크 추가 모달 UI 구현

    • textarea 영역이 있는데 따로 컴포넌트로 만들지 textField에 컴포넌트로 area를 추가하는게 좋을지 한번 확인 부탁드려요!

  • 드래그 파일 업로드 hook 추가했습니다

  • 모달 사용

    • modalStore를 사용
const { openModal, isModalOpen, closeModal } = useModalStore();

const handleOpenModal = () => {
  openModal('modalName')
}

const handleCloseModal = () => {
  closeModal('modalName')
}

{isModalOpen('modalName') && <ModalPortal>modal content</ModalPortal>}

🔍 확인할 목록

  • : 스토리북 모달 동작 확인
  • : 사이드바 북마크 모달 확인
  • : 드래그 파일업로드 확인

@rai-95 rai-95 added ✨ feat 구현, 개선 사항에 관련된 내용입니다. 🎨 style UI 관련 이슈입니다. labels Jun 2, 2024
@rai-95 rai-95 requested a review from hoongding June 2, 2024 08:57
@rai-95 rai-95 self-assigned this Jun 2, 2024
@vercel Vercel
Copy link

vercel bot commented Jun 2, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
packit-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 6, 2024 4:13am

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 2, 2024

🚀 Storybook 자동 배포 URL: https://6637bd73cba78215aa6e39fa-ffckwjqcxa.chromatic.com/

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 2, 2024

🚀 Storybook 자동 배포 URL: https://6637bd73cba78215aa6e39fa-mdwgxakyjq.chromatic.com/

hoongding
hoongding reviewed Jun 4, 2024
View reviewed changes
Copy link
Contributor

@hoongding hoongding left a comment

Choose a reason for hiding this comment

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

너무 고생하셨습니다 성은님!!!!
리뷰 확인 부탁드려용 ~! :)

src/components/Sidebar/index.tsx Outdated

const Index = () => {
const pathname = usePathname();
Copy link
Contributor

Choose a reason for hiding this comment

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

p5;
pathName으로 수정 부탁드립니당!

Copy link
Contributor

Choose a reason for hiding this comment

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

헉 다시 보니 usePathname이군요,,!
그래도 변수는 pathName으로 쓰는게 좋을까요,,(?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pathname, pathName 둘다 사용되는거 같더라구요...! 저희 컨벤션에 맞춰서 카멜케이스로 맞춰도 될거같아요!

src/components/common/Modal/ModalLayout.tsx
return (
<div ref={modalRef} className='fixed top-0 left-0 bottom-0 right-0' onClick={handleCloseModal}>
<div
className={cn([
Copy link
Contributor

Choose a reason for hiding this comment

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

p4;
혹시 성은님 이쪽에서 cn 쓰신 이유 있을까용??
한번에 className에 적는건 어떨까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

클래스가 길어지면서 가독성이 좋아 보이지 않아 보여 줄바꿈하기 위해서 작성했었습니다!

작성한거 보니깐 기준없이 나눠진거 같기는 하네요ㅠ 큰 이유는 없어서 변경해도 상관은없을거같아요!

src/components/common/Modal/ModalPortal.tsx Outdated

if (!isModalOpen) return null;

const el = document.getElementById('modal');
Copy link
Contributor

Choose a reason for hiding this comment

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

p4;
el 네이밍 말구 modalElement 이런식으로 구체화해도 좋을 것 같습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네! 명확하게 어떤 요소인지 적는게 좋을거 같네요!

반영해놓겠습니다!

src/components/common/Modal/ui/BookmarkModal.tsx
@@ -0,0 +1,141 @@
'use client';
Copy link
Contributor

Choose a reason for hiding this comment

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

그러면 추후 다른 Modal 이 생길경우 이쪽에 Modal을 생성하는 것일까용??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네! 처음 생각으로는 모달도 한곳에서 관리하면 좋을거 같아서 디렉토리를 만들었습니다!

아직 구조화가 된거 같지는 않아서 좋은 의견 있으시면 말씀해주세요!!

src/components/common/Modal/ui/BookmarkModal.tsx Outdated
<ModalPortal>
<div className='w-[432px]'>
<div className='mb-24'>
<h1 className='text-text heading-2xl-bd '>북마크 추가</h1>
Copy link
Contributor

Choose a reason for hiding this comment

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

p999;
맨 뒤 공백하나가 있습니당!!

src/stores/modalStore.ts
@@ -0,0 +1,28 @@
// src/stores/modalStore.js
import { create } from 'zustand';
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻 👍🏻 👍🏻 👍🏻 👍🏻 👍🏻

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 6, 2024

🚀 Storybook 자동 배포 URL: https://6637bd73cba78215aa6e39fa-ylmodxqoxz.chromatic.com/

@rai-95 rai-95 merged commit 73d0315 into main Jun 6, 2024
4 of 5 checks passed
@bbung95 bbung95 assigned bbung95 and unassigned rai-95 Jun 12, 2024
@hoongding hoongding deleted the feat/modal branch July 13, 2024 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoongding hoongding hoongding left review comments

Assignees

@bbung95 bbung95

Labels
✨ feat 구현, 개선 사항에 관련된 내용입니다. 🎨 style UI 관련 이슈입니다.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rai-95 @hoongding @bbung95