Skip to content

이희선 11주차 과제 제출#91

Open
hseon2 wants to merge 1 commit intosnulion11th-seminar:heeseonfrom
hseon2:main
Open

이희선 11주차 과제 제출#91
hseon2 wants to merge 1 commit intosnulion11th-seminar:heeseonfrom
hseon2:main

Conversation

@hseon2
Copy link

@hseon2 hseon2 commented Jun 2, 2023

No description provided.

Copy link

@whwoohw whwoohw left a comment

Choose a reason for hiding this comment

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

잘 작성하셨습니다!
간단한 의문점이나 수정하면 좋을만한 사항들 적어놨으니
코멘트 달아주시거나 수정할 부분 수정해서 merge까지 해주세요!

Comment on lines +7 to +10
const [email, setEmail] = useState("");
const [username, setUsername] = useState("");
const [college, setCollege] = useState("");
const [major, setMajor] = useState("");
Copy link

Choose a reason for hiding this comment

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

굳이 여러 state를 쓰지말고, 하나의 formdata에 넣는게 효율적으로 보입니다!

Comment on lines +27 to +30
setEmail(info.user.email);
setUsername(info.user.username);
setCollege(info.college);
setMajor(info.major);
Copy link

Choose a reason for hiding this comment

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

Suggested change
setEmail(info.user.email);
setUsername(info.user.username);
setCollege(info.college);
setMajor(info.major);
set�FormData({
email : info.user.email,
username: info.user.username,
college : info.college,
major : info.major,
});

위에서 이어진 내용으로, 저라면 이렇게 할 것 같습니다!

Comment on lines +15 to +17
useEffect(() => {
setContent(infoContent);
}, [infoContent]);
Copy link

Choose a reason for hiding this comment

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

이 부분이 왜 있는지 설명해주실 수 있나요?

const handleEditInfo = (e) => {
e.preventDefault();
updateInfo({ [infoLabel]: content });
setContent(infoContent);
Copy link

Choose a reason for hiding this comment

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

여기서 setContent를 한 이유가 무엇인가요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants