-
Notifications
You must be signed in to change notification settings - Fork 11
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주차] 노이진 미션 제출합니다. #18
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 이진님!! 깔끔한 주석 작성과 흥미로운 로직들이 많아 저도 코드 보면서 새롭게 알게된 정보들도 배워가고 여러 생각들을 해보게 되어 좋았습니다 ㅎㅎ 수고 많으셨어요~!!
|
||
const GlobalStyle = createGlobalStyle` | ||
|
||
${reset} | ||
*, *::before, *::after{ | ||
box-sizing: border-box; | ||
} | ||
body{ | ||
display: flex; | ||
padding: 0; | ||
margin: 0; | ||
justify-content: center; | ||
font-family: "Pretendard-Regular"; | ||
background-color: ${colors.grey_900}; | ||
}; | ||
` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분도 style에 관련된 코드라서 style 폴더로 따로 빼도 좋을 것 같아요 ㅎㅎ
.filter((user: { uid: number; userName: string }) => | ||
user.userName.toLowerCase().includes(searchValue.toLowerCase()), | ||
) | ||
.filter((user: { uid: number; userName: string }) => chatData[user.uid]?.chat.length > 0) | ||
.sort((a: { uid: number }, b: { uid: number }) => { | ||
//마지막에 보낸 채팅방이 가장 위로 올 수 있도록 | ||
const lastChat_1 = new Date(chatData[a.uid]?.chat[chatData[a.uid]?.chat.length - 1]?.time).getTime() | ||
const lastChat_2 = new Date(chatData[b.uid]?.chat[chatData[b.uid]?.chat.length - 1]?.time).getTime() | ||
return lastChat_2 - lastChat_1 | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
렌더링 하는 부분에서 filter를 수행하는 것 보다는 위에서 filter를 수행 후 얻은 데이터만 가지고 map 함수를 진행하는 것이 좀 더 깔끔할 것 같다는 개인적인 의견 드립니다 ㅎㅎ
src/components/ChatList.tsx
Outdated
{format( | ||
new Date(chatData[user.uid]?.chat?.[chatData[user.uid]?.chat.length - 1]?.time), | ||
today === | ||
format( | ||
new Date(chatData[user.uid]?.chat?.[chatData[user.uid]?.chat.length - 1]?.time), | ||
'yyyy/MM/dd', | ||
) | ||
? 'hh:mm' | ||
: 'MM/dd', | ||
) || ''} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분도 따로 함수로 뺴서 해당 함수를 호출만 해주는 형식으로 수정하면 가독성에 더 좋을 것 같아요!!
` | ||
const ChatName = styled.span` | ||
width: 100%; | ||
color: ${colors.grey_900}; | ||
font-family: 'Pretendard-Medium'; | ||
font-size: 1.125rem; | ||
line-height: 140%; /* 1.575rem */ | ||
` | ||
const ChatContent = styled.div` | ||
color: ${colors.grey_400}; | ||
width: 100%; | ||
font-family: 'Pretendard-Regular'; | ||
font-size: 0.875rem; | ||
line-height: 0.875rem; | ||
margin-top: 0.19rem; | ||
` | ||
const ChatTime = styled.span` | ||
color: ${colors.grey_400}; | ||
font-family: 'Pretendard-Regular'; | ||
font-size: 0.75rem; | ||
line-height: 0.875rem; | ||
margin-top: 0.56rem; | ||
` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
매 컴포넌트마다 폰트 스타일을 지정해 주면 번거로운 작업이 될 수 있어서 자주 쓰이는 폰트 스타일들을 따로 파일에 지정해 준 후 가져다 써도 좋을 것 같아요!!
{userData | ||
.filter((user: { uid: number; userName: string }) => | ||
user.userName.toLowerCase().includes(searchValue.toLowerCase()), | ||
) | ||
.map((user: { uid: number; userName: string }) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 데이터 및 검색에 입력한 텍스트를 모두 lowerCase 로 바꿔서 대문자로 검색해도 해당 문자의 소문자가 검색될 수 있도록 하셨네요!! 저는 생각하지 못한 디테일이여서 저도 나중에 이렇게 설정해야겠어요 ㅎㅎ
<IconBox | ||
onClick={() => { | ||
setIsChatting(true) | ||
setIsFriend(false) | ||
setIsSetting(false) | ||
}} | ||
> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재는 tabBarState의 아이콘마다 상태 관리를 하고 있어서 하나의 아이콘 클릭시 나머지 아이콘의 상태를 false 로 만들어줘야 해서 setState 함수를 3번 호출하고 있는데 tabBarState 라는 변수에 타입을 "isChat" | "isFriend" | "isSetting" 이렇게 string 으로 지정해주고 예를 들어 채팅 아이콘을 클릭하면 해당 상태로 바뀌는 로직이 한번만 실행될 수 있게 짜셔도 좋을 것 같아요!!
<IconBox | |
onClick={() => { | |
setIsChatting(true) | |
setIsFriend(false) | |
setIsSetting(false) | |
}} | |
> | |
const enum TABBAR_STATE{ | |
'IS_CHAT', 'IS_FRIEND', 'IS_SETTING' | |
} | |
const [tabBarState,setTabBarState]=useState<number>(TABBAR_STATE.IS_CHAT) | |
... | |
<IconBox | |
onClick={() => { | |
setTabBarState(TABBAR_STATE.IS_CHAT) | |
}} | |
> | |
... | |
<IconBox | |
onClick={() => { | |
setTabBarState(TABBAR_STATE.IS_FRIEND) | |
}} | |
> | |
... | |
<IconBox | |
onClick={() => { | |
setTabBarState(TABBAR_STATE.IS_SETTING) | |
}} | |
> | |
... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다음에는 꼭 이렇게...!! 해보겠습니다
export const colors = { | ||
green: '#4EDB28', | ||
purple: '#5D5FEF', | ||
white: '#FFFFFF', | ||
|
||
grey_50: '#F6F6FA', | ||
grey_100: '#DEE1E9', | ||
grey_300: '#C6CCD4', | ||
grey_400: '#A1A8B2', | ||
grey_700: '#686A73', | ||
grey_900: '#292929', | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
자주 사용하는 색깔들을 colors 파일에 깔끔히 정리하셨네요!! ㅎㅎ styled-components 에서 제공하는 ThemeProvider 도 한번 사용해보시면 좋을 것 같아요~~
{ | ||
"printWidth": 120, | ||
"tabWidth": 2, | ||
"singleQuote": true, | ||
"trailingComma": "all", | ||
"semi": false | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prettier 설정을 직접 하셨네요!! 멋집니다 ㅎㅎ
src/components/ChatList.tsx
Outdated
const editContent = (content: String) => { | ||
if (content.length >= 18) { | ||
return content.substring(0, 18) + '...' | ||
} else return content | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
substring 을 활용하여 글자 수가 넘어가면 ... 로 대체되게 하셨네요!! 제 코드에도 한번 적용해봐야겠어요 배워갑니다 ㅎㅎ
export const ChatList = ({ searchValue }: { searchValue: string }) => { | ||
const [isNew, setIsNew] = useState(false) | ||
const [chatData, setChatData] = useRecoilState(chatDataState) | ||
const today = format(new Date(), 'yyyy/MM/dd') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
format 이라는 라이브러리는 처음 봤는데 확실히 사용하면 편리할 것 같네요!! 새로운 정보 알아갑니다 ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
발생할 수 있는 모든 예외적인 상황을 신경쓰셔서 코드 리뷰하면서 감탄했습니다 ㅎㅎ
이번 과제 수행하느라 정말 고생 많으셨습니다~!
src/assets/images/profile9.png
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
강아지가 너무 귀엽네요..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅋㅋㅋ 동의합니다...
src/components/ChatList.tsx
Outdated
//채팅 목록에서 18글자가 넘어가면 ...표시 되도록 | ||
const editContent = (content: String) => { | ||
if (content.length >= 14) { | ||
return content.substring(0, 14) + '...' | ||
} else return content | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전 이 예외처리를 고려하지 못했는데 이진님은 디테일한 부분까지 고려를 잘 하신 것 같아요~👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
디테일하지 못한 주석 바로 바꿨습니다...ㅎㅎㅎㅎㅎ
<a | ||
href="https://github.com/leejin-rho" | ||
target="_blank" | ||
rel="noopener noreferrer" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a
태그에서 rel
이라는 속성이 익숙치 않아 찾아보니까 rel
속성은 주로 검색 엔진 최적화를 위해서 쓰이게 되고, noopener noreferrer
의 경우에는 `target="_blank"의 보안상 취약점을 해결하기 위해 쓰인다는 걸 알게 되었네요. 덕분히 하나 더 배워갑니다 :)
<> | ||
<TopHeading> | ||
<UserContainer> | ||
<EditIcon style={{ width: '1.9rem', color: isSetting ? colors.white : colors.grey_700 }} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in-line 스타일이 코드를 완성하는 편의성 면에서 좋지만, 리액트 공식 문서에 따르면 in-line 스타일의 경우 성능이 낮다고 합니다, 대체제로써 스타일 컴포넌트에 prop
을 전달하는 방법으로 스타일 로직을 구현할 수도 있을 거 같습니다!
setIsSetting(false) | ||
}} | ||
> | ||
<ChattingIcon style={{ color: isChatList ? colors.purple : colors.grey_400, width: '1.75rem' }} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아이콘의 css 속성에 cursor: pointer
가 추가되면 더 좋은 사용자 경험을 줄 수 있을 것 같습니다!
<Route path="/" element={<Messenger />}></Route> | ||
<Route path="/chatting/:id" element={<Chatting />}></Route> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
라우팅 구조를 채팅방과 메인 화면으로만 나누고, 메인 화면에서 보여지는 화면을 상태 관리를 통하여 분리하고 로컬 스토리지에 상태를 연동하셨는데, 프로필, 채팅방, 친구 페이지를 따로 라우팅으로 지정하지 않고 상태 관리로써 구현하신 게 인상깊었습니다:)
{format( | ||
new Date(chatData[user.uid]?.chat?.[chatData[user.uid]?.chat.length - 1]?.time), | ||
today === | ||
format( | ||
new Date(chatData[user.uid]?.chat?.[chatData[user.uid]?.chat.length - 1]?.time), | ||
'yyyy/MM/dd', | ||
) | ||
? 'hh:mma' | ||
: 'MM/dd', | ||
).replace(' ', '') || ''} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
마지막 메시지의 날짜(일)이 현재 시점과 다를 때 보일 것까지 고려하셨네요!! 이런 디테일까지 챙기신 점 대단합니다 👍👍
const handleInputClick = (e: React.MouseEvent<HTMLInputElement>) => { | ||
e.preventDefault() | ||
e.stopPropagation() | ||
inputRef.current?.focus() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
옵셔널 체이닝을 사용하여서 inputRef
의 current
값이 null
인 경우 에러 발생을 방지하여 프로그램의 안정성을 높인 점 잘하신 거 같습니다 :)
[4주차]
이번 과제를 하면서도... 뭔가 컴포넌트 분리가 완전 제대로 되진 않은 것 같아서 다음 과제에는 시정하겠습니다...!!!
그리고 제가 deploy 과정에서 실수를 해서... Redeploy commit이 굉장히 많은데 그건 무시해주시면 감사하겠습니다.
이번 과제에서는 나름대로 디테일한 기능들을 신경쓰려고 노력했습니다..!
채팅방에서는 위쪽에 ... 을 누르시면 사용자를 변경할 수 있어요.
각각 탭바 누르면 나오는 화면들이 디자인이 비슷한 부분이 많아서 따로 page로 만들지않고 component로 켰다 껐다하게 구현해보았습니다.
[디자인 및 플로우]
[배포링크]
react-messenger-18th
[Key Questions]
디자이너로부터 받은 QA 목록, QA 반영한 커밋(or 브랜치) 링크
처음에 기능관련 설명해주신 이후에 제가 구현한 것들이 다 맞게 잘 구현했다고 하셔서..... 반영한 사항이 따로 없습니다....
Routing
이번 과제에서는 친구목록, 채팅목록, 설정은 각각 component로 만들어서 tapBar.tsx에서 관리했고, 각각의 채팅방을 “/chatting/*”주소로 router를 이용했다.
Routing이란 간단하게 말하면 사용자가 요청한 URL에 따라 해당 URL에 맞는 페이지를 보여주는 것이라고 할 수 있는데, 나는 여러 라우터 라이브러리 중 리액트 라우터(React Router)를 이용해 구현했다. 컴포넌트는 여러 를 감싸서 그 중 규칙이 일치하는 라우트 단 하나만을 렌더링 시켜준다.
SPA
Single Page Application(SPA)는 그 전 모든 페이지를 재렌더링 했던 것과 달리 특정 변경이 일어나는 부분만 Ajax등을 이용해 데이터를 바인딩해준다. 하지만 자바스크립트로 DOM 조작이 빈번하게 일어나면 브라우저의 성능을 저하하고, 그래서 Virtual DOM이라는 개념이 생겨났다. SPA 프레임워크인 React, Angular, Vue는 대표적으로 Virtual DOM을 이용해 SPA를 구현하는 기술들이다. SPA 프레임워크는 처리 과정이 효율적이며 속도가 빠르지만 처음 화면을 로딩할 때 로딩 속도가 다소 걸린다는 단점이 있다.
상태관리
이번 프로젝트에서는 recoil 라이브러리를 이용해서 chatData의 상태를 관리했다. 각 user의 채팅 데이터들을 chatData에 id순서대로 넣어서 각각의 채팅데이터를 만들었고, 이를 이용해 chatList에서 마지막 채팅과 시간을 나타나는데에도 사용했다.
Recoil은 상태관리를 쉽게 해주는데, Recoil의 Atoms는 state의 단위이며 업데이트와 구독이 가능하다. atom 값을 읽는 컴포넌트들은 암묵적으로 atom을 구독하기 때문에 atom에 어떤 변화가 있다면 그 atom을 구독하는 모든 컴포넌트가 리렌더링된다.
등등 여러 사용방법이 있는데, 이번 과제에서 나는 useRecoilState를 주로 사용했다.