[Feat] SearchBar 컴포넌트 제작#35
Hidden character warning
Conversation
PR 검증 결과✅ TypeScript: 통과 |
KyeongJooni
left a comment
There was a problem hiding this comment.
수고하셨습니다! 포커스 스타일도 접근성을 위해 추가하면 좋을 것 같아요!
| /** | ||
| * true면 입력값 유무로 default/filled 자동 전환 | ||
| * (요구사항: default로 보이다가 입력하면 filled) | ||
| */ | ||
| autoFilled?: boolean; | ||
|
|
||
| /** | ||
| * 강제로 상태 고정하고 싶을 때만 사용 (기본은 auto) | ||
| */ | ||
| state?: SearchBarState; | ||
|
|
||
| /** | ||
| * wrapper에 추가 클래스 주고 싶을 때 | ||
| */ |
There was a problem hiding this comment.
주석 정리 해주시거나 불필요한 주석은 삭제해주세요!
| const [innerValue, setInnerValue] = useState<string>(defaultValue ?? ''); | ||
| const currentValue = isControlled ? value : innerValue; | ||
|
|
||
| const hasValue = (currentValue ?? '').trim().length > 0; |
There was a problem hiding this comment.
지금 스페이스 쭉 입력해도 trim 때문에 기본 상태로 보일 것 같은데 확인 부탁드립니다!
| onChange, | ||
| ...rest | ||
| }: SearchBarProps) => { | ||
| const isControlled = value !== undefined; |
There was a problem hiding this comment.
제어/비제어 모드를 이렇게 구현해주신 이유가 있을까요? useRef 사용해서 고정하여 구현하면 좀 더 안정적일 것 같습니다!
There was a problem hiding this comment.
리뷰 감사합니다! SearchBar.tsx 파일을 리팩토링하였는데, filled 상태에 따른 스타일 변경 때문에 value 추적이 필요해서 useRef 대신 useState로 처리했습니다! 컴포넌트 규모가 작아 렌더링 코스트가 크지 않다고 판단해 해당 방식으로 구현하였는데, 제어/비제어 안정성 관점에서 useRef로 처리하는 게 더 낫다면 그 방향으로 수정하겠습니다!
|
예빔님 수고하셨습니다! 저는 개인적으로 focus 상태에서도 디자인이 똑같이 filled로 바뀌는 게 더 좋을 것 같습니다...! |
PR 검증 결과✅ TypeScript: 통과 ESLint 상세
src/shared/assets/icons/index.ts
|
PR 검증 결과✅ TypeScript: 통과 |
✨ 주요 변경사항
📝 작업 상세 내용
currentColor로 설정해 상태 색상이 반영되도록 구현✅ 체크리스트
Close #번호추가📸 스크린샷 (선택)
SearchBar.mov
🔍 기타 참고사항
피그마에는 filled 상태에만 디자인이 바뀌도록 정의되어 있는데, UX 관점에서 focus 상태에서도 디자인이 filled와 동일하게 바뀌는 게 더 좋을 것 같다는 생각이 들었습니다...!
현재는 요구사항대로 filled 상태에만 디자인이 변경되도록 구현했는데, 다른 분들은 어떻게 생각하시는지 궁금합니다!
🔗 관련 이슈