-
Notifications
You must be signed in to change notification settings - Fork 2
[REFACTOR] sidebar 컴포넌트 개선 #56
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
Conversation
✅ Deploy Preview for hufscheer-manager ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for hufscheer ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
ohprettyhak
left a comment
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.
Accordion 컴포넌트를 통해 대회가 누적될 때를 대비한 부분이 정말 세심하게 뷰를 구현한다는 생각이 드네요! Sidebar 부분도 기존에 추가한 Modal 컴포넌트를 활용하여 효율적으로 구현하신 거 같습니다. 고생하셨습니다.
말씀하신 List 컴포넌트의 경우 LeagueList 라는 이름의 컴포넌트 내부에서 두 번에 걸쳐 사용 되고 있는데, List 컴포넌트의 코드를 확인하지 않으면 어떤 기능을 하는지 정확하게 판단하기가 어렵습니다.
네이밍이 비슷해서 이런 느낌을 받는 거 같기도 하고, components 폴더의 같은 레벨에 두 컴포넌트가 존재하다보니 헷갈리는 거 같아요.
| }); | ||
| // backgroundColor: theme.colors.background.normal, | ||
| // borderRadius: 8, | ||
| // }); |
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.
앗 그러네요! 수정하겠습니다ㅎㅎ
ohprettyhak
left a comment
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.
연도 클릭과 내부 대회 리스트 디자인과 관련해서는 디자인쪽과 논의해보는 것은 어떨까요? 저는 굳이 필요 없다는 의견이긴 하지만, 이쪽 디자인을 보니 이펙트가 있는 것이 사용자 경험 면에서 더 좋을 거 같기도 하네요.
|
HiimKwak
left a comment
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.
저는 이 프로젝트가 사이드 프로젝트인만큼 이것저것 시도해보는 것은 정말 바람직하다 생각합니다. List 컴포넌트도 그 일환인 것 같아요. 다만 저도 List 컴포넌트는 오히려 이해의 단계를 불필요하게 한 단계 추가한다고 느껴져 적극적인 사용을 할 필요는 없다 생각합니다.
고생하셨습니다!
🌍 이슈 번호
✅ 작업 내용
📝 참고 자료
♾️ 기타
List컴포넌트를 이용하고 있습니다.arr.map(data => <Component {...data} />처럼 명시적으로 매핑하는 구조를 제거하고 싶어서 구현해 보았는데, 막상 구현하고 나니 오히려 흐름 파악이 더 어려워진 것 같다는 생각이 들기도 하네요.. 해당 컴포넌트에 대한 여러분의 생각이 궁금합니다. 과하다는 생각이 드신다면 바로 제거하겠습니다!