[Feature] 액션시트 컴포넌트 구현 #165
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hamo-o
force-pushed
the
feature/action-sheet-component
branch
from
ab9e60c to
d45ec4a
Compare
|
2 tasks
hamo-o
requested review from
eugene028,
SeieunYoo and
soulchicken
as code owners
|
Size Change: +21.4 kB (+3.15%) Total Size: 702 kB
ℹ️ View Unchanged
|
SeieunYoo
reviewed
Feb 6, 2025
There was a problem hiding this comment.
수고하셨어요~~
저도 children 의 타입을 강제할 수 있는 방법은 없나,, 찾아봤는데 언급 주신 이슈 처럼 아직 해결되진 않은 거 같더라구요.
대안책으로는..
컴파일 타임에서 children 타입을 지정하는 건 어려울 거 같지만 런타임에서는 children 을 순회하면서 특정 타입의 컴포넌트 인지 아닌 지 판단할 수 있지 않을까...🤔 라는 생각이 드네요.
Children.forEach(children, (child) => {
if (typeof child === 'Element' && child?.type === 'ActionSheetHeader') {
...
}|
|
||
| const dialogStyle = cva({ | ||
| base: { | ||
| width: 390, |
There was a problem hiding this comment.
p3;
모바일 뷰에서는 전체 너비에 따라 100%로 조정되어야 할 거 같아요! pc 뷰에서는 max width 를 390px 로 고정해도 좋을 거 같네요
Comment on lines
+18
to
+19
| text: string; | ||
| subText: string; |
There was a problem hiding this comment.
p3;
text 와 subText 는 줄바꿈이 생길 걸 고려해서 ReactNode 로 받는 게 더 안정적일 거 같아요!
그리고 기본 h1 텍스트만 쓰이는 상황도 있을 거 같아서 sub 는 optional 로 받는 게 좋을 거 같습니당
+) style, className 은 defaultProps 를 확장하면 따로 정의하지 않아도 될 거 같습니다.
Comment on lines
+13
to
+18
| componentSubtitle: "액션시트 컴포넌트", | ||
| a11y: { | ||
| config: { | ||
| rules: [{ id: "color-contrast", enabled: false }], | ||
| }, | ||
| }, |
There was a problem hiding this comment.
p3;
합성 컴포넌트 처럼 쓰인다는 description 이 들어가면 더욱 좋을 거 같은데 description property 를 활용해서 명시하면 어떨까요?
- ActionSheet.Header Footer Body 를 조합해서 쓸 수 있어요.
Comment on lines
+41
to
+47
| const ActionSheet = ({ | ||
| isOpen, | ||
| onClose, | ||
| children, | ||
| ...rest | ||
| }: ActionSheetProps) => { | ||
| const dialogRef = useRef<HTMLDialogElement>(null); |
There was a problem hiding this comment.
p3
최상위 ActionSheetComponent 에는 ref 를 프로퍼티로 주입받을 수 있도록 열어도 좋을 거 같아요.
| return ( | ||
| isOpen && ( | ||
| <ActionSheetContext.Provider value={{ onClose: handleClose }}> | ||
| <dialog className={dialogStyle({ state })} ref={dialogRef} {...rest}> |
There was a problem hiding this comment.
p3;
외부에서 주입받은 className 은 기존의 className 과 합쳐져야 className 으로 만든 panda 스타일링이 잘 동작할 거 같아요! 관련 이슈 #186
| position="fixed" | ||
| top={0} | ||
| width="100vw" | ||
| zIndex={9998} |
There was a problem hiding this comment.
p4;
actiionsheet z Index 도 토큰으로 지정하면 좋을 거 같아요~
|
|
||
| const handleClose = () => { | ||
| setState("close"); | ||
| setTimeout(onClose, 100); |
There was a problem hiding this comment.
p4;
100ms 는 상수로 분리해서 어떤 용도인지 나타내면 좋을 거 같아요!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🎉 변경 사항
액션시트 컴포넌트를 구현합니다.
🚩 관련 이슈
🙏 여기는 꼭 봐주세요!
문제상황
ReactElement타입이면서, 원하는 컴포넌트의 타입을 제네릭으로 받으면prop타입을 제한할 수 있으니children의 형식과 타입을 제한할 수 있다고 생각했습니다.ReactElement중에서도ActionSheetHeader에서 정의한prop을 받아야 만족하는 것으로 예상했습니다.ReactElement로children개수만 맞추면 타입체킹에서 걸리지 않았습니다…해결방법으로 알아봤던 것들
-> 결국 안된다는 것 같은데, 혹시 다른 아이디어가 있으시면 리뷰 부탁드립니다..