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

Part2 장성훈 week7 #289

Conversation

hoonj170214
Copy link

요구사항

상단 네비게이션 바, 푸터는 랜딩 페이지와 동일한 스타일과 규칙으로 만들어 주세요. (week 1 ~ 3 요구사항 참고)

기본

카드 컴포넌트를 클릭하면 해당하는 링크로 새로운 창을 띄워서 이동하게 해주세요.
카드 컴포넌트에서 createdAt 데이터 기준으로 현재 Date와 차이에 따라 아래와 같은 규칙으로 설정해 주세요.
2분 미만은 “1 minute ago”
59분 이하는 “OO minutes ago”
60분 이상은 “1 hour ago”
23시간 이하는 “OO hours ago”
24시간 이상은 “1 day ago”
30일 이하는 “OO days ago”
31일 이상은 “1 month ago”
11달 이하는 “OO months ago”
12달 이상은 “1 year ago”
OO달 이상은 “{OO/12(소수점 버린 정수)} years ago”
프로필 데이터가 없는 경우 “로그인” 버튼이 보이게 해주세요.
폴더 소유자, 폴더 이름 영역, 링크들에 대한 데이터는 “/api/sample/folder”를 활용해 주세요.

(여기서 폴더란 링크들의 정보가 저장되어 있는 객체를 의미합니다. shared 페이지는 외부 유저에게 자신의 폴더 데이터 하나를 공유할 때 유저가 보게되는 화면 입니다.

디자인 시안에서 “프로필 이미지”는 폴더 소유자의 프로필, “@코드잇”은 폴더 소유자 이름, “⭐️ 즐겨찾기”는 폴더 이름이에요.)

멘토에게

베포링크 : https://hoonj170214.github.io/

  • 멘토링 시간에 코드리뷰 해주셨으면 좋겠습니다.

  • 5분전 같은 것은 어떻게 구현해야 하는지 잘 모르겠습니다..

  • inputheader 위에 겹치는데, 이게 왜 그런지 잘 모르겠습니다..

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

@hoonj170214 hoonj170214 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Mar 31, 2024
@devToram devToram self-requested a review March 31, 2024 14:50
@devToram devToram assigned hoonj170214 and unassigned devToram Mar 31, 2024
@devToram
Copy link
Collaborator

저는 Assignees 가 아닌 reviewer 로 추가해주셔야해요!!

Copy link
Collaborator

@devToram devToram left a comment

Choose a reason for hiding this comment

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

5분전 같은 것은 어떻게 구현해야 하는지 잘 모르겠습니다..

힌트를 드리면 date-fns 와 같은 라이브러리를 한 번 사용해서 구현해보시는 것을 추천드립니다!

input 이 header 위에 겹치는데, 이게 왜 그런지 잘 모르겠습니다..

input 을 감싸고 있는 친구만 postion 을 relative 속성으로 주셔서 그런 거 같아요!
전체 코드 중에서 position 을 지정해준 친구들 끼리만 z-index 를 비교하는데, 따로 z-index 는 설정해주시지 않았고, Input 을 더 나중에 선언해주었으니 input 이 header 위에 겹치게 되는 것 같습니다! 해결법으로는 z-index 를 header 에 설정해주시면 될 거 같아요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

파일의 이름은 url 인데, 실제 내부에 있는 url 은 path 나 url 과 상관없는 거 같아요!
시맨틱한 파일이름을 지으면 좋을 거 같습니다
그리고 apis 도 src 내부에 있는 게 맞을 거 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

아하! 감사합니다! 👍

Copy link
Author

Choose a reason for hiding this comment

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

시맨틱한 내용으로 수정하겠습니다! 👍
apis 도 src 내부로 수정했습니다! 👍

@@ -1,17 +1,11 @@
import React 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.

.jsx 를 쓰지 않으시고 .js 를 쓰셨는데 크게 성능이나 기능상에 차이는 없지만 JSX 를 리턴하는 파일은 보통 .jsx 를 써요!
(util 과 같은 함수들이나 custom hook 은 보통 .js 를 씀)
해당 부분에 대해서 찾아보심 좋을 거 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

유틸함수, 커스텀훅은 js 를 쓰고, 화면을 그리는 파일은 jsx를 쓰는군요!
수정했습니다 감사합니다! 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

src/assets 에 있는 파일들을 /public 으로 옮겨주셔요!
그리고 assets 이름은 특별한 이유가 없는 한 대문자로 시작하는 파스칼케이스 네이밍은 지양해주세요!
(리액트에서는 컴포넌트만 대문자로 시작합니다)

Copy link
Author

Choose a reason for hiding this comment

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

컴포넌트만 대문자 시작! 감사합니다! 수정했습니다! 👍


useEffect(() => {
fetchData(BASE_URL);
}, [BASE_URL]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

BASE_URL 은 상태값이 아닌 상수값이기 때문에 dependency 배열에 넣어도 안넣은 것과 동일하게 맨 처음 Folder 가 렌더링될 때만 실행됩니다! 제거해주세요!

Suggested change
}, [BASE_URL]);
}, []);

Copy link
Author

Choose a reason for hiding this comment

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

상수는 변하지 않으니까, dependency 배열에 안넣어도 되는군요!
수정했습니다 감사합니다! 👍

useEffect(() => {
fetchData(BASE_URL);
}, [BASE_URL]);
console.log(data);
Copy link
Collaborator

Choose a reason for hiding this comment

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

컴포넌트 내부에 console 을 찍으시면 렌더링탈 때마다 계속 출력되기에 디버깅용이시라면 올리실 땐 제거해주세요!

Suggested change
console.log(data);

Copy link
Author

Choose a reason for hiding this comment

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

제가 미처 못봤네요.. 수정했습니다 감사합니다 👍

@@ -0,0 +1,11 @@
import './App.css';
import Folder from '../pages/folder/Folder';
function App() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

App 은 따로 공통되게 사용되는 것이 아니기에 필요없는 파일인 거 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

멘토링때 말씀해주신 것 처럼 '역할이 없는 파일들은 제거!'
수정했습니다 감사합니다 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 파일은 JSX 를 리턴하는 파일이 아닌 커스텀 훅 파일이기에 .js 가 맞을 거 같습니다

Copy link
Author

Choose a reason for hiding this comment

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

커스텀훅은 js로!
수정했습니다 감사합니다! 👍

<Desc>
<Ago>dateNow - 'createdAt'</Ago>
<Content>
{desc.length > 10 ? desc.substr(0, 70) + '...' : desc}
Copy link
Collaborator

Choose a reason for hiding this comment

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

로직을 return 부에 담게되면 계속 연산을 하게 되어서 비추천드립니다!
useMemo 로 위로 빼서 변수로 가져오시는 걸 추천드려요!

Suggested change
{desc.length > 10 ? desc.substr(0, 70) + '...' : desc}
{desc.length > 10 ? subDesc : desc}

Copy link
Author

Choose a reason for hiding this comment

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

조건부 렌더링을 하려고 했던 의도였는데,
로직 자체를 return 안에 담게 되면, 계속 연산을 하게 되는군요!!
수정했습니다 감사합니다!! 👍

@devToram devToram merged commit a0bb228 into codeit-bootcamp-frontend:part2-장성훈 Mar 31, 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