2조 과제 제출(방준우, 배현수, 오혜성, 이혜란, 전소윤) #5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- detail 컴포넌트 안에 Comments 컴포넌트 추가 - 작업시 확인 용도로 main.tsx, App.tsx 수정
Feat : create Header
NotFound 페이지 작업 , 패키지json 수정
Upadate file
Feat: create view count
feat-search-page
feat: Css styling, modify comments component
Fix error, Update getViewCount
feat-search-page-styles
Update key, Fix svg
DevelopEdit: Edit WideSideBar
GyoHeon
reviewed
Jan 22, 2023
There was a problem hiding this comment.
수고하셨습니다!
리액트를 이용해서 아주 멋진 협업을 이루어내셨습니다.
이슈(해결하신건지 모르겠습니다.)
- 파일명 소문자로 작성 및 배포 후에 파일명을 대문자로 변경 시, git 상에서 해당 변경 사항을 추적하지 못하여 netlify에서 빌드 오류가 발생함
이미 소문자로 된 파일 네임이 git에 cache됐기 때문에 생긴 문제입니다.
아래의 글을 보고 따라하시면 해결이 될것입니다.
참고: https://daily-dev-tips.com/posts/git-basics-help-my-case-sensitive-filename-changes-dont-commit/ - hover 동영상 자동재생 시 이슈 부분에 cookie 관련 문제 발생
cookie 에 sameSite설정이 걸려있는듯 합니다.(inspector의 application -> cookie 에서도 확인할 수 있습니다.)
해당 쿠키가 프로젝트안에서 설정하는 것이라면 간단히 false로 바꿔주면 되고, 배포한 곳에서 설정하는 것이라면 사실 해제하지 않는 것이 좋습니다.
참고: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite
아쉬운 점
- 사용하지 않는 import는 지워주세요.
- 파일네이밍을 통일시켜주세요. 특히 리액트는 커스텀 훅과 컴포넌트 등에서 이름을 강제하고 있습니다.
- 불필요한 let의 사용이 많습니다. 특별한 이유가 없다면 전부 const로 선언해주세요.
Comment on lines
+1
to
+3
| import { useState } from 'react'; | ||
| import reactLogo from './assets/react.svg'; | ||
| // import './App.scss'; |
| ></path> | ||
| </g> | ||
| </svg> | ||
| <span id="title">보관함</span> |
There was a problem hiding this comment.
title은 적절하지 않아보이네요.
실제로 title이라면 h태그를 사용하는게 더 좋겠습니다.
Comment on lines
+8
to
+10
| <div id="contentContainer"> | ||
| <div id="wrapper"> | ||
| <div id="inner-content"> |
There was a problem hiding this comment.
- 어디서는 카멜 케이스, 어디서는 케밥케이스가 사용됩니다. 통일하는게 좋겠습니다.
|
|
||
| return ( | ||
| <div> | ||
| {Array.from(commentList).map((comment, index) => ( |
There was a problem hiding this comment.
commentList가 Array가 아닌가요? Array.from없이 commentList에 바로 map을 사용하는게 더 좋아보입니다.
| return ( | ||
| <div> | ||
| {Array.from(commentList).map((comment, index) => ( | ||
| <Comment key={index} comment={comment} /> |
There was a problem hiding this comment.
데이터를 확인해보니 데이터에 id값이 있네요.
유니크한 id값이 있다면 인덱스 대신 id값을 사용하는 것이 좋습니다.
Comment on lines
+2
to
+4
| let h = ''; | ||
| let m = '00:'; | ||
| let s = '00'; |
Comment on lines
+12
to
+14
| let items = props.dataSetAry.map((item, idx) => { | ||
| return <Item key={idx} d={item.d} text={item.text} />; | ||
| }); |
There was a problem hiding this comment.
items의 원본 데이터가 props로 부터 오고있으므로 지금처럼 일반 변수로 선언해도 잘 작동합니다.
아주 좋은 예시입니다!
Comment on lines
+15
to
+18
| const data = props.data; | ||
|
|
||
| const [channel, setChannel] = useState({}); | ||
| const ChannelId = data.snippet.channelId; |
There was a problem hiding this comment.
다른 컴포넌트에서처럼 destructing 할당을 이용하여 일관성을 유지 시키는게 좋겠습니다.
Suggested change
| const data = props.data; | |
| const [channel, setChannel] = useState({}); | |
| const ChannelId = data.snippet.channelId; | |
| const { data } = props; | |
| const {channelId} = data.snippet.channelId; |
Comment on lines
+15
to
+22
| const [videoId, setVideoId] = useState(''); | ||
| const [videoData, setVideoData] = useState({ | ||
| duration: '', | ||
| views: 0, | ||
| }); | ||
| const [date, setDate] = useState<Dayjs | null>(null); | ||
| const [channelId, setChannelId] = useState(''); | ||
| const [channelData, setChannelData] = useState(''); |
There was a problem hiding this comment.
videoId, date, channelId 모두 props에 종속적인 데이터기 때문에 useState가 아닌 일반 변수로 선언하면 됩니다.
Comment on lines
+48
to
+50
| setVideoId(data.id.videoId); | ||
| setChannelId(data.snippet.channelId); | ||
| setDate(data.snippet.publishedAt); |
There was a problem hiding this comment.
어차피 data가 바뀌면 현재 컴포넌트가 리렌더링 됩니다.(props가 바뀌기 때문입니다.)
따라서 위 세줄은 videoId, channelId, date를 일반 변수로 선언하면 필요가 없습니다.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
FastCampusToyStory - 리액트 토이 프로젝트(유튜브 클론)
데모 링크 : https://bejewelled-meringue-fd3712.netlify.app/
팀원들
📌 과제 목표
✅ 구현 기능
🛠 사용기술스택
- dayjs이슈
소감