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: toast 컴포넌트를 구현합니다 #221

Merged
merged 8 commits into from
Aug 26, 2024

Conversation

Jungjjeong
Copy link
Member

🤔 어떤 문제가 발생했나요?

🎉 어떻게 해결했나요?

  • toast component를 구현합니다
  • 다음과 같이 사용합니다
const { toast } = useToast();

// 옵션 존재
toast( '토스트 메세지 입력', { type: 'error', delay: 3000 })

// 옵션 미존재 (default: success type, delay 2000ms)
toast( '토스트 메세지 입력' )

📷 이미지 첨부 (Option)

⚠️ 유의할 점! (Option)

@king2dwellsang king2dwellsang added Review Plz🙏 # Review is not yet complete feat # Add feature labels Aug 26, 2024
Copy link
Collaborator

@hjy0951 hjy0951 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 👍

app/portal-root.tsx Outdated Show resolved Hide resolved
Comment on lines +16 to +23
/**
* @description Toast를 조작하기 위해 사용합니다.
*
* 사용 방법은 아래와 같습니다.
* const { toast } = useToast();
*
* toast('토스트 메세지', { type: 'success', delay: '300' })
*/
Copy link
Collaborator

@hjy0951 hjy0951 Aug 26, 2024

Choose a reason for hiding this comment

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

친절한 주석! 감사합니다!

</Portal>
<>
<Portal>
<>
Copy link
Collaborator

Choose a reason for hiding this comment

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

요 Fragment는 포탈과 토스트를 구분하기 위해 사용된 것일까요?!

Copy link
Member Author

@Jungjjeong Jungjjeong Aug 26, 2024

Choose a reason for hiding this comment

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

children으로 두 컴포넌트를 넣기 위해 fragment를 추가했습니다
기능은 없습니다!

* @description dialog state store
*/
export const toastAtom = atom<Map<string, ToastProps>>(
new Map<string, ToastProps>(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Map을 사용한 이유는 여러 토스트가 겹쳐 발생하였을때, 순서대로 쌓아서 보여주기 위함일까요??

Copy link
Member Author

Choose a reason for hiding this comment

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

map 객체를 사용한 이유는 다음과 같습니다.

  • id값 (난수)를 기준으로 해당 메세지(toast 상태)를 delete 처리하기 쉽습니다
  • 순서대로 쌓아 보여주는 요구사항이 있을 때, 상태 업데이트 로직 일부 수정만으로 구조를 변경할 수 있습니다.

현재 디자인 상으로는 순서대로 쌓아 보여주는 기능은 존재하지 않지만, 위 두 개의 이유로 인해 Map 객체 구조가 상태에 적합하다 생각했습니다. :)
혹시 충분한 답변이 되었을까요?

Copy link
Collaborator

@hjy0951 hjy0951 Aug 26, 2024

Choose a reason for hiding this comment

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

넵넵 제대로 이해한 것 같습니다!! 답변 감사합니다! 💯

@king2dwellsang king2dwellsang added Approved 🆗 # Review is completed and removed Review Plz🙏 # Review is not yet complete labels Aug 26, 2024
Copy link
Member

@wokbjso wokbjso left a comment

Choose a reason for hiding this comment

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

코드 너무 깔끔하네요👍🔥

const isOpen = toastState.size > 0;

return (
<dialog
Copy link
Member

@wokbjso wokbjso Aug 26, 2024

Choose a reason for hiding this comment

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

dialog 태그의 속성으로 인해 스크롤이 존재하는 페이지에서 toast가 띄워질 시,
사라질 때 스크롤이 페이지 바닥으로 이동하는 현상이 존재하는 것 같아요!

dialog 태그 스타일을 수정, 혹은 스크롤을 현재 위치로 고정해주는 로직이 추가되어야 자연스러운 동작이 될 것 같습니다~

Copy link
Member Author

Choose a reason for hiding this comment

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

오 맞아요 방금 그 부분 수정했습니다!
layout shifting이 발생하여 dialog component의 스타일 속성을 변경했습니다.

const containerStyle = css({
  position: 'fixed',
  backgroundColor: 'transparent',
  zIndex: 900,
});

@Jungjjeong Jungjjeong merged commit 9444a64 into fix/delete-cheer Aug 26, 2024
1 check passed
@Jungjjeong Jungjjeong deleted the feature/toast branch August 26, 2024 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved 🆗 # Review is completed feat # Add feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants