Skip to content
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

[week15] 배의진 #486

Conversation

BaeUiJin
Copy link

@BaeUiJin BaeUiJin commented Jun 1, 2024

변경사항

회원가입 페이지에서

  • 아이디, 비밀번호, 비밀번호 확인 input 입력 시 유효하지 않은 경우, 경고 텍스트 생성
  • 폼 제출 시, 서버로부터 아이디 중복 및 유효성 검사 리스폰스를 받음
    image

멘토님께

  • 혹시 코드 리뷰가 어려우시다면, 14주차 PR 은 그냥 merge 하고 이번 주차 것만 해주셔도 좋을 것 같습니다~!
  • 시간내주셔서 감사드립니다~

- next.js 템플릿에서 남아있던 파일들 완전 제거
- svg 파일을 ReactComponent 로 import 하기 위한 @svgr/webpack 라이브러리 설치
- 스타일이 적용 안되는 원인이었던 상대경로를 최상위 폴더로 수정 및 모듈 import 구문 전부 수정
- reset.css 파일을 _app.tsx 파일에서 import
@BaeUiJin BaeUiJin requested a review from kich555 June 1, 2024 05:10
@BaeUiJin BaeUiJin added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Jun 1, 2024
Copy link
Collaborator

@kich555 kich555 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다!!
form 다루는건 어떠셨나요? ㅎㅎ
form을 다루는게 조금 복잡할 수 있지만 리액트를 연습하기엔 좋은 주제인것 같으니
좀 더 고민해보면서 다뤄보시면 더 좋을것 같아요!!
참고로 uncontrolled Form / controlled Form의 개념에 대해 공부해보시는것도 좋을것 같습니다 ㅎㅎ

고생하셨습니다 !!

alt: "인스타그램 홈페이지로 연결된 인스타그램 로고",
};

export const logoImage = "images/linkbrary.svg";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이....건 굳이 상수 폴더에 있을 필요는 없을 것 같고 그냥 사용하는곳에서 import해오는게 좋아보입니다!

@@ -5,6 +5,45 @@ export const route = {
FAQ: "/faq",
};

export const text = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

기본적으로 상수의 네이밍 컨벤션은 상수스타일이라고
모든 문자를 대문자, 문자와 문자사이는 _로 네이밍 하는 규칙이 일반적입니다!
ex) TEXT, KAKAO, CONSTANT_NAME, 이런식으로요!

): Promise<boolean> => {
const address = "check-email";
try {
const response = await axiosInstance.post(address, { email: id });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이건 그냥

 const response = await axiosInstance.post('/check-email', { email: id });

이렇게 해주시면 될 것 같습니다 ㅎ

너무 변수를 남발하는 것도 ......굳이..라는 생각이 좀 드네요 😅

const address = "check-email";
try {
const response = await axiosInstance.post(address, { email: id });
return response.status === 200;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오잉..? 통신이 성공했는지 안했는지를 반환하나요?
옳은 방향...은 아닌것 같습니다 😅
response.data에 다른 정보는 혹시 오지 않는건가요?

export const checkAccountValid = async (
id: string,
pwd: string
): Promise<boolean> => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Promise<boolean> 같이 Promise 타입은 꽤나 많이 사용합니다 👍👍👍👍

id: "",
pwd: "",
confirmPwd: "",
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initialValue 따로 객체로 관리하는것도 좋네요 👍

};

export const Form = () => {
const [input, setInput] = useState(initialValue);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

input보다는 form이 보다 적합하지 않을까요? ㅎ


export const Form = () => {
const [input, setInput] = useState(initialValue);
const [message, setMessage] = useState(initialValue);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이것도 같이 form에서 개별 필드를 value와 message를 가진 객체로 만들면 굳이 상태를 2개로 나눌 필요는 없어보여요 ㅎ

const handleChange = (event: ChangeEvent<HTMLInputElement>) => {
const { name, value } = event.target;
setInput((prevInput) => ({ ...prevInput, [name]: value }));
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍👍👍

handleChange={handleChange}
checkInput={checkConfirmPwdInput}
/>
<ModalContentButton themeColor="blue">{text.signup}</ModalContentButton>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

button에 type='submit' 넣어주면 좋을것 같아요

@BaeUiJin BaeUiJin changed the base branch from main to part3-배의진 June 3, 2024 12:01
@kich555 kich555 merged commit ebffed5 into codeit-bootcamp-frontend:part3-배의진 Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants