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

[5주차] 한규진 미션 제출합니다. #11

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

9yujin
Copy link
Member

@9yujin 9yujin commented May 11, 2022

안녕하세요, 한규진입니다. 다음주에 있는 전공 중간고사와 과제때문에 미리미리 해서 먼저 제출합니다..

배포링크

https://react-messenger-15th-theta.vercel.app/

대부분 기능을 카카오톡 pc 앱을 보며 최대한 비슷하게 보이도록 구현했으니 같이 보시면 더욱 재밌을 것 같습니다.
저번주 과제를 할때 확장을 최대한 고려하며 한 덕분에 그나마 과제하기 편했던 듯 하네요.

카카오로그인 기능을 넣었습니다. react-kakao-login 이라는 개꿀 라이브러리가 있습니다.
로컬스토리지에 유저정보 값을 저장하고 새로고침해도 로그인이 유지되도록 했습니다.
리코일 상태를 초기화할때 애를 좀 먹었습니다. 어렵네요. 참고

과제의 핵심은 라우팅 기능이라고 생각했습니다. 조금 더 실제 프로젝트에 가깝게끔 기능을 넣었습니다.

  • requireAuth 컴포넌트를 통해 로그인되어있지 않은 상태일 때 메인 페이지로 리다이렉트 시킵니다.
    저런 컴포넌트도 HOC라고 부르는지는 모르겠습니다. 참고
  • 채팅방에 url로 직접 접근할때도 메인페이지로 리다이렉트합니다.
    채팅방은 메인페이지를 통해서만 접근할 수 있습니다. useNavigate에서 state를 넘길수 있습니다. 참고
  • 그 외에 따로 지정되지 않은 url로 들어올때는 not found 페이지로 넘깁니다.

리액트 라우터 돔 v6가 너무너무 익숙치 않습니다. 한줄한줄 쓸때마다 구글링을 엄청엄청 했네요.
과제를 하면서 덕분에 그나마 좀 친해진것 같습니다.

css가 제일 어렵습니다. 이거만 아니었다면 시간이 절반은 줄었을텐데요. 으악. css없는 세상에 살고 싶습니다.


Key Question

이전에는 페이지를 이동할때마다 새로 요청을 해서 받아와 페이지를 새로고침을 해야 했습니다.
SPA란 말그대로 page가 한개인 앱을 말합니다. 그 한 페이지 안에서 주소에 따라 다양한 화면을 렌더링할 수 있습니다.
그리고 그 주소에 따라 다른 화면을 보여주도록 하는것을 Routing이라고 합니다.
리액트 라우터 돔 문서가 꽤 잘되어있더군요.. 다행입니다.

서버사이드렌더링에 대해서는 직접 공부해본적이 없어 아쉽습니다.
이전에 인강을 짤막하게 듣다 만적이 있었는데, 그때 적어놓았던 부분을 올려봅니다.

스크린샷 2022-05-12 오전 2 47 22

Copy link

@sungwoo-shin sungwoo-shin left a comment

Choose a reason for hiding this comment

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

안녕하세요, 5주차 코드리뷰 파트너 신성우 입니다. 공부한다는 생각으로 꼼꼼히 봤습니다. 채팅방 스크롤바, 하단 이동 버튼을 구현하신것과 챗 버블 구현하신것을 보고 많이 배웠습니다. 코멘트는 이번주 과제인 라우팅 파트에 집중적으로 남겼습니다.

