Skip to content

[3주차] 김선영 과제 제출#11

Closed
seondal wants to merge 21 commits into
CEOS-Developers:masterfrom
seondal:master
Closed

[3주차] 김선영 과제 제출#11
seondal wants to merge 21 commits into
CEOS-Developers:masterfrom
seondal:master

Conversation

@seondal
Copy link
Copy Markdown

@seondal seondal commented Sep 30, 2022

배포

https://react-messanger-16th-ebon.vercel.app/


아쉬운점

  • 유저목록에서 유저를 선택했을 때 예시처럼 프로필 이미지 위에 반투명한 레이아웃과 선택중 이라는 글자가 뜨는걸 구현하지 못해서 아쉽습니다.. 임시방편으로 선택중인 프로필이미지의 투명도만 낮아지게 구현했어요 ㅠㅠ
  • 채칭내용을 구성할때 ChatItem.tsx 다음과 같이 레이아웃을 구성했는데, 더 효율적이게 구성할 수 있는 방법이 있을지, 구성 컴포넌트 이름은 어떻게 하는게 좋을지 고민입니다

스크린샷 2022-10-01 오전 3 29 04

  • 한 유저가 연속으로 메세지를 보냈을 때 프로필과 이름 없이 말풍선만 쭉 나오게 하는 기능도 구현하고 싶었는데..ㅠㅠ
  • 왜인지 App.tsx 에서 wrapper에 부여한 border 속성이 안먹힙니다.. (정확하게는 다른 컴포넌트들의 배경색에 덮여버려서 border radius를 적용한 border가 보이지 않아요)
  • 한글을 입력할 때 가끔 이상한 오류가 나는데.. 이유를 모르겠어요..
  • 마감후에 생각난 부분들 (코드리뷰 반영할때 추가로 구현할 기능들!)
    • input에 있는 값의 여부에 따라 전송버튼 활성화
    • 스크롤바 디자인 이쁘게 꾸미기
    • 시간 더 정확하게 표시하기
      • 같은날일 경우 → n시간 전
      • 다른날일 경우 → 0월0일

미션 목표

  • TypeScript를 사용해봅니다.
  • useState로 컴포넌트의 상태를 관리합니다.
  • useEffect와 useRef의 사용법을 이해합니다.
  • Custom hooks를 통해 중복되는 로직을 줄이기
  • Styled-components에서 props 사용해보기
  • (선택) Redux, Context API등 Flux 패턴 적용해보기

필수 구현 기능

  • 결과화면과 같이 구현합니다.
  • 채팅방 상단의 프로필을 클릭하면 사용자를 변경할 수 있습니다.
  • 메세지를 보내면 채팅방 하단으로 스크롤을 이동시킵니다. (Hint: useEffect + scrollTo)
  • 메세지에 유저 정보(프로필 사진, 이름)를 표시합니다.
  • user와 message 데이터를 json 파일에 저장합니다.
  • UI는 마음대로 구성하되, 반응형까진 고려하지 않으셔도 됩니다.
  • 그 외 추가하고 싶은 기능이 있다면 마음껏 추가해 주세요!

@JeeeunOh
Copy link
Copy Markdown
Member

JeeeunOh commented Oct 2, 2022

안녕하세요 이번 코드리뷰 파트너 오지은입니다:)
선영님의 채팅방에서 대화에 참여한 유저가 총 세명이라는 것이 인상깊었어요!!!
저와는 다르게 말하는 유저의 프로필이 연하게 표시된 것도 재밌었습니다:)
한글을 입력하면 마지막 글자가 한번 더 입력되는 오류가 발생하는데, 이 오류는 원인을 한번 저도 찾아보겠습니다!!

width: 100%;
background-color: white;
`;
const InputField = styled.textarea`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

styled.input 태그를 이용하면 따로 enter 처리를 해주지 않아도 돼서 편해요!
사용해보셔도 좋을 것 같아요:)

