[Feature] SearchBar 컴포넌트 구현 #83
Conversation
SeieunYoo
requested review from
ghdtjgus76,
eugene028,
hamo-o and
kongnayeon
as code owners
There was a problem hiding this comment.
수고하셨어요!!
키보드 입력에 따라 값이 계속해서 달라지는 SearchBar 같은 경우에는 제어 컴포넌트와 비제어 컴포넌트 양쪽으로 사용될 수 있을 것 같은데요! (자동완성 지원 여부나 유효성 검사 여부에 따라..) 이러한 경우를 대비해서 value 값의 존재여부로 제어되고 있는지 체크하고 그렇지 않으면 아예 상태 업데이트를 막는것도 괜찮을 것 같아요.
그런데 이게 다른 컴포넌트에서도 마찬가지일 것 같은데.. 제가 정확한 개념을 몰라서 지금까지 이 점을 고려 못한 것 같네요....?! 텍스트필드에서도 고려해야 할 부분인 것 같은데 다른 분들 어떻게 생각하시나용
@eugene028 @ghdtjgus76
|
찾아보니 다른 라이브러리도 value 에 따라 isControlled 라는 불리언으로 controlled 상태인지 확인하는 로직이 들어가있는 것 같아용 |
| * @param {(value: string) => void} [onChange] 외부 활성 상태가 변경될 때 호출될 콜백 함수. | ||
| * @param {() => void} [onBlur] 서치바가 포커스를 잃을 때 호출될 콜백 함수. | ||
| * @param {() => void} [onFocus] 서치바가 포커스됐을 때 호출될 콜백 함수. | ||
| * @param {InputHTMLAttributes<HTMLInputElement>} [inputProps] 서치바에 전달할 추가 textarea 속성. |
There was a problem hiding this comment.
p5;
textarea 부분만 수정 부탁드려요~
| onFocus={handleFocus} | ||
| {...rest} |
There was a problem hiding this comment.
p3;
rest 부분이 input이 아니라 가장 바깥에 있는 Flex에 적용되어야 하지 않을까 합니다,,
| const [variant, setVariant] = useState<VariantType | BaseVariantType>( | ||
| "default" | ||
| ); | ||
|
|
There was a problem hiding this comment.
P3;
제어 컴포넌트로 조금 더 활용하기 위해서는 외부에서 새로운 valueProp이 들어와 props가 변경되었을때 이를 value라는 state에 반영해줄 수 있어야 할 것 같아요!
| useEffect(() => { | |
| if (valueProp !== undefined) { | |
| setValue(valueProp); | |
| } | |
| }, [valueProp]); |
그래야 요런 것도 가능할것 같습니다용
There was a problem hiding this comment.
비제어/제어 에 따라 비제어일 때는 내부 상태 관리 value 값으로, 제어일 때는 외부에서 전달받은 value 값으로만 제어되도록 수정해두면서 요 부분도 해결되었습니다! be6712e
| @@ -0,0 +1,144 @@ | |||
| import { fireEvent, render, waitFor } from "@testing-library/react"; | |||
There was a problem hiding this comment.
P5;
rtl에서 fireEvent보다는 userEvent를 추천하고 있어서 조금 더 정확한 테스트를 위해서는 userEvent로 마이그레이션 하는 것을 추천드려용 >.<
There was a problem hiding this comment.
굿굿 userEvent 로 마이그레이션 해두었어요~!
맞아욥 왼쪽이나 오른쪽에 텍스트나 다른 아이콘 같은 옵션이 올 수 있도록 열린다면 Input 도 좋을 것 같아서 디자이너분께 여쭈어볼게요! 코드 리뷰들은 반영해두었습니당~! @eugene028 @ghdtjgus76 |
|
추가적으로 스펙이 수정된다면 다른 PR 에서 반영 해볼게요 요건 머지하겠습니다! |
🎉 변경 사항
텍스트필드랑 비슷한 부분이 많아서 Base 컴포넌트를 만들까,,훅을 만들까,, 고민을 하다가 텍필은 textarea 이기도 하고 추후에다른 input 컴포넌트가 생길 수도 있을 것 같아서 value 와 variant 를 핸들링 할 수 있는 useFormControl 훅을 만들어봐씀당
기본 variant 타입은 "default" | "typing" | "typed" 이고 컴포넌트에서 base variant type 을 확장해서 쓸 수 있도록 지원합니다.
🚩 관련 이슈
#84
🙏 여기는 꼭 봐주세요!