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주차] 주효정 미션 제출합니다. #12

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

Conversation

jhj2713
Copy link
Member

@jhj2713 jhj2713 commented May 12, 2022

안녕하세요. 프론트 15기 주효정입니다! 이번주차도 지난주에 이어 열심히 해봤는데 마음에 들지 않는 부분이 조금씩 있네요... 언제쯤 마음에 쏙 드는 코드를 짤 수 있을까요😅

여담으로 저는 Mock data 만드는걸 재밌어해서 그런지 이번주차 과제를 특히나 즐겁게 한 것 같아요..ㅎ 제 취향을 담아서 재밌게 만들었습니다.

배포링크

https://react-messenger-15th-6dd1t6qwf-jhj2713.vercel.app

추가한 부분

단체 채팅

사실 지난주에도 단체 채팅을 구현해보고 싶었는데 시간이 많지 않아서 구현하지 못했는데요.. 이번주에는 조금 시간이 생겨서 추가로 구현을 해봤습니다.

화면 기록 2022-05-12 12 20 32

개인 chat과 같이 toggle로 사용자를 선택할 수 있도록 구현했습니다.

스크린샷 2022-05-12 12 23 31

아쉬운 점은 ChatList에서 채팅에 참여한 여러 user의 프로필사진을 함께 표시하고 싶었는데 그 부분을 구현하지 못했습니다. 카카오톡 단체 채팅방처럼 여러 프로필사진을 분할해서 보여주고 싶은데 아직 명확한 방법이 떠오르질 않네요... 더 여유가 있을때 꼭 생각해보겠습니다😢

Key Questions

Routing

  • URL에 따라 그에 맞는 화면을 보여주는 것으로 단순하게 우리가 페이지 이동이라고 말하는 것을 의미
  • React에서는 React-router를 사용해 routing을 구현한다

SSR (Server Side Rendering)

  • 페이지를 이동할 때마다 새로운 페이지에 대한 요청을 보냄
  • 초기 로딩 속도가 빠르다
  • 하지만 서버에서 렌더링을 마치고 data가 결합된 HTML 파일을 내려주기 때문에 페이지를 요청할 때마다 새로고침이 발생한다

SPA (Single Page Application)

  • 하나의 페이지 안에서 필요한 데이터만 가져오는 Single Page Applicaion
  • 초기 렌더링 후 데이터만 받아오기 때문에 상대적으로 서버 요청이 적다
  • 하지만 처음 화면을 로딩할 때 모든 화면이 준비되어있어야 하기 때문에 첫 로딩에 시간이 오래 걸릴 수 있다

감사합니다😄

Copy link

@jdaeheon jdaeheon left a comment

Choose a reason for hiding this comment

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

안녕하세요, 코드 리뷰 파트너 정대헌입니다. 코드에서 웹 개발에 대한 내공이 느껴지는 것 같아요. 알지 못했던 JS 문법들도 많이 배워갑니다. Recoil 문법도 정말 간단하다는 생각도 드네요. 단체톡 로직에 관해서는 개선이 가능할 것 같은데, 이렇게 저렇게 해보다가 개선을 못했네요. 그래도 전반적으로 많은 배움이 되었습니다. 좋은 코드 공유주셔서 감사해요!

Comment on lines +11 to +17
const _handleResize = () => {
if (window.innerWidth <= 640) {
setIsMobile(true);
} else {
setIsMobile(false);
}
};

Choose a reason for hiding this comment

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

App 컴포넌트에서 window 값을 계산한 뒤 recoil state에 저장하는군요.
미디어 쿼리 외에 상태값을 이용한 반응형 구현도 괜찮은 것 같아요!

Comment on lines +11 to +20
const filterUser = (text: string): void => {
if (!text.trim()) {
setShowUser(totalUser);
} else {
const filtered = totalUser.filter((user) =>
user.name.toLowerCase().includes(text.trim().toLowerCase())
);
setShowUser(filtered);
}
};

Choose a reason for hiding this comment

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

Suggested change
const filterUser = (text: string): void => {
if (!text.trim()) {
setShowUser(totalUser);
} else {
const filtered = totalUser.filter((user) =>
user.name.toLowerCase().includes(text.trim().toLowerCase())
);
setShowUser(filtered);
}
};
const filterUser = (text: string): void => {
const filtered = totalUser.filter((user) =>
user.name.toLowerCase().includes(text.trim().toLowerCase())
);
setShowUser(filtered);
};

우선 trim을 통해 공백을 처리하신 점이 인상 깊습니다. 다만, 입력이 없으면 모든 includes 값이 true로 나와서 별도로 전체 보여주기를 처리할 필요가 없더라구요.

Copy link
Member Author

Choose a reason for hiding this comment

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

오 그렇네요! 알려주셔서 감사합니다~!

Comment on lines +12 to +22
if (!text.trim()) {
setShowMessage(message);
} else {
const filtered = message.filter(
(msg) =>
msg.user.filter((user) =>
user.name.toLowerCase().includes(text.trim().toLowerCase())
).length !== 0
);
setShowMessage(filtered);
}

Choose a reason for hiding this comment

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

Suggested change
if (!text.trim()) {
setShowMessage(message);
} else {
const filtered = message.filter(
(msg) =>
msg.user.filter((user) =>
user.name.toLowerCase().includes(text.trim().toLowerCase())
).length !== 0
);
setShowMessage(filtered);
}
const filtered = message.filter(
(msg) =>
msg.user.filter((user) =>
user.name.toLowerCase().includes(text.trim().toLowerCase())
).length !== 0
);

