-
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
feat: add SearchBar to Leaderboard #84
Conversation
SearchBar 스타일도 추가했습니다~ |
Select 요소의 기본 값이 "기수"이고, 이 것을 선택하면 모든 기수를 보여주네요. 제가 피그마에서 목업을 볼 때는 크게 못 느꼈는데 막상 사용해보니 좀 햇길리는 경험인 것 같아요. 어떻게 생각하시나요? @yolophg Shot.2024-11-23.at.11.21.10.mp4 |
@sounmind 님, 유저 네임으로 필터링할 때 대부분의 사용자가 대소문자 구분을 신경쓰지 않을 것 같아요. 대소문자 무시해주실 수 있으실까요? Shot.2024-11-23.at.11.25.42.mp4 |
65f1b3c
to
d04bfaa
Compare
고민해봤는데 아래 두가지 선택지로 추려질 것 같습니다. 2번은 기본값을 '1기'로 두고, 전체 기수 보여주는 것을 아예 제외하고 가는 방법도 있을 것 같아요. 개인적으로는, 전체 기수 옵션도 사용자에게 제공하고, 리더보드인만큼 사용자들로 하여금 다양한 데이터를 볼 수 있도록 주도권을 주면 좋을 것 같다는 생각이라, 텍스트를 좀 더 직관적으로 변경하는 1번 방법이 심플하고, 더 괜찮을 것 같다는 의견입니다! |
@sounmind Chromatic이 UI 변경 사항을 추척할 수 있도록 리베이스 부탁드립니다! |
b0a8d85
to
8465960
Compare
8bf14a5
to
fa16c3f
Compare
fa16c3f
to
eb99dd1
Compare
완료했습니다~ |
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.
피드백 반영해주셔서 감사합니다 🙇♂️ 검색 바 테스트해봤는데 잘 되는 것 같아요 😁
- remove local cohort state and inject them from props
- add filter, setFilter, totalCohort to return value
6351a80
to
87ade24
Compare
체크리스트