-
Notifications
You must be signed in to change notification settings - Fork 34
[Team-06-FE] 3주차 PR #54
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
[Team-06-FE] 3주차 PR #54
Conversation
- 파일 업로드 로직 리팩토링 - Error 컴포넌트 리팩토링 - home에 item, category 컴포넌트들 key값 index에서 고유한 id로 변경 - home에 item list가 비어 있는 경우 예외 처리 추가 - App에 useEffect event 추가, 삭제 callback 변경 - useScreenConfigStore에 set 수정
…toring 사소한 refactoring
* feat: 레디스를 통한 좋아요, 조회수 구현 * feat: 상품 등록, 조회 시 stat 기능 추가 * chore: 레디스 의존성, properties 추가 * chore: 상품 상세 조회 응답에 stat 추가 * feat: RedisConfig 추가 * rename: 접근제어자, 디렉토리, 클래스명, 메소드명 수정 * refactor: 메소드 분리, 메소드명 수정
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.
리태, 아티 안녕하세요!
코드가 가독성이 좋네요! 이번 리뷰도 수월했습니다. 팀원을 배려하며 코드를 작성해주신 게 눈에 보여요👍
Oauth 로그인 구현은 어렵지 않으셨나요? 리액트 쿼리, 테스트 코드도 작성해보셨군요.
매주 새로운 시도를 하며 성장하시는 모습을 보여 저도 본받고 갑니다👏
3주차도 고생 많으셨어요!
추가로 궁금하신 사항 있으시면 코멘트 달아주세요~
[궁금한 점에 대한 답변]
버튼 중 텍스트가 '홈화면'인 버튼을 제대로 못가져오는 게 아닌가 싶은데요..
const homeButton = getByRole('button', { name: "홈화면" }) 으로 해도 잘 안되던가요?🤔
|
|
||
| export const AccountPage = () => { | ||
| const [profileImg, setProfileImg] = useState<File>(); | ||
| const isLogin = 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.
[질문]
엇..상태로 관리해야하지 않을까요? 🤔
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.
UI 작업하면서 테스트용으로 썼어요😅 업데이트 하면서 변경하겠습니다!
| useEffect(() => { | ||
| if (hasName) return; | ||
| setRecommendCategories(generateRecommendCategory(category)); | ||
| setSelectedCategory(undefined); |
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.
비유가 쉽고 재밌네요!!😁 null과 undefined을 구분해서 잘 사용하겠습니다👍
| const URL = window.location.href; | ||
| const parts = URL.split('?'); | ||
| const queryString = parts[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.
[제안]
URL API를 쓰시면 더 간단하게 작성해볼 수 있겠네요~
URL: searchParams property
| ]; | ||
|
|
||
| export const HistoryPage = () => { | ||
| const [selectedCategory, setSelectedCategory] = useState<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.
[제안]
data 값을 보아하니 string type보다 좁힐 수 있을듯 합니다~!
| onClick(): void; | ||
| }; | ||
|
|
||
| export const Tag: React.FC<Props> = ({ title, isSelected, onClick }) => { |
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.
[제안]
Tag 라는 컴포넌트 명이 너무 일반적이라고 느껴집니다. 좀 더 구체화 하면 좋겠네요~
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.
컴포넌트 네이밍 할 때 조금 더 구체적으로 작성하겠습니다🙌
| <Fab> | ||
| <FabButton onClick={() => navigate('/add')} /> | ||
| </Fab> |
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.
[질문]
혹시 추후 FabButton에 Add 외의 다른 요소를 구현하실 계획이실까요?
그게 아니라면 add 버튼 용으로 리팩터링해도 괜찮겠네요~
| import displayTimeAgo from '../utils/displayTimeAgo'; | ||
|
|
||
| describe('displayTimeAgo 함수 테스트', () => { | ||
| it('초단위로 경과된 시간을 올바르게 표시해야 한다', () => { |
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.
[제안]
'올바르게 표시해야한다'라는 표현은 테스트 케이스를 명확히 전달하기 어려워 보이는데요.
어떤 형태의 값이 반환되어야 하는지 구체적으로 작성해보면 어떨까요?
| @@ -0,0 +1,24 @@ | |||
| import formatPrice from '../utils/formatPrice'; | |||
|
|
|||
| describe('formatPrice 함수 테스트', () => { | |||
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.
👍👍
|
|
||
| <Content> | ||
| {items.length === 0 ? ( | ||
| <EmptyMessage>판매 내역이 없습니다</EmptyMessage> |
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.
[제안]
- 아이템이 0개일 때 노출된다.
- '판매 내역이 없습니다' or '관심 상품이 없습니다' 2가지 문구가 존재한다.
- 스타일이 동일하다.
위 공통점을 토대로 EmptyMessage를 공통 컴포넌트로 구현하면 좋겠네요~
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.
저도 여기 작성하면서 History페이지와 Liked 페이지가 비슷하다고 생각되면서 작성하긴 했었네요!
한번 만들어보겠습니다!
| }; | ||
|
|
||
| export const usePrefetchCategoriesWithoutImages = (): void => { | ||
| const queryClient = useQueryClient(); |
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.
[질문]
useQueryClient를 사용하셨군요~
usePrefetchCategoriesWithoutImages는 어떤 경우에 사용하실 계획이신가요?
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.
제품 상세 페이지에서 상품 이름을 입력하면 랜덤으로 카테고리를 추천해주는 기능이 있어서, 해당 기능에서 사용하려고 했습니다
* #51 chore: redis 설정 추가 * #51 feat: 로그아웃 기능 구현 - 로그아웃 api 요청 시 redis에 엑세스토큰을 저장하고 db에서 리프레쉬토큰을 삭제합니다. * #51 chore: 테스트에 redis 설정 추가 * #51 refactor: logout 메서드를 AuthService로 이동 * #51 test: Redis 연결 테스트 * feat: logout 기능 필터 제외 * fix: claim이 추가되지 않는 문제 해결 * chore: docker 환경에 redis 컨테이너 추가 * #51 refactor: 리뷰반영 수정 * #51 refactor: 리뷰반영 수정 --------- Co-authored-by: Jeongyong Park <1239824@naver.com>
…e#48 docs: RestDocs 이미지 request 변경
* feat: SSE notification 추가 * feat: 채팅 알림 기능 추가 * refactor: join on 조건, 안 쓰는 부분 제거 * feat: 안 읽은 전체 채팅 개수 조회 기능 추가 * fix: 컨트롤러에서 sseEmitter 반환 * refactor: 상대방이 채팅방에 접속해 있을 때는 알림을 보내지 않도록 수정

[TEAM#06-FE] 3주차 PR
데이지 안녕하세요!! 벌써 반절이나 지나서 3주차가 되었네요!
이번주는 백엔드와 Oauth 로그인 구현과 각 페이지별 UI 작업을 진행했어요🙌
그리고 추가적으로 간단한 api 작업과 환경변수 설정, 유틸함수의 테스트 코드를 추가하였습니다.
저희 조 진행 속도가 빠른 편은 아니지만 아티랑 다음주부터 열심히 또 진행해볼게요! 감사합니다😀
이번 주 진행사항
진행 상황 미리보기(배포링크)
2023-09-08.5.33.50.mov
고민이나 궁금한 점 👋
[고민한 부분]
[궁금한 점]
util 함수 외의 동작 관련 테스팅을 해보고 싶어서 NavigationBar 클릭 했을 때 탭 안의 글자색을 통해 제대로 활성화 되는지 테스트를 해보고 싶었습니다. 기본으로 홈메뉴가 활성화 되어있는 상태라 홈메뉴
const homeButton = getByRole('button', { name: /홈화면/i });를toHaveStyle로 먼저 기본 색상이 제대로 들어가 있는지 확인하려고 했는데, test 해보면 color 색상이 제대로 안뜨고 결과에ButtonText라고 나오더라구요.. 이유를 모르겠어서 해결하지 못했네요.🥲 (이 코드가 현재 PR에 반영되어있진 않습니다)관련 링크