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 #414

Merged
merged 19 commits into from
May 15, 2024

Conversation

SeungRyeolBaek
Copy link
Collaborator

요구사항

기본 요구사항
React 프로젝트에서 진행했던 작업물을 Next.js 프로젝트에 맞게 변경 및 이전해 주세요.
팀내 2 ~ 3명이 짝을 지어 페어프로그래밍으로 로그인, 회원가입 페이지에 사용하는 Input 컴포넌트를 만들어 주세요.
Vercel로 Github 레포지토리를 연결해 Next.js 프로젝트를 배포해 주세요.
Github에 위클리 미션 PR을 만들어 주세요.
React, Next.js를 사용해 진행합니다.

image
image

주요 변경사항

  • next.js사용
  • 아직 랜딩페이지, 로그인 페이지, 회원가입 안 만들었어요

멘토에게

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

@SeungRyeolBaek SeungRyeolBaek requested a review from kiJu2 May 12, 2024 06:50
@SeungRyeolBaek SeungRyeolBaek self-assigned this May 12, 2024
@SeungRyeolBaek SeungRyeolBaek added the 순한맛🐑 마음이 많이 여립니다.. label May 12, 2024
@SeungRyeolBaek SeungRyeolBaek changed the title [Week13] 백승렬 [백승렬] Week13 May 12, 2024
@kiJu2
Copy link
Collaborator

kiJu2 commented May 15, 2024

수고 하셨습니다 ! 위클리 미션 하시느라 정말 수고 많으셨어요.
학습에 도움 되실 수 있게 꼼꼼히 리뷰 하도록 해보겠습니다.

@kiJu2
Copy link
Collaborator

kiJu2 commented May 15, 2024

커밋 단위가 깔끔하군요 😊👍

역시 승렬님 ! 기본기가 훌륭합니다.

중간에 feat: 기본적인 코드 일단 전부 다 불러옴(refactoring 필요) cc0f79f 커밋에서 템플릿 변경사항들이 있기에 커밋 히스토리를 따라가면서 승렬님께서 작성하신 코드 위주로 코드리뷰를 진행하도록 하겠습니다 !

@@ -2,7 +2,7 @@ import { Html, Head, Main, NextScript } from 'next/document'