위와 동일한 내용입니다!

const Search = ({ filter }: ISearch) => {
const { text, handleTextChange } = useInput("");
const isMobile = useRecoilValue(resizeState);

Choose a reason for hiding this comment

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

컴포넌트를 재사용 하는군요, 아주 멋집니다!

Comment on lines +9 to +11
<IconBox to="/" selected={path === "friends"}>
<AiOutlineUser size={22} />
</IconBox>

Choose a reason for hiding this comment

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

Link 컴포넌트 안에 아이콘을 넣어서 사용하시는 군요. 저는 버튼 안에 navigate를 넣어서 사용했는데, 같은 기능에 대해서 서로 구현 방법이 다른 점이 인상깊습니다.

Comment on lines +28 to +31
${ListContent} {
opacity: 0.7;
transition: 0.15s;
}

Choose a reason for hiding this comment

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

styled 개체 내에서 하위 개체의 스타일을 제어하는 방법 배워갑니다.

String(new Date().getHours()).padStart(2, "0") +
":" +
String(new Date().getMinutes()).padStart(2, "0");
const messageObj = {

Choose a reason for hiding this comment

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

padStart도 처음 알았는데, 시간을 사용할 때 유용할 것 같네요. 감사합니다!

Comment on lines +1 to +23
import { IUserType, IUserState } from "./user";
import {
IInputMessageForm,
IMessageType,
IInputEmoticon,
IEmoticonPopover,
} from "./message";
import { IAlert } from "./alert";
import { IChatListItem, IList, IChatRoomState, ISearch } from "./chat";

export type {
IUserType,
IUserState,
IInputEmoticon,
IEmoticonPopover,
IAlert,
IInputMessageForm,
IMessageType,
IChatListItem,
IList,
IChatRoomState,
ISearch,
};

Choose a reason for hiding this comment

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

맞아요, 이렇게 어떤 분들은 index.tsx로 모아서 출력하시더라구요. 혹시 자주 사용되는 패턴인가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

저번에 멘토님이 코드리뷰해주신대로 파일이 추가되거나 변경될때 같이 수정해줘야되는 부분에서 장단점이 확실해서 그런지 자주 사용되는지는 모르겠지만,,, import코드가 너무 길어질때 몇 번씩 본 것 같아요!

Copy link

@poodlepoodle poodlepoodle left a comment

Choose a reason for hiding this comment

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

안녕하세요 효정 님, 5주차 코드 리뷰 파트너 최어진입니다...!
사실 제가 느끼기에 효정 님께서 작성하신 코드에
바꾸면 좋겠다 싶은 부분들이 거의 없을 뿐더러...
그나마 감탄 포인트들마저도 대헌 님께서 많이 선점하셨기에...
저는 방명록 느낌으로 남기고 갑니다.......!

다음 주 대헌 님과 효정 님이 함께 수행하는 미션의 결과가 더욱 기대되네요...!
저 사실은 다른 파트 사람들이 프론트 개발자들에 대해서 궁금해 할 때
둘 다 대단한 실력이면서도 성실한 개발자라고 알리고 다니거든요.....
두 분 팬클럽 회장인 것 처럼... 두 분 홍보대사인 것 처럼.....
TMI 죄송... 그럼 고생 많으셨습니다!

link={msg.id}
img={msg.user[0].name}
title={_handleFriendsName(msg.user)}
subTitle={msg.messages[msg.messages.length - 1].text}

Choose a reason for hiding this comment

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

가장 최근 대화 내역을 채팅방의 서브 타이틀로 보여주도록 구현하신 방식 잘 참고하고 가겠습니다...!!

Comment on lines +56 to +64
const AlertContent = styled.section`
margin: 1.8rem;
font-size: 1.1rem;
`;
const AlertButtonBox = styled.section`
display: flex;
justify-content: flex-end;
margin: 1.5rem;
`;

Choose a reason for hiding this comment

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

"메세지를 입력해 주세요" Alert Modal을 구현하신 방식도 배우고 갑니다...!

return (
<ListItem to={"/chatRoom/" + link}>
<ProfileImgBox>
<ProfileImg alt="profile" src={`images/${img}.jpg`} height={50} />

Choose a reason for hiding this comment

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

효정 님 코드 보면서 제가 이미지 리소스를 가져다 쓰고 구성하는 방식에 대해서
반성하고 개선할 수 있는 부분 느끼고 갑니다...!

Comment on lines +28 to +45
const PopoverContainer = styled.section`
position: absolute;
transform: translate(0, -120%);
background-color: #f1f1f3;
display: flex;
padding: 0.6rem;
border-radius: 0.5rem;
:before {
border-color: #f1f1f3 transparent;
border-style: solid;
border-width: 0.5rem 0.5rem 0 0.5rem;
content: "";
display: block;
left: 1.15rem;
position: absolute;
bottom: -0.5rem;
}
`;

Choose a reason for hiding this comment

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

제가 맨 처음에 만들고 싶었던 이모티콘 박스 기능을
효정 님께서 그대로 구현해 주신 걸 보고 나중에 코드 리뷰가 아니더라도 참고하고 싶었는데
이렇게 구성하셨군요..!
잘 배우고 갑니다.

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.

3 participants