Skip to content

Conversation

@yehey-1030
Copy link

@yehey-1030 yehey-1030 commented Apr 16, 2021

리액트 라우터 시작할때는 좀 많이 막막 했는데 여전히 막막한 것 같아요 하하
코드가 좀 더러운데 주석처리도 안해서.. 코드 리뷰 하시는 분 죄송합니다ㅠㅠ
이것저것 다양한 기능 추가하고 싶었는데 상상만큼 구현이 잘 안되서 아쉬웠던 것 같아요
저는 react router 사용했습니다. 따로 local storage는 사용하지 않았고, 그래서 아주 기본적인 기능만..! 들어가있습니다. 아마 부족한 부분도 많을 것 같아요. 앞으로 더 발전해야겠죠 >.0

https://react-messenger-13th-ashy.vercel.app/profile
갑자기 vercel 오류가 떠서 배포가 안되고 있어요ㅠㅠ 코드 리뷰하는 분 한번 더 죄송,,,합니다
vercel 오류 해결했는데 파일 이름 수정한거 깃에 반영 안돼서 계속 오류가 났네요 하핳핳
이제 해결했습니다~~!!

Copy link

@S-J-Kim S-J-Kim 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 +20 to +24
<Route path="/" component={MenuBar} />
<Route path="/profile" component={ProfileListView} />
<Route path="/chatting-list" component={ChattingList} />
<Route path="/chat-with/:userID" component={MessageChattingView} />
<Route path="/more" component={MoreView} />
Copy link

Choose a reason for hiding this comment

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

컴포넌트 이름이 굉장히 직관적이어서 좋군요 1따봉 드립니다..

Comment on lines +16 to +20
for (let userProfile of userProfileSet) {
if (userProfile.name.includes(searchUserInput)) {
setMatchUserSet((matchUser) => [...matchUser, userProfile]);
}
}
Copy link

Choose a reason for hiding this comment

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

Suggested change
for (let userProfile of userProfileSet) {
if (userProfile.name.includes(searchUserInput)) {
setMatchUserSet((matchUser) => [...matchUser, userProfile]);
}
}
userProfileSet.forEach(elem => {
if (elem.name.includes(searchUserInput)) {
setMatchUserSet(matchUser => [...matchUser, elem]);
}
}

멋지게 forEach를 한번 써보자구요 ㅎㅎ 물론 이 방식도 좋습니다.

Comment on lines +8 to +14
{ chattingUser: "2", messageContext: "3주차", id: Date.now() },
{ chattingUser: "3", messageContext: "미션", id: Date.now() },
{ chattingUser: "4", messageContext: "거의 끝...", id: Date.now() },
{ chattingUser: "5", messageContext: "오늘의 TMI!", id: Date.now() },
{ chattingUser: "6", messageContext: "아직 중간고사", id: Date.now() },
{ chattingUser: "7", messageContext: "안끝났음....", id: Date.now() },
{ chattingUser: "8", messageContext: "깔깔....", id: Date.now() },
Copy link

Choose a reason for hiding this comment

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

지금은 각 채팅별로 기존 대화목록이 딱 한개이기 때문에 괜찮지만,,, 앞으로 채팅 내용을 저장하고 불러올 기능까지 구현을 염두해 둔다면 messageContext는 array의 형태를 갖는게 적절해 보입니다!

Choose a reason for hiding this comment

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

저도 선종오빠랑 같은 의견입니다!

Copy link

@GeonHeeAhn GeonHeeAhn left a comment

Choose a reason for hiding this comment

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

예진님 코드 잘 봤습니다. 전체적으로 코드가 깔끔해서 직관적으로 이해할 수 있어 좋았어요. 저의 스파게티 코드가 반성이 되는ㅠㅠ...진짜 프로의 코드처럼 너무 깔끔하게 잘 작성되어 있어서 제가 더 덧붙일 말이 없었어요. 감상하듯이 잘 봤습니다. 첫번째로 과제 제출하셨던데 어쩜 그럴 수 있는지...코딩짱이 되어버린건지....!! 무튼 이번 과제도 수고하셨습니다!!

<MessageBox userChattingMessageSet={userChattingMessageSet}/>
<MessageInputBar clickInputButton={clickInputButton}/>
<Container>
<Route path="/" component={MenuBar} />

Choose a reason for hiding this comment

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

"/"에 메뉴바 설정해서 모든 경우에 뜨게 하는 방법 되게 좋네요.


function ChattingProfileBubble({ profile }) {
//파라미터 쓰기
const history = useHistory();

Choose a reason for hiding this comment

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

로컬 스토리지처럼 history에 채팅 정보가 저장되도록 한건가요? 너무 대단...

border: none;
padding: 0;
margin: 1vw;
opacity: ${(props) => (props.clicked === 1 ? "100%" : "40%")};

Choose a reason for hiding this comment

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

이렇게 간단하게 디폴트/클릭시 상태를 변경할 수 있다니!! 짱

object-fit: contain;
`;

const ProfileModal = ({ modalState, setModalState, detailProfile }) => {

Choose a reason for hiding this comment

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

이 파일은 프로필목록에서 특정 클릭 시 프로필 창이 뜨게 하도록 구현한 코드인가요? 너무 좋은 것 같아요. 저도 예진님꺼 보고 이렇게 하면 좋겠다 싶어서 시도해 봤는데 저는 잘 안되더라구요ㅎㅎ..

Comment on lines +8 to +14
{ chattingUser: "2", messageContext: "3주차", id: Date.now() },
{ chattingUser: "3", messageContext: "미션", id: Date.now() },
{ chattingUser: "4", messageContext: "거의 끝...", id: Date.now() },
{ chattingUser: "5", messageContext: "오늘의 TMI!", id: Date.now() },
{ chattingUser: "6", messageContext: "아직 중간고사", id: Date.now() },
{ chattingUser: "7", messageContext: "안끝났음....", id: Date.now() },
{ chattingUser: "8", messageContext: "깔깔....", id: Date.now() },

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