-
Notifications
You must be signed in to change notification settings - Fork 10
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주차] 전시원 미션 제출합니다. #18
base: main
Are you sure you want to change the base?
Conversation
socket connection setting both front and back
without TS and commit message
in user boxm, there are information below user name, nickname, profile image, profile music ...
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 Btn = styled.button``; | ||
|
||
export default ChatModal; |
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.
컴포넌트가 많아지다 보니 보기가 좀 어렵네요...ㅜㅜ 기능에 맞게 폴더 분리해보시면 좋을 것 같아요!!!
import { atom } from 'recoil'; | ||
import messageData from '../static/msg.json'; | ||
import userData from '../static/user.json'; | ||
import { IMessage, IUser } from '../../interfaces/data'; | ||
|
||
export const Message = atom<IMessage[]>({ | ||
key: 'message', | ||
default: messageData.rooms, | ||
}); | ||
|
||
export const userInfo = atom<IUser[]>({ | ||
key: 'userInfo', | ||
default: userData.users, | ||
}); |
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.
리코일 적용하셨군요... 저는 적용하다가 중간에 실패했는데 나중에 시원님 코드 참고해서 해봐야겠어요 ,,,!!
String(new Date().getHours()).padStart(2, '0') + | ||
':' + | ||
String(new Date().getMinutes()).padStart(2, '0'); |
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 default MessageBox; |
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.
} else { | ||
const userNameArray = user.map((u) => u.userName).slice(1); | ||
const result = userNameArray.filter((name) => name.includes(text)); | ||
console.log(result); |
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.
앗 console.log 발견 ~ . ~
switch (name) { | ||
case '침착맨bde': | ||
return 0; | ||
case '주호민qwt': | ||
return 1; | ||
case '김풍zxcv': | ||
return 2; | ||
case '태완씨liup': | ||
return 3; | ||
case '슈말코qwejrl': | ||
return 4; | ||
default: |
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.
user.json
에 useId를 숫자로 세팅해주셨던데 userId를 활용해보면 어떨까요,,?
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.
안녕하세요, 코드리뷰 파트너 정대헌입니다. 우선, 코드 공유주셔서 감사해요. 이런 저런 새로운 시도들을 구체화하고 적용하신 부분들이 인상깊었습니다. 다만, 확실히 웹앱이 복잡해질수록 상태에 대한 이해가 중요한 것 같아요. 각 상태의 역할을 추정하고 관계를 이해하는데 어려움이 있었습니다. 저를 포함해서 이런 부분들에 대해 설명을 적는다면 코드를 이해하는데 큰 도움이 될 것 같습니다!
<div>일반채팅</div> | ||
<div>오픈채팅</div> | ||
</div> | ||
<Btn onClick={onToggleChatModal}>❌</Btn> |
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.
<Btn onClick={onToggleChatModal}>❌</Btn> | |
<Btn onClick={()=>{setChatModalToggle(!chatModalToggle)}}>❌</Btn> |
식으로도 할 수 있을까요? Recoil 문법과 맞는지는 모르겠지만...
const [text, setText] = useState(''); | ||
const [prevUser, setPrevUser] = useState<string| null>(''); | ||
const [roomId, setRoomId] = useState<number>(0); | ||
const scrollRef = useRef<HTMLInputElement>(null); | ||
const onChangeTextArea = | ||
(e: React.ChangeEvent<HTMLTextAreaElement>):void => { | ||
setText(e.target.value); | ||
} | ||
|
||
const [chatRoomToggle, setChatRoomToggle] = | ||
useRecoilState(chatRoomToggleState); | ||
const [currentUser, setCurrentUser] = useRecoilState(userClicked); | ||
const [chat, setChat] = useRecoilState(Message); | ||
const ToggleChatRoom = useCallback(() => { | ||
setChatRoomToggle((prev) => !prev); | ||
}, []); |
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.
혹 여유를 가지고 코드를 작성한다면
코드를 읽는 측면에서는 각 state에 대한 주석과 구분이 있다면 훨씬 편할 것 같아요!
아마 바빠서 어쩔 수 없었겠지만요
// when press 'enter', then submit form | ||
// when press 'shift+enter', then change line |
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.
키보드 커맨드 좋은 아이디어 인 것 같아요! keyCode attribute를 통해 간단하게 구현하셨군요.
const location = useLocation(); | ||
|
||
useEffect(() => { | ||
setUrlPath(location.pathname.slice(1)); | ||
}, [location]); |
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.
React Router의 location 함수를 사용하셨군요. 혹시 path를 별도의 state로 관리하시는 이유가 있을까요?
const Main:FC<Props> = ({ children }) => { | ||
return <Container>{children}</Container>; | ||
} |
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 findUrl = () => { | ||
const [a] = user.filter((u) => u.userName === currentUser); | ||
const [b] = user.filter((u) => u.userName === '전시원'); | ||
const source = [b.profileImg, a.profileImg]; | ||
// @ts-ignore | ||
setSource([...source]); | ||
}; |
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.
일반 함수는 컴포넌트 바깥에서 정의하고 사용해도 좋을 것 같아요.
<Container | ||
key={i} | ||
onClick={() => { | ||
setChatRoomToggle((prev) => !prev); | ||
setCurrentUser(d.userName); | ||
}} | ||
> | ||
<ImgDiv> | ||
<Img src={d.profileImg} /> | ||
</ImgDiv> | ||
{urlPath === '' && ( | ||
<NickDiv> | ||
<Name>{d.userName}</Name> | ||
<Intro>{d.introduce}</Intro> | ||
</NickDiv> | ||
)} | ||
{urlPath === 'rooms' && ( | ||
<NickDiv> | ||
<Name>{d.userName}</Name> | ||
<Intro>{d.last}</Intro> | ||
</NickDiv> | ||
)} | ||
{urlPath === '' && <div>{d.profileMusic}</div>} | ||
{urlPath === 'rooms' && <div></div>} | ||
</Container> |
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.
별도의 컴포넌트로 분리해도 좋을 것 같습니다!
<MenuContainer> | ||
{urlPath === '' && ( | ||
<> | ||
<Img src="home_black.png" onClick={() => navigate('/')} /> | ||
<Img src="chat.png" onClick={() => navigate('/rooms')} /> | ||
<Img src="settings.png" onClick={() => navigate('/general')} /> | ||
</> | ||
)} | ||
{urlPath === 'rooms' && ( | ||
<> | ||
<Img src="home.png" onClick={() => navigate('/')} /> | ||
<Img src="chat_black.png" onClick={() => navigate('/rooms')} /> | ||
<Img src="settings.png" onClick={() => navigate('/general')} /> | ||
</> | ||
)} | ||
{urlPath === 'general' && ( | ||
<> | ||
<Img src="home.png" onClick={() => navigate('/')} /> | ||
<Img src="chat.png" onClick={() => navigate('/rooms')} /> | ||
<Img | ||
src="settings_black.png" | ||
onClick={() => navigate('/general')} | ||
/> | ||
</> | ||
)} | ||
</MenuContainer> |
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.
styled component에 인자를 전달하는 방식으로 개선할 수 있을 것 같습니다.
가장 좋은 건 이미지 대신 외부 의존성을 이용하는 방법이나(font awesome이나 react icon등)
혹은 css filter를 이용해서 수정할 수 있을 것 같습니다.
https://developer.mozilla.org/en-US/docs/Web/CSS/filter
#제출
안녕하세요. 세오스 15 프론트 전시원입니다. 과제 제출드립니다. 감사합니다.
[필수 구현 기능]
배포 링크
chat-siwonblue.vercel.app
자정에 회의라 갔다와서 새벽에 다 할 예정
Key Question
1.라우팅
네트워크에서 경로를 설정하는 것
프로트엔드에서 라우팅
브라우저에서 get 요청을 받게 되면 페이지를 전송해주는 역할을 함
리액트에서 npm react-router-dom 을 사용
각 요청에 대해서 어떤 데이터를 전송해줄지 결정짓는 것이 라우팅
2. SPA & CSR & SSR
SPA (Single Page Application)
하나의 페이지 내부에서 여러 데이터를 보여줄 수 있음.
CSR ( Client Side Rendering)
SPA 를 사용하게 되면서 CSR 이 주목받기 시작
처음 로딩하게 되면 정적파일(HTML) 을 먼저 제공
그 후에 데이터를 받아와서 다시 채워넣는 과정을 거침
처음에 데이터가 없기 때문에 SEO(검색엔진 최적화)에 불리함
하지만 사용자에게 빠르게 화면을 보여줄 필요가 있는 경우 사용
SSR (Server Side Rendering)
한번에 모든 데이터를 받아온 후에 화면으로 보내줌
CSR 에 비해 로딩 속도가 느림
검색엔진 최적화가 되어 있음
prerender, SSR 두 가지 방식이 있음.
pre render 는 검색엔진이 아닌 경우 CSR 처럼 동작하여 빠르게 페이지 전달
SSR 은 첫 방문만 모든 데이터 전송, 그 후에는 CSR 처럼 동작