-
Notifications
You must be signed in to change notification settings - Fork 1
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: Textarea 컴포넌트 구현 #150
Conversation
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.
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.
@nijuy 스크롤바 적용 완료하였습니다! 이게 스크롤바 적용이 패딩값을 무시하고 textarea 전체에 맞게 스크롤바가 생겨서 StyledTextareaWrapper
로 감싸서 패딩값 적용된 컨테이너 안에 스크롤바가 생긴 형태(피그마와 같은 형태)로 작업하였습니다!
윈도우 환경 확인 부탁드려욥!
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.
placeholder?: string; | ||
disabled?: boolean; | ||
error?: boolean; | ||
value?: string; |
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.
음 사실 value를 prop으로 정한 이유가 사용자가 value를 자신이 정한 value 값을 prop으로 넣어서 사용할 수 있도록
하려는 의도 였습니다!!
생각해보니 내부에서 제어(usestate)를 하게 되니 적용이 되지 않겠네요..
음 value와 onChage를 외부에서 prop으로 제어하도록
하는게 맞는 방법인지, 내부에서 상태 관리
를 하는게 맞는 방법인지 고민고민,,, 입니당....
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.
생각해보니 내부에서 제어(usestate)를 하게 되니 적용이 되지 않겠네요..
사실 제가 입력값이 화면에 반영이 안돼요
라고 표현했던 경우는 사용처에서 잘못 사용한 거긴 해요.
(onValueChange
를 넘기지 않고 value
만 설정했음)
value
: A string. Controls the text inside the text area.When you pass value, you must also pass an onChange handler () that updates the passed value. - https://react.dev/reference/react-dom/components/textarea
value
prop을 전달할 거면 onValueChange
도 같이 주거나 readOnly
로 설정하는 게 올바른 사용이고,
그렇게 하지 않았을 때 리액트에서 사진과 같은 콘솔 경고를 띄워주는데
지금 Textarea
에는 onChange={handleChange}
가 있으니까 value
만 넘겨도 경고가 안 뜨는 점이 좀 걸렸어요 저는!
(어쨌든 onValueChange
가 없어서 문제가 발생해도 경고가 뜨지 않으니 잘못된 사용을 단번에 알아차리기 어렵다고 생각)
지금처럼 value prop을 전달할 수 있게 갈 거라면 적절한 예외 처리 + 문서 수정이 필요하다고 생각합니다 @.@
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.
사용하는 입장에서 가장 편한 방식은
- 내부에서 상태 관리
- 외부(사용하는곳)에서는
onValueChange
이벤트를 사용해서 내부 value를 받아올 수 있도록
하는거라고 생각해요
이 방법을 사용하는 가장 대표적인 라이브러리가 react-hook-form
이긴하져
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.
value prop 삭제하는 방안 으로 갔습니다!
- 내부에서 상태 관리
- 외부(사용하는곳)에서는
onValueChange
이벤트를 사용해서 내부 value를 받아올 수 있도록
이 방식으로 수정했습니다 확인 부탁드려욥!
(plaholder, disabled, maxLength)
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.
굿임니다!! 👍
<StyledHelperText $error={error}>
{helperText && helperText} {maxLength && `(${currentLength}/${maxLength})`}
</StyledHelperText>
논의한 방식은 위 로직보다는 아래 로직쪽에 가깝긴해요
<StyledHelperText $error={error}>
{maxLength ? `(${currentLength}/${maxLength})` : helperText && helperText}
</StyledHelperText>
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.
윈도우 스크롤 추가랑 helperText 로직만 변경되면 어프룹 때릴게요
굿굿
1️⃣ 어떤 작업을 했나요? (Summary)
2024-08-21.5.30.09.mov
2️⃣ 알아두시면 좋아요!
최대 글자 수를 설정했을 때 helperText 부분을
사용자가 넣고 싶은 문구 옆에 (현재 글자 수 / 최대 글자 수) 텍스트
로 넣어두었습니다.사용자가 helperText 문구를 따로 넣지 않고 최대 글자 수를 설정하면 (현재 글자 수 / 최대 글자 수)만 나오는 상태로 자유롭게 설정을 해두었습니다.
@fecapark 이 구현하신 textField와 맞춰서 추후 수정 작업하겠습니다! 확인해주시고 의견 주시면 감사하겠습니다ㅎㅎ
+++
문서 작업이나 디자인 시스템 작업이 처음이라 "이렇게 문서 작성하면 좋을 것 같다.", "코드 작성할 때는 이런 걸 고려하면서 작성해야 한다" 등 마음껏 알려주시면 제가 열심히 깊이 새겨들으며 차차 성장해나갈 수 있도록 하겠습니다👊🏻🔥
3️⃣ 추후 작업
리뷰 반영하기
째깍째깍 작업하기..
4️⃣ 체크리스트 (Checklist)
main
브랜치의 최신 코드를pull
받았나요?