-
Notifications
You must be signed in to change notification settings - Fork 0
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
[#70] - 사이드바 완성 디자인 적용 #76
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
🚀 Storybook Deploy : https://67a21b40b49ad2ad0c08c418-znboccqauf.chromatic.com/ |
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.
저도 저번 Toast 공통 컴포넌트를 구현할때 많은 고민을 했어서 공감이 많이 됩니다..!
저는 Toast 공통 컴포넌트를 만들 때, 스타일 객체로 variant를 관리하고, duration, name 같은 설정 값들을 객체로 분리했습니다.
이렇게 한 이유는 사용성을 최대한 간편하게 유지하면서, 가독성을 높이기 위해서였습니다.
공통 컴포넌트에 대한 저의 생각은 컴포넌트 구조가 동일하거나 동일한 기능을 가지는 컴포넌트에 대해서 공통 컴포넌트로 만들어야 한다는 의견에는 저도 동의합니다.
하지만 너무 많은 props를 전달해야 하거나 공통 컴포넌트를 사용하기에 많은 불편함(이때 불편함의 기준은 각자마다 다르다고 생각합니다)이 생긴다면 공통 컴포넌트로 만드는 것이 아닌 따로 분리를 해야 한다고 생각합니다. 특히 도메인이 다른 경우에는 분리하는게 맞다고 생각합니다..!
공통 컴포넌트는 다양한 상황에서 재사용될 수 있도록 설계해야 하지만, 불필요한 유연성은 오히려 유지보수를 어렵게 하기 때문입니다.
이에 대한 다른 분들의 의견도 궁금합니다!!
<Icon | ||
name="logo-full" | ||
customStyle={css` | ||
path { | ||
fill: ${theme.colors.SORA[200]}; | ||
} | ||
`} | ||
width={99.429} | ||
/> |
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.
이 부분에서 Icon 공통 컴포넌트를 사용하는것 같은데
<Icon
name="logo-full"
color = {theme.colors.SORA[200]}
width={99.429}
/>
이런 방식으로 호버했을때 아이콘 색상 변경이 아닌 단순히 색상 변경만 하고 싶을때에는 위와 같이 color 에 해당 스타일 코드나 색상 값만 넣어주시면 적용됩니다!!
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.
앗 이전에 타입이 변경되기 전에 사용을 잘못 하고 있었네요
알려줘서 감사합니닷~
74ca79e
to
60d60ac
Compare
🚀 Storybook Deploy : https://67a21b40b49ad2ad0c08c418-pzxftjuxwo.chromatic.com/ |
🚀 Storybook Deploy : https://67a21b40b49ad2ad0c08c418-efjseldctk.chromatic.com/ |
🚀 Storybook Deploy : https://67a21b40b49ad2ad0c08c418-qbnzrdzvfn.chromatic.com/ |
저는 공통 컴포넌트는 최소한의 명확한 기능만 가지도록 설계하는 것이 좋다고 생각합니다. |
<Accordion.Item key={projectTitle} value={projectTitle} css={styles.container}> | ||
<Accordion.Header> | ||
<Accordion.Trigger asChild> | ||
{typeof renderTriggerButton === 'function' && renderTriggerButton(projectTitle)} |
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.
RenderAccordionTriggerButtonType이 이미 함수 타입인데, typeof로 한 번 더 확인하신 이유가 궁금합니다!
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.
RenderAccordionTriggerButtonType이 옵셔널 타입이라 주입해주지 않을 경우 undefined가 될 수 있기 때문에 타입가드를 한번더 사용했습니다.
그러나 지금 생각해보니, 어차피 트리거 버튼이나 컨텐츠 버튼의 경우 옵셔널이 아니라 무조건 넣어줘야 하는게 맞기 때문에 옵셔널을 지우고 타입 가드를 제거하는게 낫겠네요!
<div css={styles.wrapper}> | ||
{feedbackPages.map((page, buttonIndex) => ( | ||
<div key={page}> | ||
{typeof renderContentButton === 'function' && renderContentButton(page, buttonIndex)} |
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.
이 부분도 typeof로 한 번 더 타입 확인하신 이유가 궁금합니다 👀
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 Deploy : https://67a21b40b49ad2ad0c08c418-vcpwpgnllv.chromatic.com/ |
🚀 Storybook Deploy : https://67a21b40b49ad2ad0c08c418-yyapanuxzt.chromatic.com/ |
📌 연관된 이슈 번호
close #70
🌱 주요 변경 사항
완성된 디자인에 맞게 사이드바 디자인을 적용합니다
기존 사이드바 로직을 수정합니다
이전에는 재사용을 목적으로
아코디언
및사이드바
로직을 구현할 때 웬만한 기능들은 전부 다 props, children, render props로 주입받았었습니다. 주입받는 것 외의 기본 공통 컴포넌트 로직을 전부 재사용을 하기 위함이었습니다.그러나, 현재 디자인 시스템 특성상 사이드바는 많은 곳에서 사용되지 않으며 정해진 디자인 테마가 별도로 정의되어 있지 않습니다. (=버튼처럼 정확히 어느 디자인 규격대로 쓰인다는 것이 없음)
그렇기 때문에 현재 상황에서 사이드바나 아코디언 컴포넌트를 재사용 목적으로 구현하는 것은 결국
"모든 디자인에 대응해야 한다"
라는 오버엔지니어링에 빠지게 됩니다.이를 위해 모든 스타일, 구조, 컴포넌트, 이벤드, 상태 등을 전부 주입받게 되면 오히려 역설적으로 복잡성이 증가하게 된다 생각했습니다.
그러므로 우선은 기존 구조를 허물고, 재사용 보다는 최대한 간결한 구조로 작성했습니다.
📸 스크린샷 (선택)
🗣 리뷰어에게 할 말 (선택)
공통 컴포넌트를 구현하는 부분에서 아직도 그 경계선이 모호합니다.
버튼같이 규모가 작고, 사용되는 디자인 및 테마가 정해져 있는 경우는 문제가 되지 않습니다. 그냥 관련 테마값만 지정해둔 뒤, 정해진 props만 전달받으면 되니까요.
그러나 사이드바 같이 어느정도 크기가 있는 합성 컴포넌트가 만약 서비스의 여러 곳에 사용될 경우 사이드바에 대한 공통 컴포넌트를 만들어야 합니다.
여기서
"어느 부분까지 공통으로 적용하고, 어느 부분까지 주입을 허용해야 할까?"
라는 그 경계선에 대해 아직도 확신이 서질 않습니다."현재 디자인에서, 최대한 공통된 부분까지 컴포넌트로 묶고, 서로 다른 부분들은 외부에서 주입받는다."
이 의견에는 저도 동의합니다. 그러나 만약 기존의 공통된 부분에서 '약간' 차이가 있는 사이드바가 새로 생길 때마다 매번 공통 컴포넌트의 허용 범위를 수정해야 합니다.
"기존 공통 컴포넌트의 복잡성이 올라가면, 기존 도메인이 아닌 새로운 공통 컴포넌트를 별도로 만들어서 사용한다"
그나마 제일 괜찮은 방향이라고 생각합니다. 그러나 '복잡성'을 정의하는 기준이나 공통컴포넌트의 도메인을 구분짓는 그 경계를 어떻게 정의해야 할지 잘 모르겠습니다.
그 범위를 작게 잡으면 (=조금이라도 차이가 있으면) 계속 새로운 공통 컴포넌트를 만드는 행위가 공통 컴포넌트의 목적성을 퇴색할지도 모른다는 생각이 듭니다.
그렇다고 그 범위를 크게 잡으면 props가 너무 복잡해질 수도 있습니다.
"최소한의 기능인 사이드바 애니매이션 토글 여부만 공통 컴포넌트로 정의해놓고 웬만한 부분은 전부 주입받는다"
이상적인 방법이지만 디자인 및 기능의 세분화가 진행될 수록, 주입받는 양이 어마어마해지게 됩니다. 이는 오히려 공통 컴포넌트 내부에서 처리해야 하는 양이 많아지므로 복잡성을 초래합니다. 또한 최소한의 기능 역시 사람마다 정의하기 나름입니다.
"애초에 정해진 디자인 종류나 테마가 없는데 뭐하러 공통 컴포넌트로 만드나?"
사실 이 부분에 대해 정확히 대답을 할 수는 없습니다. 지속 가능한 컴포넌트를 만드는 명분이 무조건 현재 존재하는 불필요한 반복과 복잡성을 단편화하기 위한 것도 있지만 때로는 추후를 대비한 확장성에 초점을 맞추기 위한 것도 있습니다.
그러나 다른 시각에서는 추후 확장성을 대비한다는 부분이 오버엔지니어링이라는 덫으로 볼 수도 있기에, 이에 대해선 단지 저는 신입으로서 기존 방법을 탈피해보고싶은 기술적인 도전이라고 말을 할 수 밖에 없네요...
어쨌든, 공통된 데이터의 흐름("a라는 input을 받으면 b라는 인터렉션을 한다" 같은 컴포넌트 로직)을 기반으로 최대한 재사용성을 높여보고 싶은데 그 방향성을 잡기가 어려운 것 같습니다.
어떻게 보면 "은탄환은 없다" 라는 말이 있듯이, 제가 실제로는 존재하지 않는 완벽한 방법론을 모색하는데 에너지를 필요 이상으로 쏟고 있는 것 같긴 합니다.
그렇지만 완벽한 방법이 아니더라도 불필요함이나 복잡성을 상대적으로 줄이고자 하는 노력의 일환으로써 어떻게든 많은 의견과 생각들을 서로 얘기해보고 싶습니다.