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

refactor: Dialog 레이어 쌓임 문제 해결 #96

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

hae-on
Copy link
Collaborator

@hae-on hae-on commented Mar 25, 2024

Issue

✨ 구현한 기능

여러분 드디어 고쳤습니다...!
toast id를 따로 상태로 둬서 일반 상황에서는 'toast-container'라는 div에 붙고
dialog 내부에서는 'toast-in-dialog-container' div에 붙도록 하였습니다.

이랬는데
스크린샷 2024-03-25 오후 6 17 46

요래 됐슴다
스크린샷 2024-03-25 오후 9 40 44

이제 BottomSheet 사용처에서 hasToast라는 props를 주면 상위에 뜹니다!

<BottomSheet hasToast ...>

📢 논의하고 싶은 내용

styled를 module css로 바꾸려고 했는데 다른 곳에서는 문제가 없는데
스토리북에서는 계속 에러가 뜨더라구요.
이게 찾아봤을 때 storybook main.ts webpack 설정 때문에 그렇다는데 있는 코드들도 main.js 밖에 없고 제대로 되는 것도 없더라구요.
어차피 우리 rollupjs로 바꿀거여서 그때 다시 적용해야하니까 스타일은 따로 안 바꿨습니다!

🎸 기타

갑자기 디자인시스템 스토리북에 왜 React를 import를 안해주면 에러가 나죠? 저만 그런가요??

@hae-on hae-on self-assigned this Mar 25, 2024
@hae-on hae-on changed the title refactor: 토스트가 다이얼로그 위에 뜨도록 수정 refactor: Dialog 레이어 쌓임 문제 해결 Mar 25, 2024
Copy link
Collaborator

@Leejin-Yang Leejin-Yang left a comment

Choose a reason for hiding this comment

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

어차피 우리 rollupjs로 바꿀거여서 그때 다시 적용해야하니까 스타일은 따로 안 바꿨습니다!

굿입니다. 나중에 바꿔도 될거 같네요

갑자기 디자인시스템 스토리북에 왜 React를 import를 안해주면 에러가 나죠? 저만 그런가요??

현재 tsconfig에서 스토리북 파일은 제외되어 있습니다. 이것도 rollup 하면서 수정했어요~


그럼 사용하는 곳에서 컨테이너를 두개 두어야 하는건가요?
수고했어요 해온~

border: none;
z-index: 9999;
z-index: 1000;
Copy link
Collaborator

Choose a reason for hiding this comment

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

펀잇 헤더가 1001이라 헤더가 위에 있지 싶네요..?
나중에 z-index도 상수화 얘기해보죠.!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

얘는 아마 z-index가 상관이 없을거에여!
제가 알기로는 dialog가 z-index 상관없이 무조건 최상위라고 알고있거든요?
그래서 저거 지울까하다가 일단 수만 줄여서 나뒀는데 저거 지울까여 아니면 9999로 다시 올릴까여??

const [isClosing, setIsClosing] = useState(false);
const ref = useRef<HTMLDivElement>(null);
const ref = useRef<HTMLDialogElement>(null);
Copy link
Collaborator

Choose a reason for hiding this comment

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

오 dialog 태그로 바꿔서 ref로 핸들링하는거군요 👍

@hae-on
Copy link
Collaborator Author

hae-on commented Mar 26, 2024

현재 tsconfig에서 스토리북 파일은 제외되어 있습니다. 이것도 rollup 하면서 수정했어요~

아아 그래서 저랬구나... rollup은 작업하신거에요???
사용하는 곳에서는 그냥 바텀시트 위에 토스트가 올라가야한다 싶으면 hasToast 요 props만 적어주면 됩니다~~

<BottomSheet hasToast ...> 요렇게!

토스트 필요 없는 바텀시트면 안 적어도 되고!

Copy link
Member

@xodms0309 xodms0309 left a comment

Choose a reason for hiding this comment

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

오오오오오 고생했어요 해옹
토스트의 달인..🥪

@hae-on
Copy link
Collaborator Author

hae-on commented Mar 28, 2024

최대 높이 설정하였습니다!
디자이너분 말로는 436px이 최대 높이라고 하네요!

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.

Dialog 개선 및 토스트 레이어 쌓임 문제 해결
3 participants