Skip to content

Conversation

@wodnd0131
Copy link

@wodnd0131 wodnd0131 commented Jun 16, 2024

주요 기능

  • 리뷰 수정 시 이미지 등록
    (단, 현재 백앤드 방침에 따라, 이미지는 전체 수정만 가능합니다.)
  • 리뷰 등록 시 loading svg 출력하여 이미지 깨짐 방지
  • 고객의 '나의 리뷰 관리'에 대한 my Page 추가
  • 디자인 개선

@wodnd0131 wodnd0131 changed the title Public view image 조회,수정 개선 myPage 개선 Public view image 조회,수정 개선 및 myPage 추가 Jun 16, 2024
Copy link
Contributor

@hello-mansoo hello-mansoo left a comment

Choose a reason for hiding this comment

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

내일 오후 2시 정도에 머지 후 배포 에정이니 코드 리뷰 사항 중 고칠 수 있는 부분은 최대한 고쳐 주시고 테스트 많이 돌려주세요 데모 과정에서 문제 발생하지 않도록..!

그리고 준서님 PR에서 봤던 리뷰 사항과 동일한 부분이 보이는 것 같은데 git branch 가 어디서 뻗어 나온 걸까요...??


import CloseButton from '@/components/close-button';
import MyReviewList from '@/components/review/my-list';
import CloseButton from '@/components/custom-button';
Copy link
Contributor

Choose a reason for hiding this comment

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

Q. Custom Button이라는 파일 안에서 Close Button이랑 다른 버튼도 관리하는 건가요...? 일단 디렉토리가 세분화 되더라도 명확하게 의존성이 이해되도록 웬만하면 컴포넌트 이름과 파일 이름을 맞춰 주세요!

디자인 시스템에서 쓰는 방식처럼 컴포넌트들을 모아둘 index 파일을 만들고 거기서 export {} 이런 식으로 하는 방법도 있습니다

import CloseButton from '@/components/close-button';
import MyReviewList from '@/components/review/my-list';
import CloseButton from '@/components/custom-button';
import MyReviewListOnProduct from '@/components/review/my-review/my-review-list';
Copy link
Contributor

Choose a reason for hiding this comment

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

my-review/list -> 이렇게 이미 my-review 라는 바운더리로 묶인 후에 안에서 구분되는 행위로 디렉토리 네임을 간소화 할 수 있지 않을까요..?!

<div className="flex flex-col p-4">
<div className="flex gap-0.5 m-2 items-center w-fit">
<Star
setStar={() => {}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Star의 setStar가 Read 목적일 때는 쓰일 일이 거의 없는 것 같은데 향후 optional props로 개선해야겠네요

}, [reviewDetailQuery.data]);

useEffect(() => {
const textarea = document.querySelector('textarea');
Copy link
Contributor

Choose a reason for hiding this comment

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

textarea가 문서 상에서 여러 개가 될 확률은 없나요? 웬만하면 혹시나 하는 상황이 발생하지 않게 명확하게 scope를 selector로 정의해주거나 id, class 등을 활용해주세요! 여러 개를 선택해야 하는 상황이면 querySelectorAll을 사용해주시고...

useRef로 ref로 연결하는 걸 권장합니다!

Comment on lines 111 to 122
placeholder="리뷰를 작성해주세요. 10자 이상 작성해주세요!"
ref={textareaRef}
rows={6}
value={content}
/>
<div className="absolute right-1 bottom-0 pr-4 text-gray-400">{content.length}/2000</div>
</div>
<div
className="flex flex-row-reverse pr-4"
hidden={content.length >= 10 || content.length === 0}
>
<div className="text-gray-500">*10자 이상 입력하시면 댓글 등록이 가능합니다!</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

전체적으로 안내 문구에 느낌표 제거해주세요~

@@ -0,0 +1,27 @@
import tw, { styled } from 'twin.macro';

import ClosingSvg from '@/assests/icon/icon-closing.svg';
Copy link
Contributor

Choose a reason for hiding this comment

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

ClosingIcon 등으로 파일 타입보다 명확하게 네이밍 하는 게 좋습니다

${tw`m-2 p-2`}
border: 2px solid ${({ isActive }) => (isActive ? '#5C6BC0' : '#9E9E9E')};
background-color: ${({ isActive }) => (isActive ? '#1E88E5' : '#BEBEBE')};
color: white;
Copy link
Contributor

Choose a reason for hiding this comment

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

color는 브라우저마다 white, red 등의 hex 값이 다를 수 있기 때문에 명확하게 #fffff 로 해 주세요

ImageUrls.map((imageUrl: string, index: number) => (
<div
className="my-5"
key={index}
Copy link
Contributor

Choose a reason for hiding this comment

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

index도 좋지만 key에는 난수 라이브러리를 활용해 난수를 넣기도 합니다.
향후 참고해주세요

Comment on lines 27 to 38
filters={[
{
value: 'ALL',
label: '전체',
},
{
value: 'IMAGE_VIDEO',
label: '포토/동영상',
},
{
value: 'GENERAL',
label: '일반',
Copy link
Contributor

Choose a reason for hiding this comment

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

이런 부분은 constant로 관리하는 게 좋습니다. 특히 shop-admin에서도 동일하게 쓰이는 필터일 확률이 크니 theme 등의 모노레포 패키지에 관리할 필요가 있는지 개발 과정에서 고려해주세요


export interface RetrieveMyReviewListRequest {
mallId: string;
memberId: string | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

undefined면 memberId?: string으로 해 주세요

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
29.4% Duplication on New Code (required ≤ 10%)

See analysis details on SonarCloud

@wodnd0131
Copy link
Author

어제 기준 devlop 브랜치입니다!
준서님 pr에서 파생시킨건 아니라 머지할게 좀 있을 거 같습니다..
너무 늦게 확인해서, 당장 반영 가능한 피드백 우선처리했습니다.
(리뷰에 이모지 달린 건 당장은 보류된 것입니다).

wodnd0131 added 6 commits June 17, 2024 16:28
…image

# Conflicts:
#	apps/web/src/app/mypage/[productID]/connected-page.tsx
#	apps/web/src/app/products/[productID]/reviews/connected-page.tsx
#	apps/web/src/app/reviews/[reviewID]/connected-page.tsx
#	apps/web/src/components/review/board-style-item.tsx
#	apps/web/src/components/review/list.tsx
#	apps/web/src/components/review/my-review/infinite-list.tsx
#	apps/web/src/components/review/my-review/list.tsx
@wodnd0131
Copy link
Author

메인에 배포된 준서님 코드에 빌드 에러만 수정한 pr입니다

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.

4 participants