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

[4주차] 유선호 미션 제출합니다. #17

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

YooSeonHo
Copy link
Member

@YooSeonHo YooSeonHo commented Nov 4, 2022

저번 과제 때 확장성을 고려하지 않아서 애를 먹었고 정말 반성했습니다.ㅠ.ㅠ
state와 interface를 쓰는 부분을 조금 더 공부 해야겠다고 뼈 저리게 느꼈습니다.
그래도 나름대로 모두 구현하게 되어 기분이 좋습니다.

배포 링크 : https://react-messanger-16th-seonho-gntulvc7d-yooseonho.vercel.app

1.Routing
파일 또는 페이지로 이동시켜주는 기능
react-router-dom 라이브러리를 이용하여 초보자도 간편하게 할 수 있습니다.
history, routes 등 문법이 자주 바뀌기 때문에 주의하며 써야합니다.

2.SPA
Single Page Application의 약어로, 말 그대로 서버에 의해 받아 오는 페이지가 하나고, 뷰 랜더링을 사용자의 브라우저가 담당합니다.
리액트의 장점이 SPA라고 생각했는데 여러가지 파일이 커지면 코드가 너무 길어진다는 단점도 있어서 꼭 필수는 아니라고 합니다.

3.상태관리
상태는 컴포넌트 내부에서 관리 되면서 랜더에 영향을 미치는 객체들입니다.
컴포넌트간 상태를 전달하는 것이 굉장히 까다로운데, 부모 자식간이 아니면 직접 전달할 수도 없습니다.
전역으로 상태를 관리해주는 recoil, redux를 쓰면 이 상태관리 문제를 해결할 수 있습니다.

@jhj2713
Copy link
Member

jhj2713 commented Nov 5, 2022

안녕하세요, 프론트 파트장 주효정입니다🙌

항상 간결하게 코드를 작성해주셔서 코드 읽기가 쉬운 것 같아요👍 그리고 user들의 상태메시지가 너무 귀엽네요. 삥빵뿡

몇 가지 말씀드리자면 style-component로 인해서 코드가 두 배로 늘어나고 있는데, 스타일 컴포넌트를 다른 파일로 빼보는게 어떨까 생각이 듭니다. 물론 파일로 스타일을 빼면 스타일 수정에 있어서 불편함이 있을 수 있어서 상황에 따라 사용하면 좋을 것 같지만 훨씬 간결해보이지 않을까 싶네요!

이번주도 수고많으셨고 스터디 시간에 뵙겠습니다🥳

Copy link

@hamo-o hamo-o left a comment

Choose a reason for hiding this comment

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

이번 코드리뷰 파트너 이현영입니다!!
코드가 깔끔해서 보기 정말 쉬웠어요!!! 저도 가독성 좋은 코드를 위해 노력해야겠다는 생각을 했슴당,,
폰트도 넘 귀엽네요 수고하셨습니다~~!!

children: React.ReactNode | React.ReactNode[];
};

const MainBox: React.FC<BoxProps> = ({ children }: BoxProps) => {
Copy link

Choose a reason for hiding this comment

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

앗 저도 React.FC 사용했었는데 이 글 참고해보셔도 좋을 것 같아요!!

import Input from '../components/chatting/input';
import Box from '../components/box/box';

const Chatting = (props: any) => {
Copy link

Choose a reason for hiding this comment

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

저도 참 많이 애용할 수 밖에 없었던 ..ㅎㅎ any 네요
props 안쓰시면 비워도 될 것 같아요!!


return (
<TextBox>
{chatList[userNum].chat.map((li, index) => (
Copy link

Choose a reason for hiding this comment

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

와 코드가 엄청 깔끔해서 신기했는데 이런 방법을 쓰셨군요 ..!! 배워갑니다

Copy link

@heeeesoo heeeesoo left a comment

Choose a reason for hiding this comment

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

안녕하세요! 저도 저번에 확장성을 고려하지 않아서 이번 과제 처음부터 다시 짜느라 힘들었네요😤 커밋 메세지 잘 작성해주셔서 코드 읽는데 좋았어요! 오늘 스터디에서 뵐게요~!!

import { useRecoilState, useRecoilValue } from 'recoil';
import { chatSelector, nowUserState } from '../../state/state';

const UserInfo = styled.div`
Copy link

Choose a reason for hiding this comment

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

아 이렇게 diplay 요소가 중복되면 안에 .으로 구분해서 넣는거 좋네요! 전 하나씩 다 지정하느라 코드가 좀 길어진 것 같네요 저도 저렇게 한번 해봐야겠어요!

@@ -0,0 +1,45 @@
{
Copy link

@heeeesoo heeeesoo Nov 6, 2022

Choose a reason for hiding this comment

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

오 이렇게 userId를 따로 지정하고 안에 chat key에 메세지 배열 넣는거 좋네요! 나중에 채팅이 많아질 때 보기 편할 것 같아요😀

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