export default function Document() {
return (
<Html lang="en">
<Html lang="ko">
Copy link
Collaborator

Choose a reason for hiding this comment

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

굳굳. html도 잊지 않고 잘 설정해주셨군요 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

global.css를 포함하여 변수 css도 함께 작성하셨군요. 👍

또한 이번에는 sass를 사용하실건가 보군요 🤔

@@ -2,7 +2,7 @@ import { Html, Head, Main, NextScript } from 'next/document'

export default function Document() {
return (
<Html lang="ko">
<Html lang="en">
Copy link
Collaborator

Choose a reason for hiding this comment

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

다시 en으로 바뀌었군요 ! 한 번 확인해주세요 😊

import { useAsync, instance, mapFolderData } from "@/src/util";
import { SampleFolderRawData } from "@/src/type";

export const useGetFolder = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

크으 각 API 별로 커스텀 훅을 만드셨군요 👍👍

이렇게 되면 API 별 분기 로직 처리에 용이할거예요. 추 후 리액트 쿼리로 마이그레이션할 때에도 편리하겠네요 👍


export const useGetFolder = () => {
const getFolder = () =>
instance.get<{ folder: SampleFolderRawData }>("sample/folder");
Copy link
Collaborator

Choose a reason for hiding this comment

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

제네릭을 통하여 Response 타입을 적절하게 작성하셨군요 👍👍👍👍

여윽시 승렬쌤 👍

Comment on lines +4 to +6
export const useGetFolders = () => {
const getFolders = () =>
instance.get<{ data: FolderRawData[] }>("users/1/folders");
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
export const useGetFolders = () => {
const getFolders = () =>
instance.get<{ data: FolderRawData[] }>("users/1/folders");
export const useGetFolders = (userId: string = '1') => {
const getFolders = () =>
instance.get<{ data: FolderRawData[] }>(`users/${userId}/folders`);

const { loading, error, data } = useAsync(getFolders);

const folders = mapFoldersData(data?.data);
const sortedFolders = folders.sort((a, b) => a?.id - b?.id);
Copy link
Collaborator

Choose a reason for hiding this comment

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

sort는 비용이 높을 수 있으므로 메모이제이션을 통하여 최적화를 시켜볼 수 있습니다 😊:

Suggested change
const sortedFolders = folders.sort((a, b) => a?.id - b?.id);
const sortedFolders = useMemo(folders.sort((a, b) => a?.id - b?.id), [folders]);

Comment on lines +9 to +30
const snsLinks = [
{
href: "https://www.facebook.com/",
src: "images/facebook.svg",
alt: "facebook 홈페이지로 연결된 facebook 로고",
},
{
href: "https://twitter.com/",
src: "images/twitter.svg",
alt: "twitter 홈페이지로 연결된 twitter 로고",
},
{
href: "https://www.youtube.com/",
src: "images/youtube.svg",
alt: "youtube 홈페이지로 연결된 youtube 로고",
},
{
href: "https://www.instagram.com/",
src: "images/instagram.svg",
alt: "instagram 홈페이지로 연결된 instagram 로고",
},
];
Copy link
Collaborator

Choose a reason for hiding this comment

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

굳굳. 컴포넌트 매핑을 사용하셨군요 👍

반복된 데이터를 효율적으로 처리할 수 있는 패턴이죠 😊👍

Comment on lines +19 to +27
/**
* @component
* @param {Object} props - 컴포넌트의 props입니다.
* @param {string} props.value - 텍스트 입력의 현재 값입니다.
* @param {Function} props.onChange - 입력 값 변경을 처리하는 콜백 함수입니다.
* @param {Function} props.onSubmit - 폼 제출을 처리하는 콜백 함수입니다.
* @param {React.Ref<HTMLFormElement>} ref - 폼 요소에 전달할 ref입니다.
* @returns {JSX.Element} 사용자가 링크 추가하기 위해 해당 입력 칸에 url작성하는 그러한 컴포넌트
*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

깔끔한 jsdoc입니다 👍👍

다만, typescript를 사용하실 경우 jsdoc의 타입 지정은 불필요합니다 !:

Suggested change
/**
* @component
* @param {Object} props - 컴포넌트의 props입니다.
* @param {string} props.value - 텍스트 입력의 현재 값입니다.
* @param {Function} props.onChange - 입력 변경을 처리하는 콜백 함수입니다.
* @param {Function} props.onSubmit - 제출을 처리하는 콜백 함수입니다.
* @param {React.Ref<HTMLFormElement>} ref - 요소에 전달할 ref입니다.
* @returns {JSX.Element} 사용자가 링크 추가하기 위해 해당 입력 칸에 url작성하는 그러한 컴포넌트
*/
/**
* @component
* @param props - 컴포넌트의 props입니다.
* @param props.value - 텍스트 입력의 현재 값입니다.
* @param props.onChange - 입력 변경을 처리하는 콜백 함수입니다.
* @param props.onSubmit - 제출을 처리하는 콜백 함수입니다.
* @param ref - 요소에 전달할 ref입니다.
* @returns 사용자가 링크 추가하기 위해 해당 입력 칸에 url작성하는 그러한 컴포넌트
*/

import { KeyboardEventHandler, MouseEventHandler } from "react";
import { KakaoIcon, FacebookIcon, LinkIcon } from "./constant";
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 파일들은 상수(constant)로 보기 어려울 것 같아요 !:

icons와 같은 파일을 따로 만드시는게 어떨까요?

Comment on lines +6 to +9
/**
*
* @returns 폴더에 저장된 링크가 없을 시 디폴트로 띄우는 화면
*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

jsdoc활용 👍👍👍

@kiJu2
Copy link
Collaborator

kiJu2 commented May 15, 2024

안녕하세요 승렬님 ! 오랜만에 보는 승렬님 코드군요 ㅎㅎㅎ
이번 코드리뷰에서는 템플릿 코드로 추정되는 commitfeat: 기본적인 코드 일단 전부 다 불러옴(refactoring 필요)를 제외한 코드들을 위주로 리뷰를 작성하였습니다 !

혹시 궁금하신 점이나 더욱 살펴봤음 하는 부분 있다면 사전 질의를 통해 남겨주세요 ! 😊

@kiJu2 kiJu2 merged commit 6e0fb52 into codeit-bootcamp-frontend:part3-백승렬 May 15, 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