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

[조혜진] Week7 #272

Merged
merged 36 commits into from
Apr 1, 2024
Merged

[조혜진] Week7 #272

merged 36 commits into from
Apr 1, 2024

Conversation

MEGUMMY1
Copy link
Collaborator

요구사항

기본

  • 상단 네비게이션 바, 푸터가 랜딩 페이지와 동일한 스타일과 규칙으로 만들어졌나요? (week 1 ~ 3 요구사항 참고)
  • 상단 네비게이션 바에 프로필 영역의 데이터는 https://bootcamp-api.codeit.kr/docs 에 명세된 “/api/sample/user”를 활용했나요?
  • 상단 네비게이션 바에 프로필 영역의 데이터가 없는 경우 “로그인” 버튼이 보이도록 만들었나요?
  • 폴더 소유자, 폴더 이름 영역, 링크들에 대한 데이터는 “/api/sample/folder”를 활용했나요?
  • Static, no image, Hover 상태 디자인을 보여주는 카드 컴포넌트를 만들었나요?
  • Hover 상태에서 이미지가 기본 크기의 130%로 커지나요?
  • 카드 컴포넌트를 클릭하면 해당 링크로 새로운 창을 띄워서 이동하나요?
  • Tablet에서 카드 리스트가 화면의 너비 1124px를 기준으로 이상일 때는 3열로 작을 때는 2열로 배치되나요?
  • Tablet, Mobile에서 좌우 최소 여백은 32px 인가요?

심화

  • 커스텀 hook을 만들어 활용했나요?

주요 변경사항

  • html 프로젝트 React로 리팩터링

스크린샷

image
image
image
image

멘토에게

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

@MEGUMMY1 MEGUMMY1 requested a review from 13akstjq March 27, 2024 09:12
@MEGUMMY1 MEGUMMY1 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Mar 27, 2024
@@ -0,0 +1,26 @@
// Article.js
import { useFetch } from './hooks/useFetch';
Copy link
Collaborator

Choose a reason for hiding this comment

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

경로 잘못 지정된 것 같습니다~!

@@ -0,0 +1,26 @@
// Article.js
import { useFetch } from './hooks/useFetch';
import '../style.css';
Copy link
Collaborator

Choose a reason for hiding this comment

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

style.css에서 모든 css를 작성하기 보다는 컴포넌트별로 css 파일을 작성하는 것이 좋습니다.
react의 css module에 대해서 알아보시면 컴포넌트 별 스타일을 작성하시는데 도움이 되실거에요!

const folderData = useFetch('https://bootcamp-api.codeit.kr/api/sample/folder');

// 날짜 형식 변경 함수
const formatDate = (dateString) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

날짜 형식에 대한 함수는 공통으로 사용될 수 있으니 utils 폴더를 만들어 작성하면 좋을 것 같습니다~!

// Cards.js
import { useFetch } from '../hooks/useFetch';
import { Link } from 'react-router-dom';
import moment from 'moment';
Copy link
Collaborator

Choose a reason for hiding this comment

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

moment를 사용하셨네요 잘하셨습니다!

const months = Math.floor(timeDiff / (60 * 24 * 30));
return months === 1 ? '1 month ago' : `${months} months ago`;
}
const years = Math.floor(timeDiff / (60 * 24 * 30 * 12));
Copy link
Collaborator

Choose a reason for hiding this comment

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

60, 24, 30, 12 는 상수로 작성해서 사용하면 더 알기 쉬울 것 같습니다!

{generateTimeText(link.createdAt)}
</p>
</div>
<div className="card_txt_div_body">
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기도 굳이 p태그를 감싸야할 이유가 있으신가요??
불필요한 태그 중첩은 가독성이 오히려 떨어지기 때문에 필요한 요소만 작성해주시는 것이 좋습니다!

return (
<header className="navbar">
<Link to="/" className="logo">
<img src={logo} width="133" height="24" alt="logo" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

logo 보다는 "Linkbrary" 텍스트를 넣어주는 것이 접근성 측면에서 더 좋겠죠??
이 내용도 멘토링 시간에 얘기나눌게요!

// useFetch.js
import { useState, useEffect } from 'react';

export function useFetch(url) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

데이터를 가져오는 훅을 잘 만들으셨네요! 👍

import '../style.css';

function Article() {
const folderData = useFetch('https://bootcamp-api.codeit.kr/api/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.

baseurl 을 constants 폴더를 만들어 상수로 분리해서 사용하면 좋을 것 같습니다!

<link rel="apple-touch-icon" href="%PUBLIC_URL%/logo192.png" />
<link rel="manifest" href="%PUBLIC_URL%/manifest.json" />
<title>Linkbrary</title>
``````
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 텍스트는 왜 들어간걸까요??🤔

@13akstjq 13akstjq merged commit 42c26cb into codeit-bootcamp-frontend:part2-조혜진 Apr 1, 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.

5 participants