const chatListRef = useRef<HTMLDivElement>(null);
useEffect(() => {
chatListRef.current?.scrollTo(0, chatListRef.current.scrollHeight);
console.log("스크롤!");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

요런 콘솔로그는 없애주시는게 좋을 것 같아요!!!

background-color: white;
border-radius: 10px;
}
::-webkit-scrollbar-track {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

속성을 부여하지 않은 스크롤 커스텀은 없애줘도 될 것 같아요!

Comment thread src/StyledComponents.tsx
height: 40px;
border-radius: 15px;
object-fit: cover;
opacity: ${({ selected }: { selected?: boolean }) =>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

이렇게 props를 전달해줘서 불투명도를 조절하는 것도 너무 좋네요! 배워갑니다:)

Copy link
Copy Markdown
Member

@YooSeonHo YooSeonHo left a comment

Choose a reason for hiding this comment

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

안녕하세요 선영님~!
단톡방을 만드실 줄은 상상도 못 했어요~~~~
4주차 과제 때 편하시겠다 생각했습니다!!! 부러워용
recoil, redux를 이용해서 상태 관리 하는 것도 좋은 방법 일 거 같아요!!!
고생 많으셨고 스터디 때 봬요!! 저희 토론 세션도 함께 한답니당.

@@ -0,0 +1,11 @@
import { Chat } from "../interface";

interface ChatItemProps {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

오 props용 interface를 만들면 더 편하게 할 수 있군요~!

<ChatWrapper>
<ChatBalloon>{chat.text}</ChatBalloon>
<p>
{time}:{minute}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

시간 부분도 구현 잘 하시네요 ㅠ.ㅠ

gap: 10px;
margin: 10px;
justify-content: ${({ isCurUser }: { isCurUser: boolean }) =>
isCurUser ? "flex-end" : "flex-start"};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

오 저는 margin으로 처리했는데 flex로도 해봐야겠어요

onConcat(value);
setValue("");
// 엔터로 전송, shift+엔터로 줄바꿈
const handleEnter = (e: React.KeyboardEvent<HTMLTextAreaElement>) => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

되게 번거로울 거 같아서 input으로 바꿨는데 textArea가 더 좋은 거 같아요 ㅠ.ㅠ

Copy link
Copy Markdown
Member

@SeieunYoo SeieunYoo left a comment

Choose a reason for hiding this comment

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

안녕하세요 선영님 ! 프론트 운영진 유세은 입니다 👍

데모링크 돌려보면서 채팅 인원을 3명으로 구성해주신 부분들에서 인상 깊다고 느꼈습니다! 다른 상태 관리 라이브러리 없이 기능 구현도 잘해주시고 디자인도 잘해주셔서 몇 가지 코멘트만 남겨봤습니다 ~ 참고로 커스텀 훅으로 useInput 같은 훅을 만든다면 로직을 줄여볼 수 있을 것 같습니다.

PR 에 적어주신 border 속성이 적용되지 않은 부분은 저는 예전에 id 로 해결했던 기억이 나네요~ globalStyle 에서 id 로 css 속성 정해주듯이

#border {
 border: 1px solid lightgray;
 border-radius: 20px;
}

이런 식으로 필요한 부분에 border 를 id 로 주면 해결할 수 있지 않을까요?

그럼 이따 스터디 시간에 뵙겠습니다 👋

Comment thread src/App.tsx
Comment on lines +58 to +59
width: 350px;
height: 95vh;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

px 과 vh 차이점을 두신 게 있을까요?!

Comment on lines +29 to +32
<ChatWrapper>
<ChatBalloon isCurUser={false}>{chat.text}</ChatBalloon>
{time}:{minute}
</ChatWrapper>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

이 부분은 위의 코드와 중복되는 부분이 있는 것 같아요 !
isCurUser 를 props 받는 컴포넌트로 분리하는 게 좋을 것 같아요~

Comment thread src/App.tsx
<GlobalStyle />
<UserList curUser={curUser} users={users} changeUser={changeUser} />
<ChatList curUser={curUser} users={users} chats={chats} />
<InputForm onConcat={onConcat} />
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Concat 이라는 네이밍이 모호한 부분이 있는 거 같아서 AddChats 이런 식으로 구체적으로 명시해줘도 좋을 듯 합니다 ~

@9yujin 9yujin closed this Oct 2, 2022
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.

5 participants