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

[FE][FEAT] #145 - 플로팅 버튼 구현 #150

Merged
merged 16 commits into from
Nov 13, 2024

Conversation

leedongyull
Copy link
Collaborator

@leedongyull leedongyull commented Nov 12, 2024

📝 PR 개요

Floating 버튼 구현
애니메이션 구현
tsDoc 작성


✅ 체크리스트 (Checklist)

  • 우측 하단에 버튼이 존재
  • 토글이 가능해야하며 메뉴가 위쪽으로 펼쳐져야 함.
  • 메뉴를 통해 타입을 선택할 수 있음.
  • 타입 선택 시 토글이 닫히고 아이콘의 모양이 변경돼야함.

🔄 관련 이슈 (Linked Issues)

#145 해결


📷 스크린샷 및 동영상 (선택 사항)

Vite-React-TS-외-페이지-2개-개인-Microsoft_-Edge-2024-11-12-17-16-05

image

  • 디자인 수정해보았습니다!

참고사항

  • 컴포넌트를 만들기는 했습니다만, 들어가야 할 것들이 많고 애니메이션도 있어 공통 컴포넌트가 맞는지가 의문이 듭니다!
  • 또한, 확장성도 저작도구의 종류가 늘어나는 정도는 type 선언을 통해 고려했습니다만 공통 컴포넌트로 사용하기 위해서는 더 분리를 해야할 것 같습니다!
  • 따라서 이 컴포넌트를 종속 시킬 것 인지 공통 컴포넌트로 마이그레이션 해야할 지 궁금합니다!
  • 이외에도 잘못된 부분들 알려주시면 감사하겠습니다!!

@leedongyull leedongyull added the 확인 요청 리뷰어에게 리뷰 요청 PR 날린 상태 (PR 머지 전) label Nov 12, 2024
@leedongyull leedongyull requested review from effozen and happyhyep and removed request for effozen November 12, 2024 08:35
@leedongyull leedongyull requested a review from effozen November 12, 2024 08:35
@github-actions github-actions bot requested a review from juwon5272 November 12, 2024 08:35
@leedongyull leedongyull removed the request for review from happyhyep November 12, 2024 08:38
@effozen effozen added 수정 요청 특정 이유로 PR을 approve 할 수 없어 반려한 상태 (리뷰어 2명 중 PR 반려 한 사람이 상태 변경) and removed 확인 요청 리뷰어에게 리뷰 요청 PR 날린 상태 (PR 머지 전) labels Nov 12, 2024
@github-actions github-actions bot added the 확인 요청 리뷰어에게 리뷰 요청 PR 날린 상태 (PR 머지 전) label Nov 12, 2024
@github-actions github-actions bot requested a review from happyhyep November 12, 2024 08:44
@effozen effozen removed the 확인 요청 리뷰어에게 리뷰 요청 PR 날린 상태 (PR 머지 전) label Nov 12, 2024
Copy link
Member

@happyhyep happyhyep left a comment

Choose a reason for hiding this comment

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

comment 남겨주신 부분은 전체적인 플로우를 확인해야 답변할 수 있을 것 같아서, 백엔드 구현 하고나서 보면 너무 늦어질 것 같아 저는 코드만 보고 수정하면 좋을 것 같은 부분 남겨뒀습니다!

코멘트 달아주신 내용은 재도님과 주원님께서 확인해보시고 결정하시는 게 더 빠르고 좋을 것 같습니다!

그리고 영상 캡처해주신 것만 확인하고 코드로는 확인하지 않아서 정확하지는 않은데, 디자인이 많이 반영되지 않은 것 같습니다! 토글 간의 간격도 조금 달라 보이고. 토글 간에 그림자도 넣어뒀었는데 그런 디테일이 반영되지 않은 것 같습니다! 확인 부탁드립니닷!

@effozen
Copy link
Collaborator

effozen commented Nov 12, 2024

공통컴포넌트 이슈 관련해서는 크게 신경쓰지 않아도 될 것 같아요. FlatingButton에는 추가적인 요소만 들어갈 수 있도록 확장성만 고려해서, 도메인 종속적으로 만들어도 될 것 같아요. 근데 일케 적고보니 걍 공통컴포넌트 느낌이긴 하네요 ㅎㅎ...

공통컴포넌트가 완벽한 껍데기(보일러 플레이트)가 될 필요는 없다고 생각해요. 애니메이션이나 디자인들은 정해놓고 데이터와 이벤트만 다르게 받아도 여러 곳에서 재사용이 가능하겠죠! 그러면 그게 곧 공통컴포넌트라고 생각합니다.

만약 공통컴포넌트로 만든다고 하면, 제 방식으로는 의존성(데이터 등을)외부에서 전부 주입한다고 생각하고 작업하는 것 같아요.
다만, 이렇게 되면 버튼 하나하나 당 이벤트를 어떻게 처리할지에 대한 의문이 생기게 되는데, 이는 이벤트 위임으로 한번에 처리하는 방법도 있긴 합니다.

Dropdown을 기준으로 말하면, 드랍다운도 하나하나 다 Router를 걸어줘야하는데, 저는 이에 대해서는 Dropdown wrapper 측에서 공통 이벤트를 하나 받아서, 그걸 기준으로 처리하게 할 생각입니다.

데이터도 외부에서 넘겨주는 만큼 이벤트도 외부에서 하나만 넘겨주면 결과적으로 Dropdown은 배열 하나와 이벤트 하나 이렇게 받으면 되는 형태가 나올 것 같아서요.

다만 이게 좋은 방법인지는 고민좀 해보려고 합니다.

@leedongyull leedongyull added 확인 요청 리뷰어에게 리뷰 요청 PR 날린 상태 (PR 머지 전) and removed 수정 요청 특정 이유로 PR을 approve 할 수 없어 반려한 상태 (리뷰어 2명 중 PR 반려 한 사람이 상태 변경) labels Nov 12, 2024
@github-actions github-actions bot requested a review from effozen November 12, 2024 09:43
happyhyep
happyhyep previously approved these changes Nov 12, 2024
effozen
effozen previously approved these changes Nov 12, 2024
@leedongyull leedongyull dismissed stale reviews from effozen, juwon5272, and happyhyep via b667d1a November 13, 2024 10:59
@leedongyull leedongyull merged commit d1c2e3e into frontend Nov 13, 2024
4 of 9 checks passed
@leedongyull leedongyull deleted the feature/fe/#145-FloatingButton branch November 17, 2024 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
확인 요청 리뷰어에게 리뷰 요청 PR 날린 상태 (PR 머지 전)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants