-
Notifications
You must be signed in to change notification settings - Fork 6
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-fe: DropdownSub 컴포넌트 구현 #762
Conversation
1728030968.302279 |
1728030972.419849 |
1728031004.015119 |
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.
제가 봤을 떄 예측 불가능한 동작은 없었습니다. 남긴 코멘트에 대해서 검토하시고 반영해주세요~
items: (ClickableItem | SubTrigger)[]; | ||
} | ||
|
||
export type DropdownItemType = ClickableItem | SubTrigger; |
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.
타입 오버라이딩을 사용해서 잘 구현해주셨네요👍
padding: 0.4rem; | ||
|
||
top: -0.36rem; | ||
right: ${({ width }) => `${width - 4}px`}; |
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.
현재는 왼쪽에 고정적으로 렌더링되는데요,
prop으로 렌더링 방향을 받아서 선택할 수 있게 만들 수 있을까요?!
@@ -1,3 +1,4 @@ | |||
import { useDropdown } from '@contexts/DropdownContext'; |
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.
저는 SubTrigger가 atom이라고 생각하는데 러기는 어떻게 생각하시나용?
@@ -0,0 +1,61 @@ | |||
import DropdownSubTrigger from '@components/_common/molecules/DropdownSubTrigger'; |
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.
molecules로 바꿔주세요~
size?: 'sm' | 'md'; | ||
} | ||
|
||
export default function RecursiveDropdownItem({ items, size = 'sm' }: RecursiveDropdownRendererProps) { |
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.
러쉬: DropdownItemSelector 어떤가요?
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.
렛서: DropdownItemRenderer 어떤가요?
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.
러기가 판단하고 변경or유지 후 props의 interface와 이름 일치시켜주세요~
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.
러기 고생하셨어요. 쉽지 않은 작업을 해주셨네요. 지난 주에 렛서가 피드백 드렸던 내용도 반영되었고, 서브 드랍다운 노출 위치를 정할 수 있도록 유연하게 만들어 주신 것 같아요. Storybook과 local test 환경에서 각각 테스트했고, 모두 의도대로 동작하는 것을 확인했기에 Approve 드립니다. 🙏
setIsOpen(!isOpen); | ||
if (isOpen) close(); | ||
if (!isOpen) open(); |
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.
Early return이나 if-else보다는 이런 방식이 더 직관적이라고 생각하신 것일까요? 🤔
Co-authored-by: Jeongwoo Park <[email protected]>
Original issue description
목적
작업 세부사항
참고 사항
COM_DROP_SUB
closes #761