Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: Dialog 컴포넌트 구현 #19
feat: Dialog 컴포넌트 구현 #19
Changes from 1 commit
a0b64fa
6eaedd5
5f9547f
f013caa
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아이콘은 나중에 모아서 svgr 넣어서 파일로 만들거나.. 아이콘 폴더 따로 만들어야겠다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
하드코딩하는 색상은 최대한 피하는게 좋을 듯!?
dim.dialog
같이 팔레트에 넣어서 써보는건 어떨까 싶어!한번 설정하면 잘 바뀌지는 않겠지만, 나중에 찾기 어려울수도 있으니..!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6eaedd5 에서 반영 완료
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
id같은 것을 추가로 받아서 이미 만들어졌으면 사용하고 없으면 새로 생성하는 방식으로 써도 좋겠다!
모달, 다이얼로그 등등.. 혹시나 포탈 쓰는 것들이 많아지면 필요할 것 같아!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
포탈에 id를 달아야 하는 이유가 있을까? id로 뭔가 관리를 해야하는 경우가 생길까?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
포탈 쓰는 이유가 계층 내부에수 선택자 충돌에 대비한거니까, 모달, 다이얼로그 같은 컴포넌트는 각각의 포탈을 갖는게 맞다고 생각했어! (사실 크게 중요치는 않을 듯)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
내가 Portal에 대해서 잘 모르고 하는 이야기일 수도 있지만, Portal을 생성하는데 많은 비용이 들지는 않을 거라고 생각해서 요건 이슈가 생기면 달아줘도 좋을 것 같다고 생각해!
현재 Dialog 컴포넌트도 각각 Portal을 갖도록 구현하기도 했으니까!