-
Notifications
You must be signed in to change notification settings - Fork 8
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
[5주차] 김윤일 과제 제출합니다. #21
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 윤일님,
매 과제마다 성장하시는게 눈에 보여서 코드리뷰가 즐겁습니다 👍
이번 과제도 고생 많으셨고, 피드백 남겼으니 확인 부탁드려요 ~
@@ -33,6 +33,9 @@ export default [ | |||
'warn', | |||
{ allowConstantExport: true }, | |||
], | |||
'react/react-in-jsx-scope': 'off', // JSX에서 React 자동 import 규칙 비활성화 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
매번 react 오류 해결을 위해 import 해주는게 번거로웠는데 저도 사용해봐야겠어요 👍
src/Router.tsx
Outdated
element: <App />, | ||
children: [ | ||
{ | ||
path: 'message/:userId', // userId 파라미터 추가 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍👍
<img src={Profile} css={userProfileImage} /> | ||
<section css={userWrapper}> | ||
{/* 이름과 아이디 */} | ||
<span css={userName}>이민형</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이름이 하드코딩 되어있네요! 실제 데이터를 반영할 수 있도록 수정해주면 좋을 것 같아요 🤩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
앗 ..! 채팅 목록 페이지에서 사용자 프로필을 컴포넌트로 따로 빼두려고 만든 파일인데
까먹고 가만히 냅뒀네요 ㅠㅠ 수정하겠습니다
<section css={userWrapper}> | ||
{/* 이름과 아이디 */} | ||
<span css={userName}>이민형</span> | ||
<span css={userId}>마지막 메세지</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분도요!
// 프로필, 알림 기능 있는 헤더바 | ||
return ( | ||
<div css={headerWrapper}> | ||
<UserProfile userImage={userImage} userName={userName} userId={userId} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
user 정보 전체를 넘기는 것과 각각 image, username, userId를 따로 넘겨줄 때의 성능 차이가 얼마나 나는지 궁금합니다! 차이가 많이 난다면, 각각 넘겨주되 객체로 묶어 한 번에 전달해주는 것도 좋을 것 같습니다 🤩
// 메세지 입력창을 포함한 모든 대화창 | ||
const users = useRecoilValue(userState); | ||
const chatData = useRecoilValue(chatState); | ||
const setChatData = useSetRecoilState(chatState); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chatData에 대한 부분은 useRecoilState를 사용하여 한 줄로 작성해보면 좋을 것 같아요 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
바로 반영해보겠습니다 !
src/components/OtherMessageItem.tsx
Outdated
return ( | ||
<div css={otherMessageItemWrapper}> | ||
<img | ||
src={'../../' + user.userImage} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분도 수정이 필요할 것 같네요 🤩
"messages": [ | ||
{ | ||
"id": 1, | ||
"userId": "onyourM__ark", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
userId는 중복이 없도록 다른 고유한 값으로 설정해주는 것이 좋을 것 같아요! 닉네임을 userId로 설정하면 중복되는 경우가 생길 것 같아서요 ~
export default function NotFound() { | ||
return ( | ||
<> | ||
<h1>404 NOT FOUND</h1>{' '} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not found 페이지까지 챙겨주셨군요 디테일 🤩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이게 다 주희님 보고 배웠습니다 ㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 챙겨갑니다 디테일🥹
src/App.tsx
Outdated
export default function App() { | ||
const navigate = useNavigate(); | ||
useEffect(() => { | ||
navigate('/chatList'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
배포링크에서 채팅방 링크 입력으로 새로고침 시 링크 자체가 not found가 되는데, 이 부분 때문일 것 같아요! 한 번 확인해주세요 🤩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정했습니다 !
useEffect를 삭제하고 router에서 chatList경로를 기본 경로인 ''로 수정해주니까 되더라구요👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
윤일님! 이번 미션도 수행하시느라 너무 고생많으셨어요!
윤일님 코드 보면서 디테일하게 챙기시는 부분까지 많이 배우고갑니다!
남은 미션까지 화이팅해봐요☘️
'react/react-in-jsx-scope': 'off', // JSX에서 React 자동 import 규칙 비활성화 | ||
'no-unused-vars': ['error', { varsIgnorePattern: '^React$' }], // React 변수에 대한 미사용 경고 무시 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 이부분 사용해봐야할 것 같아요!
<img src={Profile} css={userProfileImage} /> | ||
<section css={userWrapper}> | ||
{/* 이름과 아이디 */} | ||
<span css={userName}>이민형</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 주희님이랑 같은 생각입니다! props로 이름과 메세지를 전달하면 다양하게 데이터 처리가 가능할 것 같아요!
return ( | ||
<div css={userProfileItems}> | ||
<img src={Arrow} css={arrowLeft} onClick={handleGoToChatList} /> | ||
<img src={'../../' + userImage} css={userProfileImage} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 저번에 이런식으로 이미지 불러와서 이미지 파일 경로가 고정되어있지 않을때 에러가 엄청 났던 것 같습니다! 전체 URL을 Prop으로 전달받아도 좋을 것 같아요!
|
||
setNewMessage(''); | ||
textareaRef.current && textareaRef.current.focus(); // 메세지 전송 후 textarea에 포커스 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onCheckEnter 함수에서 setNewMessage('')를 한 번 더 호출하는데, 이미 handleSendMessage 함수 내에서 처리되어 중복 처리 되는게 아닌지 궁금합니다!
/** @jsxImportSource @emotion/react */ | ||
import { css } from '@emotion/react'; | ||
import { UserProfile } from './UserProfile'; | ||
import { Settings } from '../UI/Settings'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 다음에 UI 폴더를 적용해봐야겠어요!!
messageWindowRef.current?.scrollIntoView({ behavior: 'smooth' }); | ||
}, [messages]); // messages가 업데이트될 때마다 실행 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저번에 윤일님이 리뷰해주셨던 부분인데! 복습하고 갑니다🥹
<> | ||
<div css={messageListWrapper}> | ||
{messages.map((message) => { | ||
const user = users.find((user) => user.userId === message.userId)!; // '!'를 추가해줌으로서 user는 반드시 존재한다. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오홍 제 코드에 알려주신 부분이 이거군요! 저도 바로 적용해보겠습니다!
@@ -0,0 +1,81 @@ | |||
/** @jsxImportSource @emotion/react */ | |||
import { css } from '@emotion/react'; | |||
import { useEffect, useRef } from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오홍 useRef 사용하신 부분이 어디인지 궁금해요! 코드 어디부분인지 못찾겠어열..ㅠㅠ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정하면서 지운다는 걸 깜빡했네요 ! 적용하겠습니당👍
{users | ||
.filter((user) => user.userName !== '김윤일') // 사용자 '김윤일'은 채팅 목록에서 제외 | ||
.map((user) => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
필터링으로 특정 사용자를 제외하셨군요! 하드코딩 값보다는 상수나 설정파일에서 관리해도 좋을 것 같아요!
export default function NotFound() { | ||
return ( | ||
<> | ||
<h1>404 NOT FOUND</h1>{' '} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 챙겨갑니다 디테일🥹
안녕하세요 유레카 프론트엔드 1기 김윤일입니다.
PR 늦게 해서 죄송하단 말씀 올리면서 시작하겠습니다. recoil과 router 등등 안 써본 거라면 겁부터 먹었었는데
직접 구현해보니 두려웠던 마음은 없어졌습니다😁
여태 배운 내용 토대로 싸-악 새로 만들어 보고 싶네요 !
💡 알게된 부분
path: 'message/:userId'
이처럼 parameter추가해서 사용하기, Outlet으로 자식 요소 사용 가능하게 하기 등등🏷️ 개선하고 싶은 부분
📖 생각해 볼 질문들
1. SPA / MPA / CSR / SSR 이란 무엇인가요?
2. React에서 상태는 어떻게 관리할 수 있나요?
3. 어떤 상태관리 라이브러리를 사용했고, 그것의 특징은 무엇인가요?
🚀 배포
https://react-sns-pink.vercel.app/