Skip to content

Feat/dialog 컴포넌트 추가 #31

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
Merged
hongggyelim merged 10 commits into from
Aug 31, 2025
Merged

Feat/dialog 컴포넌트 추가 #31

hongggyelim merged 10 commits into from
Aug 31, 2025

Conversation

@hongggyelim
Copy link
Collaborator

@hongggyelim hongggyelim commented Jul 26, 2025

package/ui dialog 컴포넌트 추가

  • 합성 컴포넌트로 구현
  • portal 사용
  • 모바일에서 width : 90% (모달과 배경이 형제 노드여서 배경 padding이 모달에 영향을 미치지 않음)

@changeset-bot
Copy link

changeset-bot bot commented Jul 26, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: 1e7ed47

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@repo/ui Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@hongggyelim hongggyelim requested a review from jongjunpark July 26, 2025 12:48
@github-actions
Copy link
Contributor

github-actions bot commented Jul 26, 2025 •
edited
Loading

💄 Storybook: https://67c9a019e2c059ab48fd9565-dueiasyqbb.chromatic.com/
🕖 Update: 2025년 07월 26일 22시 26분 19초

@hongggyelim hongggyelim marked this pull request as ready for review July 26, 2025 13:26
@jongjunpark jongjunpark changed the base branch from main to feature July 28, 2025 04:54
jongjunpark
jongjunpark reviewed Aug 5, 2025
View reviewed changes
Copy link
Collaborator

@jongjunpark jongjunpark left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Portal은 나중에 다른 컴포넌트에서 쓸수도 있고,
개인적으로 너가 다른 프로젝트에서도 충분히 활용할 수 있기 때문에 별도 컴포넌트로 분리해보자
+ Portal에 대해서 제대로 한번 공부해보는 것도 필요할 것 같음!

그리고 Portal의 container는 default로 body로 잡는게 일반적임 참고

packages/ui/src/components/dialog/Dialog.tsx Outdated
Comment on lines 12 to 24
interface DialogComposition {
Header?: ReactNode;
Content?: ReactNode;
Footer?: ReactNode;
}

interface DialogProps {
children: ReactNode;
show: boolean;
isMobile?: boolean;
onClose?: () => void;
closeButton?: boolean;
}
Copy link
Collaborator

@jongjunpark jongjunpark Aug 5, 2025

Choose a reason for hiding this comment

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

기존에는 interface가 아닌 type을 썼음

packages/ui/src/components/dialog/Dialog.tsx
Copy link
Collaborator

@jongjunpark jongjunpark Aug 5, 2025

Choose a reason for hiding this comment

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

small size도 고려해야함!

Copy link
Collaborator Author

@hongggyelim hongggyelim Aug 16, 2025

Choose a reason for hiding this comment

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

width 말고는 padding, gap 동일해보이는데 다른거 더 있으??

Copy link
Collaborator

@jongjunpark jongjunpark Aug 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

size는 width만 다른게 맞음

@hongggyelim
Copy link
Collaborator Author

hongggyelim commented Aug 10, 2025

Portal은 나중에 다른 컴포넌트에서 쓸수도 있고, 개인적으로 너가 다른 프로젝트에서도 충분히 활용할 수 있기 때문에 별도 컴포넌트로 분리해보자 + Portal에 대해서 제대로 한번 공부해보는 것도 필요할 것 같음!

그리고 Portal의 container는 default로 body로 잡는게 일반적임 참고

포탈 추가할때 #modal-root 라는 div 에 포탈을 추가하지말고 body로 잡으라는거지?

@jongjunpark
Copy link
Collaborator

jongjunpark commented Aug 12, 2025 •
edited
Loading

Portal은 나중에 다른 컴포넌트에서 쓸수도 있고, 개인적으로 너가 다른 프로젝트에서도 충분히 활용할 수 있기 때문에 별도 컴포넌트로 분리해보자 + Portal에 대해서 제대로 한번 공부해보는 것도 필요할 것 같음!
그리고 Portal의 container는 default로 body로 잡는게 일반적임 참고

포탈 추가할때 #modal-root 라는 div 에 포탈을 추가하지말고 body로 잡으라는거지?

응응 정리하면

  1. Portal을 컴포넌트로 만들기 + Portal이 왜 필요하고 기존 #modal-root에 넣으면 의미가 없고 왜 body에 넣으려고 하는지 이해하기
  2. Portal의 container(createPortal의 두번째 arg)를 default로 body로 잡기
  3. #modal-root라는걸 useEffect로 매번 생성하지말고 component로 만들기
  4. 3번으로 인해 생긴걸로 background용 컴포넌트를 제거할 것
  5. 3번으로 인해 생긴걸로 absolute를 굳이 사용해야하나?를 고민해볼 것

@hongggyelim
Copy link
Collaborator Author

hongggyelim commented Aug 16, 2025

  • Portal 분리하고 dialog 컴포넌트 내에서 감싸주었음
  • Background 컴포넌트 삭제하고 Wrapper 내부에서 배경 처리함
  • 모달 컴포넌트와 배경이 형제 노드여야한다고 생각했는데 굳이 그럴 필요 없는거였군
  • absolute와 width 90% 으로 처리하던 것 수정함

jongjunpark
jongjunpark reviewed Aug 19, 2025
View reviewed changes
packages/ui/src/components/dialog/Dialog.tsx
Copy link
Collaborator

@jongjunpark jongjunpark Aug 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

size는 width만 다른게 맞음

jongjunpark
jongjunpark reviewed Aug 26, 2025
View reviewed changes
packages/ui/src/components/dialog/Dialog.stories.tsx Outdated
<Dialog.Header description={headerDescription}>
{headerTitle}
</Dialog.Header>
{headerTitle}
Copy link
Collaborator

@jongjunpark jongjunpark Aug 26, 2025

Choose a reason for hiding this comment

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

{headerTitle} 두개 들어가있음 하나 지우면 될듯

@hongggyelim hongggyelim merged commit 0133fef into feature Aug 31, 2025
@hongggyelim hongggyelim deleted the feat/dialog branch August 31, 2025 07:02
@jongjunpark jongjunpark mentioned this pull request Oct 13, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jongjunpark jongjunpark jongjunpark approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hongggyelim @jongjunpark