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

[김주동] sprint8 #109

Conversation

joodongkim
Copy link
Collaborator

요구사항

기본

  • [x]스프린트 미션 1 ~ 7에 대해 typescript를 적용해주세요
  • []
  • []

심화

  • [x]any타입을 최소한으로 써주세요
  • []

주요 변경사항

스크린샷

image

멘토에게

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

@joodongkim joodongkim added the 순한맛🐑 마음이 많이 여립니다.. label Oct 18, 2024
@joodongkim joodongkim requested a review from GANGYIKIM October 18, 2024 15:20
@GANGYIKIM GANGYIKIM changed the title React 김주동 sprint8 [김주동] sprint8 Oct 21, 2024
Copy link
Collaborator

@GANGYIKIM GANGYIKIM 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,44 @@
// 날짜 관련 Util Functions

// Javascript에서 날짜를 다룰 때는 library 사용을 추천해요.
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
동의합니다 👍

case "email":
if (!trimmedValue) return "이메일을 입력해 주세요";
if (!isValidEmail(trimmedValue)) return "잘못된 이메일 형식입니다";
return "";
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
case 별로 에러 문구를 반환하도록 하신 점 좋습니다~
다만 아래처럼 default error message를 정해주시면 좋지 않을까싶네요~

Suggested change
return "";
return "입력값을 다시 확인해주세요"; // 문구는 일반적인 에러 케이스를 포함할 수 있으면 좋겠습니다.

Comment on lines +24 to +39
interface ErrorState {
email?: string;
nickname?: string;
password?: string;
passwordConfirmation?: string;
}

const SignupPage: React.FC = () => {
// 상태 관리를 위해 하나의 object로 묶어주었어요.
const [formState, setFormState] = useState<FormState>({
email: "",
nickname: "",
password: "",
passwordConfirmation: "",
});
const [errors, setErrors] = useState<ErrorState>({});
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:
FormState와 대응하는 error 값으므로 따로 type을 정의하기보다 아래처럼 사용하시는것이
의도를 더 잘 나타내고 유지보수하기도 좋습니다~

Suggested change
interface ErrorState {
email?: string;
nickname?: string;
password?: string;
passwordConfirmation?: string;
}
const SignupPage: React.FC = () => {
// 상태 관리를 위해 하나의 object로 묶어주었어요.
const [formState, setFormState] = useState<FormState>({
email: "",
nickname: "",
password: "",
passwordConfirmation: "",
});
const [errors, setErrors] = useState<ErrorState>({});
const SignupPage: React.FC = () => {
// 상태 관리를 위해 하나의 object로 묶어주었어요.
const [formState, setFormState] = useState<FormState>({
email: "",
nickname: "",
password: "",
passwordConfirmation: "",
});
const [errors, setErrors] = useState<�Partial<FormState>>({});

https://www.typescriptlang.org/docs/handbook/utility-types.html#partialtype

e: FocusEvent<HTMLInputElement | HTMLTextAreaElement>
) => {
const { id, value } = e.target;
const errorMessage = getErrorMessage(id as LoginInputId, value);
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
as 보다 다른 방법을 쓰시면 좋겠어요~
id는 e.target의 속성으로 string이니 이를 어떻게 해야 원하시는 값- "email" | "nickname" | "password" | "passwordConfirmation" 중 하나-로 받을 수 있을지 고민해보시면 좋겠습니다!


<BestItemsCardSection>
{itemList?.map((item) => (
<ItemCard item={item} key={`best-item-${item.id}`} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
key값은 일반적으로 map을 돌릴때 꼭 필요한 값이니 앞에 두시는 것을 추천합니다~

Suggested change
<ItemCard item={item} key={`best-item-${item.id}`} />
<ItemCard key={`best-item-${item.id}`} item={item} />

Comment on lines +48 to +52
if (error instanceof Error) {
setError(error.message);
} else {
setError("알 수 없는 오류가 발생했습니다.");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
👍

Comment on lines +3 to +10
interface FeatureProps {
image: string;
alt: string;
featureName: string;
title: string;
description: string;
direction?: "row" | "row-reverse";
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
FeatureProps 값들은 크게 3가지로 구분할 수 있습니다.
FeatureContainer에서 사용되는 direction, img 태그에서 사용되는 image와 alt, 그외입니다~

   <FeatureContainer direction={direction}>
      <img src={image} alt={alt} />
      <FeatureContent>
        <h2>{featureName}</h2>
        <h1>{title}</h1>
        <p className="feature-description">{description}</p>
      </FeatureContent>
    </FeatureContainer>

타입을 정의하실 때도 이가 나타나게 정의하시면 더 좋습니다.

type DirectionType =  "row" | "row-reverse";
interface FeatureContentProps {
  featureName: string;
  title: string;
  description: string;
}

interface FeatureProps extends FeatureContentProps {
  imageProps: ImgHTMLAttributes<HTMLImageElement>; // img 태그의 속성값을 다 넘길 수 있으며 이를 보고 어떻게 사용되는 값인지 타입만으로 알 수 있음
  direction?: DirectionType;
}

const FeatureContainer = styled.div<{ direction?: DirectionType }>`

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

function useDebounce<T>(value: T, delay: number): T {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
제너릭 타입 쓰신거 좋습니다 👍

}) => {
const [isVisible, setIsVisible] = useState(isLoading);

// 로딩이 너무 빨라서 로딩 스피너가 순간적으로 나타났다 사라지는 것을 방지하기 위해 설정된 최소시간 동안은 스피너가 떠있도록 했어요.
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
👍

label: string;
}

const DeleteButton: React.FC<DeleteButtonProps> = ({ onClick, label }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
아래처럼 작성하시면 button 태그의 속성을 그대로 타입으로 이용할 수 있습니다.

Suggested change
const DeleteButton: React.FC<DeleteButtonProps> = ({ onClick, label }) => {
const DeleteButton: React.FC<ButtonHTMLAttributes<HTMLButtonElement>> = ({ onClick, label }) => {

@GANGYIKIM GANGYIKIM merged commit b8cce04 into codeit-bootcamp-frontend:React-김주동 Oct 22, 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