-
Notifications
You must be signed in to change notification settings - Fork 40
[김태완] Sprint 11 #330
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
[김태완] Sprint 11 #330
The head ref may contain hidden characters: "React-\uAE40\uD0DC\uC644-sprint11"
Conversation
jyh0521
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.
고생하셨습니다~
| import FAQ from "./components/FAQ"; | ||
| import LoginPage from "./page/LoginPage"; | ||
| import RegisterPage from "./page/RegisterPage"; | ||
| import { useSelector } from "react-redux"; |
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.
api 함수들이 점점 늘어나는데 각 함수마다 하나의 파일로 만들어두셔도 좋을 것 같습니다.
| function Header() { | ||
| const [isDropdownVisible, setIsDropdownVisible] = useState(false); | ||
| const dispatch = useDispatch(); | ||
| const count = useSelector((state: any) => state.counter.value); |
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.
state의 타입을 지정해줄 수 있으면 지정해보셔도 좋을 것 같습니다.
| const toggleDropdown = () => { | ||
| setIsDropdownVisible((prev) => !prev); | ||
| }; |
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, | ||
| }: InputFieldProps) => { | ||
| const [isPasswordVisible, setIsPasswordVisible] = useState<boolean>(false); | ||
| const InputField = forwardRef<HTMLInputElement, InputFieldProps>( |
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.
forwardRef 잘 사용해주셨는데, 혹시 ref를 사용하는 경우가 있나요? 이번 PR에서는 안보이는 것 같아서요.
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.
아래 input 태그에서 ref를 사용했습니다!
| <div className="input-pass-box"> | ||
| <input | ||
| ref={ref} | ||
| className={`input-tag ${error ? "input-error" : ""}`} |
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.
clsx 같은 라이브러리를 활용해보시는 것도 좋을 것 같습니다.
요구사항
기본
심화
주요 변경사항
멘토에게