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주차] 한규진 미션 제출합니다. #24

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

Conversation

9yujin
Copy link
Member

@9yujin 9yujin commented Nov 4, 2022

안녕하세요, 운영진 한규진입니다!

제가 담당한 과제이기도 하고 오래전에 짰던 코드를 보니 리팩토링하고 싶은 부분이 여럿 보여서 이번주에는 저도 과제를 같이 해보았습니다.
기능 구현같은거 저번에 했던 로직을 복붙해서 시간이 별로 들지 않은 덕분에 다른 고민을 더 해볼수 있게 되었습니다.

블로그 : [React] 다시 만드는 React-Messenger
배포링크 : React-messenger

+ 특히 리코일의 selector를 좀더 공부해보고 적극적으로 사용해보았는데요, 문서 외에 아직 정보가 많이 없더군요.. 리코일의 원래 의도대로 잘 사용하고 있는지는 아직 잘 모르겠습니다.
+ 주워 들은 이런저런 것들을 도입해보고싶은 욕심이 있다보니, 오히려 안좋은 패턴이 나오는 것 같은 걱정도 있습니다.
+ 타입스크립트를 사용하면 확실히 도움은 많이 되지만 그만큼 소요가 많이 들어가는것 같습니다. 매번 비슷한 부분에서 고민하느라 막히는것 같네요. 어렵습니다ㅠㅠ

다들 과제 너무 잘해주셨고, 한 주간 수고하셨습니당

Copy link
Member

@jhj2713 jhj2713 left a comment

Choose a reason for hiding this comment

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

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

역시 운영진이네요. 자랑스럽습니다. 코드가 너무 깔끔하고 렌더링 최적화나 구조에 신경을 많이 쓰신 것 같습니다👍 특히 squircle로 프로필 이미지 만든 부분이 굉장히 좋았던 것 같아요. 진짜 카카오톡 같네요.

수고많으셨고 다음 과제도 제출해주세요🥳

Comment on lines +16 to +20
width: 100vw;
height: 100vh;
@media screen and (min-width: 600px) {
width: 400px;
height: 700px;
Copy link
Member

Choose a reason for hiding this comment

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

반응형 최고네요~!!~👍

Comment on lines +3 to +5
import { ReactComponent as FriendIcon } from '../../assets/icons/friends.svg';
import { ReactComponent as ChatIcon } from '../../assets/icons/chat.svg';
import { ReactComponent as ThreeDot } from '../../assets/icons/threeDot.svg';
Copy link
Member

Choose a reason for hiding this comment

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

먼 경로의 import를 할때는 path alias 설정하면 더 깔끔하지 않을까요~?

Comment on lines +12 to +14
// searchResult 상태가 바뀌면서 매번 페이지가 렌더링됨...
// 그렇기 때문에 useRef로 변수 관리
const initSearchResult = useRef(searchResult);
Copy link
Member

Choose a reason for hiding this comment

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

이야👍 깔끔하네요

Comment on lines +16 to +23
<ChattingListHeadless chatting={chatting} key={chatting.chattingId}>
{({ chatting, handleClickListItem }) => (
<ListItem
data={chatting}
handleClickListItem={handleClickListItem}
/>
)}
</ChattingListHeadless>
Copy link
Member

Choose a reason for hiding this comment

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

오 Headless 컴포넌트를 이렇게 사용할 수 있군요~!!

);

const sendChat = (value: string) => {
if (value.length !== 0 && value.replace(/ /g, '').length !== 0) {
Copy link
Member

Choose a reason for hiding this comment

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

공백 처리해서 length를 확인할거면 그냥 value는 length 확인이 필요없지 않나요~?

Comment on lines +22 to +24
const user = isChatting
? friends.filter((friend) => friend.userId === data.userIdList[1])[0]
: data;
Copy link
Member

Choose a reason for hiding this comment

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

현재 채팅방 참여 인원을 배열로 저장해두고 index 1인 유저와 비교하게 되는 로직은 의미가 불분명해 보이고 확장성이 떨어지지 않나 싶습니다. 그리고 filter한 뒤 index 0으로 접근하게 된 경우에도 혹시 모를 상황의 오류 처리가 제대로 되지 않는 것 같아요!

@SeieunYoo
Copy link
Member

안녕하세요 코드 구경하다가 많이 배우고 가네요~!

역시 코드가 깔끔하고 기능별로 컴포넌트가 매우 구조화가 잘 되어있는 것 같아요 👍 특히 HeadLess 나 Search 부분에서 느낄 수 있었습니다 디자인도 너무 예쁘네요 💯

다만 채팅방 하단으로 가는 Bottom 버튼은 위로 스크롤하면 바로 볼 수 있는건가요 ❓

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