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/#40] 홈 이동 안내 팝업 모달 제작 #41

Merged
merged 4 commits into from
Feb 2, 2025
Merged

Conversation

sikkzz
Copy link
Collaborator

@sikkzz sikkzz commented Feb 2, 2025

💡 변경사항 & 이슈

공통 모달 컴포넌트 제작
홈 이동 안내 팝업 모달 제작

✍️ 관련 설명

우선 modal은 createPortal 사용해서 제작했어. createPortal에 elementId를 따로 추가한 이유는 modal말고 toast 메시지의 확장성이 생길 거 같아서 그때는 toast도 동일하게 portal을 사용할 예정이라 추가해줬어 (토스트 메시지 디자인 시안에 있더라구 웹에서 구현할지는 아직 미정)

그리고 지금은 home 컴포넌트에 갤러리 버튼에 modal을 붙여놨는데 이후에 리뷰 생성 완료 페이지 완성하면 거기로 옮기면 될 거 같아
테스트용으로는 여기다 유지해놓을게

/ 경로에서 갤러리 버튼 클릭하면 모달 확인 가능해

추가적으로 모달 컴포넌트 이름이 좀 마음에 안드는데.. 혹시 좋은 의견 있으면 얘기해줘! 아무리 생각해봐도 더 좋은 이름이 생각 안나더라..

(아이콘 이슈로 #34 코드를 pull해와서 #34 머지 이후에 rebase하고 머지 예정!)

⭐️ Review point

  • 모달 로직 개선이나 의견
  • 모달 컴포넌트 이름 의견
  • 이외 피드백이나 추가 의견

📷 Demo

스크린샷 2025-02-02 오후 6 28 39

@sikkzz sikkzz added the Feature new feature label Feb 2, 2025
@sikkzz sikkzz self-assigned this Feb 2, 2025
@sikkzz sikkzz requested a review from lgrin-byte as a code owner February 2, 2025 09:29
@sikkzz sikkzz linked an issue Feb 2, 2025 that may be closed by this pull request
1 task
Copy link

github-actions bot commented Feb 2, 2025

🧷 Storybook: https://6798a432c75b75bf6b81bc26-ndwmhrrkqx.chromatic.com/

⏰ Update: 2025년 02월 03일 00시 00분

Copy link
Member

@lgrin-byte lgrin-byte left a comment

Choose a reason for hiding this comment

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

HomeReturnModal..?
ResetModal? 흠,,

모달 외부 눌렀을때도 닫히는게 좋을 것 같은데 이건 디자이너한테 물어볼까??

@sikkzz
Copy link
Collaborator Author

sikkzz commented Feb 2, 2025

HomeReturnModal..? ResetModal? 흠,,

모달 외부 눌렀을때도 닫히는게 좋을 것 같은데 이건 디자이너한테 물어볼까??

나도 그게 더 좋기는 한데 얘도 그렇고 팝업 시트도 그렇고 관련 정책이 없어서 이후에 물어보고 수정 생긴다면 한번에 수정할 생각이였어!

모달 이름은 우선 냅둘게? 마땅히 생각나는게 없긴하네 지금은 단어 자체는 직관적이지만 좀 길어서 별로인거라!

@sikkzz sikkzz merged commit 73269e8 into develop Feb 2, 2025
3 checks passed
@sikkzz sikkzz deleted the feat/#40 branch February 2, 2025 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] 홈 이동 안내 팝업 모달 제작
2 participants