background-color: #fff;
width: 100vw;
height: 100vh;
@media screen and (min-width: 600px) {

Choose a reason for hiding this comment

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

Suggested change
@media screen and (min-width: 600px) {
@media screen and (min-width: 768px) {

상황에 따라 적절한 값을 사용해야 겠지만, 반응형 제작 시 참고하는 모바일 디바이스 표준 width 값은 <= 768px 입니다.

Comment on lines +18 to +22
/* export const ListContainer = styled.div`
padding: 0px 18px;
`; */

export const ListBody = styled.div``;

Choose a reason for hiding this comment

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

Suggested change
/* export const ListContainer = styled.div`
padding: 0px 18px;
`; */
export const ListBody = styled.div``;

Comment on lines +22 to +24
<RequireAuth>
<Router />
</RequireAuth>

Choose a reason for hiding this comment

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

image
Route의 children으로 들어가는 JSX 엘리먼트(중첩 라우트)를 보여줄땐 리액트 라우터에서 제공하는 Outlet 컴포넌트를 사용합니다.

Comment on lines +13 to +15
<GlobalStyle />
<RecoilRoot>
<BrowserRouter>

Choose a reason for hiding this comment

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

image

GlobalStyle, RecoilRoot, BrowserRouter는 일반적으로 src/index.tsx 에서 App을 감싸는 형태로 사용합니다.
App.tsx 리턴문 안의 컴포넌트들의 인덴팅이 과도하게 복잡해진것 같습니다.

Comment on lines +28 to +36
display: flex;
height: 100%;
justify-content: center;
align-items: center;
flex-direction: column;
background-color: #fcec5c;
@media screen and (min-width: 600px) {
border-radius: 15px;
}

Choose a reason for hiding this comment

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

Suggested change
display: flex;
height: 100%;
justify-content: center;
align-items: center;
flex-direction: column;
background-color: #fcec5c;
@media screen and (min-width: 600px) {
border-radius: 15px;
}
height: 100%;
display: flex;
justify-content: center;
align-items: center;
flex-direction: column;
background-color: #fcec5c;
@media screen and (min-width: 600px) {
border-radius: 15px;
}

관련 있는 CSS 코드를 분리하면 가독성에 도움이 되는것 같습니다!
(코딩 스타일 영역이라 단지 추천 입니다!)

Comment on lines +41 to +43
if (inputRef.current) {
inputRef.current.focus();
}

Choose a reason for hiding this comment

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

저는 다른 방식으로 했는데 이렇게 하는게 효율적이네요!


const onEnter = (e: KeyboardEvent<HTMLTextAreaElement>) => {
if (e.key === 'Enter' && !e.shiftKey) {
// 우왕 : https://velog.io/@corinthionia/JS-keydown에서-한글-입력-시-마지막-음절이-중복-입력되는-경우-함수가-두-번-실행되는-경우

Choose a reason for hiding this comment

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

빅 샤랏투 프짱!

Comment on lines +90 to +92
// asdfasdfsadfasdf 와 같이 영어를 공백없이 길게 넣었을때 줄바꿈이 되지않고
// 화면을 넘어가는 경우가 있음.
// 왜이럴까요

Choose a reason for hiding this comment

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

이거 저도 그런데 해결방법을 모르겠네요,,

Copy link
Member

Choose a reason for hiding this comment

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

word-break: break-all을 추가해 보세요!

Comment on lines +21 to +51
// 맨 아래로 버튼 보이기 & 스크롤바 숨기기
const handleScrollY = throttle(() => {
if (bodyRef.current) {
const roomBodyScrollY = bodyRef.current.scrollHeight - bodyRef.current.scrollTop;

if (roomBodyScrollY > 800 && !scrollButtonVisible) {
setScrollButtonVisible(true);
} else if (roomBodyScrollY <= 800 && scrollButtonVisible) {
setScrollButtonVisible(false);
}
}
setScrollBarVisible(true);
setTimeout(() => {
setScrollBarVisible(false);
}, 2000);
}, 300);

const goToBottom = () => {
if (bodyRef.current) {
bodyRef.current.scrollTo(0, bodyRef.current.scrollHeight);
}
};

useEffect(() => {
goToBottom();
setScrollBarVisible(false);
}, [chats]);

const handleClickGoToBottom = () => {
goToBottom();
};

Choose a reason for hiding this comment

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

많이 배우고 갑니당~ 🙌

);
};

export default RoomBody;

Choose a reason for hiding this comment

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

규진님께서 작성하신 코드는 currentUser를 토글 하는 순간 기존 메세지가 모두 리랜더 됩니다. 상태 관리 구조 개선을 통해 최적화할 수 있습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

감사히 뚜까맞고갑니다♥️... 특히 상태관리와 네이밍에 많이 신경을 써야겠군요..!!

Copy link
Member

Choose a reason for hiding this comment

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

뚜까팬다는 투두메이트 알림 보고 왔는데 정말 자세히 달아 주셨네요... 최고예요 👍🏻👍🏻

Choose a reason for hiding this comment

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

저는 투두메이트를 안하는데도 소식을 듣고 리뷰 봤는데 진짜 정성 대박이네요... 최곱니다.. 저도 같이 뚜까맞고 가요......

Copy link
Member

@corinthionia corinthionia left a comment

Choose a reason for hiding this comment

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

안녕하세요 규진 님 🙌🏻

규진 님 코드리뷰를 굉장히 오랜만에 해 드리는 것 같네요! 사실 다른 일을 하고 있다가 성우 님의 투두메이트 알림을 받고 구경을 와 봤는데요, 성우 님께서 코멘트를 너무 잘 달아 주시고, 제가 아직 자세히 보지를 못해서 (ㅠㅠ) 코멘트를 많이 못 달아 드렸네요. 시간이 남는다면 나중에 더 자세히 리뷰해 드리겠습니다...!

과제하시느라 고생 많으셨습니다!
그럼 스터디 시간에 뵐게요~

Comment on lines +90 to +92
// asdfasdfsadfasdf 와 같이 영어를 공백없이 길게 넣었을때 줄바꿈이 되지않고
// 화면을 넘어가는 경우가 있음.
// 왜이럴까요
Copy link
Member

Choose a reason for hiding this comment

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

word-break: break-all을 추가해 보세요!

Copy link

@chaaerim chaaerim left a comment

Choose a reason for hiding this comment

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

규진님 안녕하세요. 코드리뷰 파트너 김채림입니다 😇
이번주차 코드리뷰로 저를 킹받게 하는 것이 목표이셨다면 대성공하셨습니다..^6^.........(스포는 죽음뿐 ㅎㅎ......)
그렇지만 규진님 코드는 감탄만 하면서 봤네요... 진짜 디테일하게 코딩하시는 것 같아요..
앞서서 성우님이 열심히 남겨주신 코멘트 덕분에 저도 많이 공부하면서 리뷰했습니다 . .
제가 제안드리고 싶은 점은 없었고 그냥 감탄사 몇개 적어봤어요.. 그럼 스터디 시간에 뵐게요 ~ ! !

Comment on lines +1 to +10
{
"root": true,
"parser": "@typescript-eslint/parser",
"plugins": ["@typescript-eslint", "no-loops"],
"extends": ["eslint:recommended", "plugin:@typescript-eslint/eslint-recommended", "plugin:@typescript-eslint/recommended"],
"rules": {
"no-console": 2,
"no-loops/no-loops": 2
}
}

Choose a reason for hiding this comment

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

👍👏

Comment on lines +15 to +51
const RoomBody = ({ chats, users, currentUser }: RoomBodyProps) => {
const [scrollButtonVisible, setScrollButtonVisible] = useState<boolean>(false);
const [scrollBarVisible, setScrollBarVisible] = useState<boolean>(false);

const bodyRef = useRef<HTMLDivElement>(null);

// 맨 아래로 버튼 보이기 & 스크롤바 숨기기
const handleScrollY = throttle(() => {
if (bodyRef.current) {
const roomBodyScrollY = bodyRef.current.scrollHeight - bodyRef.current.scrollTop;

if (roomBodyScrollY > 800 && !scrollButtonVisible) {
setScrollButtonVisible(true);
} else if (roomBodyScrollY <= 800 && scrollButtonVisible) {
setScrollButtonVisible(false);
}
}
setScrollBarVisible(true);
setTimeout(() => {
setScrollBarVisible(false);
}, 2000);
}, 300);

const goToBottom = () => {
if (bodyRef.current) {
bodyRef.current.scrollTo(0, bodyRef.current.scrollHeight);
}
};

useEffect(() => {
goToBottom();
setScrollBarVisible(false);
}, [chats]);

const handleClickGoToBottom = () => {
goToBottom();
};

Choose a reason for hiding this comment

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

저도 진짜 배우고 갑니다..전 라이브러리 쓸 생각만 했는데 ..


const ChatItem = ({ chat, user, currentUser, prevChat }: ChatItemProps) => {
const isCurrentUser = chat.userId === currentUser.userId;
const isPrevSame = prevChat ? chat.userId === prevChat.userId : false;

Choose a reason for hiding this comment

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

prevChat을 props로 왜 넘겨주나 했는데 같은 사람이 여러번 채팅을 보냈을때를 위한거였군요....0o0....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants