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

Modal 컴포넌트 개선방안 논의 #1879

Closed
torres-triple opened this issue Jan 27, 2022 · 5 comments
Closed

Modal 컴포넌트 개선방안 논의 #1879

torres-triple opened this issue Jan 27, 2022 · 5 comments

Comments

@torres-triple
Copy link
Contributor

torres-triple commented Jan 27, 2022

위클리문서: https://titicaca.atlassian.net/wiki/spaces/dev/pages/2912911384

@torres-triple
Copy link
Contributor Author

위클리 논의 중 나온 의문

  • 랜덤 유니크 아이디 해시 부여를 해버리는것은 어떠한가
  • 다이나믹한 액션들이 있을경우에 대한 대응을 어떻게해야하는가?
  • useModal 에 캡슐화 고려
    • useModal 마운트시 유니크 해시
  • 뒤로가기시 모달 보여주기 지원 ?

@dddeok
Copy link
Contributor

dddeok commented Jan 27, 2022

useModal 캡슐화 구체화 되면다면 정말 편리할거 같네요.
호텔 버그 수정작업중 fixed inside fixed 상황에서 부모의 bottom = 0 스타일의 영향으로 인해서 같이 최하단으로 내려가며 모달이 제대로 뜨지 못하는 상황이 발생했는데, 상단으로 올리자니 엮인 함수들도 함께 올라와 불필요한 props drilling 발생하게 되고 하니
간단한 버그인데도 수정하기 애매해지더군요 😢
페이지단에서 모달 및 팝업 같은 overlay 형태의 컴포넌트들이 함께 관리되면 좀더 효율적일거 같습니다.

@giwan-dev
Copy link
Contributor

giwan-dev commented Jan 27, 2022

useModal의 인터페이스는 요런 형태를 상상했습니다.

const { status, open, close } = useModal()

return (
  <>
    <button onClick={() => open()}>모달 열기</button>
    <Modal open={status === 'opened'} onClose={() => close()} />
  </>
)

구현은 #1846 을 참고해주시면 됩니다.

@polysiya
Copy link
Contributor

다이나믹한 경우는 사실 잘 떠오르진 않지만, Provider에 모든 모달이 미리 정의된 상태로 있어야 한다는 것은 부담도 크고 불필요한 경우가 많을 것 같아요. 위키 문서의 예시 코드엔 아래와 같이 4개뿐이지만 얼마든지 다양한 디자인으로 더 많은 모달이 필요할 수 있으니까요.

  <Alert />
  <Modal />
  <Confirm />
  <TransitionModal />

각 모달은 필요한 곳에서 만들어서 선언되어 있는 것이 좋을 것 같아요.

또한 2주 전 @inbeom 의 위클리 발표 주제로 나왔던 이슈와 비슷한 문제점도 있을듯해요. 특정 페이지에서만 필요한 모달을 ModalContext.Provider에 모두 선언해놓아야 할테니까요.

따라서 저도 useModal과 같은 방안이 더 좋아보이네요!

@inbeom
Copy link
Contributor

inbeom commented Apr 28, 2022

React@18에 추가된 useId 라는 훅이 있는데, 서버-클라이언트에서 동일한 랜덤 값 생성을 보장해준다고 해요. useModal 구현에 잘 쓸 수 있을 것 같아서 기록 남겨 둡니다!

@drakang4 drakang4 added this to the ui-renewal milestone Nov 9, 2022
@drakang4 drakang4 removed this from the ui-renewal milestone Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants