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

[조유담] week13 #496

Conversation

youdame
Copy link
Collaborator

@youdame youdame commented Dec 3, 2023

요구사항

기본

  • ‘/folder’ 페이지를 Next.js 프로젝트에 맞게 변경 및 이전 했나요?

  • ‘/shared’ 페이지를 Next.js 프로젝트에 맞게 변경 및 이전 했나요?

  • 다른 페이지로 이동이 필요한 곳에 next/link의 Link를 적용했나요?

  • [] Input 컴포넌트에 값이 없는 경우 회색의 placeholder값을 볼 수 있나요?

  • Input 컴포넌트에 focus in 하면 파랑색 테두리를 볼 수 있나요?

  • Input 컴포넌트에 눈 모양 아이콘을 누르면 비밀번호 가리기/보기 기능이 토글 되나요?

-[] Input 컴포넌트에 값이 에러케이스일 경우 빨강색 테두리와 에러 메세지를 볼 수 있나요?

-[] Input 컴포넌트에서 focus out 하면 실행할 함수를 설정할 수 있나요?

심화

  • [x]
  • []

주요 변경사항

스크린샷

image

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@youdame youdame requested a review from InSeong-So December 3, 2023 14:23
Copy link
Collaborator

@InSeong-So InSeong-So left a comment

Choose a reason for hiding this comment

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

수고 많으셨어요 🥹
어느 부분을 수정했는지만 이야기해주면 리뷰하는 게 좀 더 편해질 것 같네요.
대부분의 케이스는 공통 리뷰로 풀어드릴 수 있을 것 같습니다 🙇‍♂️

@@ -0,0 +1,19 @@
import { ReactNode, useEffect, useState } from "react";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Next.js useLayoutEffect를 제어하는 방식이 있답니다!
요건 공유해드릴게요 🤗

Comment on lines +2 to +3
import styles from "./ImageWrapper.module.scss";
type ImageWrapperProps = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

import과 본문은 한 칸 띄우기...! 가독성이 좋아져요.

className: string;
};

export const ImageWrapper = ({ children, className }: ImageWrapperProps) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

React Type과 DOM Element Type을 잘 섞을 수 있답니다!

Comment on lines +53 to +56
const handleKebabClick: MouseEventHandler<HTMLButtonElement> = (event) => {
event.preventDefault();
setIsPopoverOpen(() => (isPopoverOpen ? false : true));
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const handleKebabClick: MouseEventHandler<HTMLButtonElement> = (event) => {
event.preventDefault();
setIsPopoverOpen(() => (isPopoverOpen ? false : true));
};
const handleKebabClick: MouseEventHandler<HTMLButtonElement> = (event) => {
event.preventDefault();
setIsPopoverOpen(() => !isPopoverOpen);
};

@InSeong-So InSeong-So merged commit e368f60 into codeit-bootcamp-frontend:part3-조유담 Dec 12, 2023
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