Skip to content
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

feat: 리뷰 작성 폼 개편 #77

Merged
merged 28 commits into from
Apr 14, 2024
Merged

feat: 리뷰 작성 폼 개편 #77

merged 28 commits into from
Apr 14, 2024

Conversation

Leejin-Yang
Copy link
Contributor

Issue

✨ 구현한 기능

  • 리뷰 작성 페이지 추가
  • 태그 선택 바텀시트 추가
2024-04-07.13.34.16.mov

📢 논의하고 싶은 내용

  • 태그 추가까지 하면 바텀바텀시트여서 리뷰 작성 페이지로 추가했습니다. 우선 ${PATH.PRODUCT_LIST}/:category/:productId/review-register 의 경로로 가게 했는데 추천 부탁드려요.
  • TopBar가 추가되면 등록까지 진행할 예정입니다.
  • 사진 등록하고 미리보기 및 삭제는 현재 디자인이 없어서 여쭤봐야합니다.
  • 폼에서 태그 삭제시 x 버튼이 넘 작아서 전체 클릭으로 지울수 있게 했는데 의견주세요.!
  • [질문] 바텀시트 높이 조절 어떻게 했죠? 저희 태그 무조건 세개 선택인가요?

🎸 기타

  • 특이 사항이 있으면 작성합니다.

⏰ 일정

  • 추정 시간 : 4시간
  • 걸린 시간 : 4시간

Copy link

github-actions bot commented Apr 7, 2024

🔗 배포된 storybook 주소: https://65f015a4aed45406385006ee-bkohzwqjhr.chromatic.com/

@hae-on
Copy link
Contributor

hae-on commented Apr 7, 2024

황펭 지금 코드를 볼 수 없는 상황이라 미리 답변 남기자면...

태그 추가까지 하면 바텀바텀시트여서 리뷰 작성 페이지로 추가했습니다. 우선 ${PATH.PRODUCT_LIST}/:category/:productId/review-register 의 경로로 가게 했는데 추천 부탁드려요.

저희 태그 추가도 페이지 아닌가여?? 저는 리뷰 작성 페이지에 버튼 누르면 태그 추가 페이지로 이동한다고 생각했습니다! 피그마 시안도 페이지 같게 생기고 또 바텀시트 높이가 한정 되어 있습니다! 디자이너분들이 최대 높이 따로 정해주셨어요! 그래서 디자인 시스템에서 (아직 머지는 안했다만...) 제가 최대 높이 지정해버렸는디...? 400몇 픽셀임!

폼에서 태그 삭제시 x 버튼이 넘 작아서 전체 클릭으로 지울수 있게 했는데 의견주세요.!

전체 클릭으로 지운다는게 어떤건가요? 동영상에는 따로 지우는게 안보이는데 스토리북 가면 지워볼 수 있나요??

[질문] 바텀시트 높이 조절 어떻게 했죠? 저희 태그 무조건 세개 선택인가요?

위에 답하긴 했는데 지금은 아마 maxHeight로 될껄여? 저거 페이지인지 바텀시트인지 디자이너분들께 물어보시죠! 그리고 태그 선택 갯수도 우리 회의 때 말해봐야 할 듯? 3개 넘 작다는 사람도 있었어서...

Copy link
Member

@xodms0309 xodms0309 left a comment

Choose a reason for hiding this comment

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

안녕하세요 황펭 변경점 진짜 많네요..!!!! 고생하셨습니다!코멘트 확인해주세요~~~
저는 태그 선택하는 부분 바텀시트가 좀 더 자연스러운 느낌인 것 같긴 해요 저기서 페이지 또 들어가면 너무 뎁스가 깊어지는 느낌이라서

@@ -41,33 +46,17 @@ const ImageUploader = ({ previewImage, uploadImage, deleteImage }: ReviewImageUp
</Button>
</PreviewImageWrapper>
Copy link
Member

Choose a reason for hiding this comment

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

요거 피그마에 올라왔네용

Comment on lines 29 to 31
export const label = style({
fontWeight: 600,
});
Copy link
Member

Choose a reason for hiding this comment

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

Text 컴포넌트를 쓸 수 있을 것 같아요 참고로 크기도 body입니다!

{reviewFormValue.tags.map((tag) => (
<li key={tag.id}>
<button type="button" onClick={handleTagSelect(tag)} className={tagButton}>
<span>{tag.name}</span>
Copy link
Member

Choose a reason for hiding this comment

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

텍스트 크기 caption2입니다

userSelect: 'none',

selectors: {
[`${checkbox}:checked + &`]: {
Copy link
Member

Choose a reason for hiding this comment

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

오..이렇게 쓰는거군.... selector 쓰기 어렵던데

);
};

export default ReviewTagSheet;
Copy link
Member

Choose a reason for hiding this comment

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

얘도 스토리북 만들어주세욥

Comment on lines +13 to +14
type ReviewFormValue = Omit<ReviewRequest, 'tagIds'> & { tags: TagValue[] };
type ReviewFormValues = Exclude<ReviewFormValue[keyof ReviewFormValue], TagValue[]> | TagValue;
Copy link
Member

Choose a reason for hiding this comment

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

이거 두 개가 어떻게 다른가요... 코드만 봐서는 모르겠어요,,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

작명의 어려움이 있어서.. 위에껀 객체 타입, 아래껀 그 값들의 타입입니다

Comment on lines 31 to 35
export const status = style({
fontSize: '1.2rem',
fontWeight: 500,
color: vars.colors.gray3,
});
Copy link
Member

Choose a reason for hiding this comment

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

이거도 Text 컴포넌트 쓸 수 있을 것 같아요 (피그마에 색상이 변경되어있네요)

Comment on lines +10 to +12
export const itemTitle = style({
fontSize: '1.6rem',
fontWeight: 600,
Copy link
Member

Choose a reason for hiding this comment

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

Text컴포넌트로 써주시고 나머지 스타일은 className으로 넘겨주세요!

import NotFoundPage from '../NotFoundPage';

import { ReviewRegisterForm } from '@/components/Review';
import ReviewTagSheet from '@/components/Review/ReviewTagSheet/ReviewTagSheet';
Copy link
Member

Choose a reason for hiding this comment

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

요거 index.ts에서 export 해주세요!

@@ -151,6 +151,15 @@ const router = createBrowserRouter([
return { Component: ProductDetailPage };
},
},
{
path: `${PATH.PRODUCT_LIST}/:category/:productId/review-register`,
Copy link
Member

Choose a reason for hiding this comment

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

요기도 그럼 url 바뀌나요? 아니면 상관 없나요? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

바껴야겠네요 추천 받습니다

@Leejin-Yang
Copy link
Contributor Author

@hae-on

전체 클릭으로 지운다는게 어떤건가요? 동영상에는 따로 지우는게 안보이는데 스토리북 가면 지워볼 수 있나요??

태그를 눌렀을 때 지울 수 있게 하는게 어떤지입니다. x 버튼이 너무 작아서요

@Leejin-Yang
Copy link
Contributor Author

@xodms0309 @hae-on

우선 태그는 바텀시트로 선택은 1~3개로 진행하겠슴다
리뷰 작성 url은 더 좋은 의견 있음 주세요 🙏

@hae-on
Copy link
Contributor

hae-on commented Apr 10, 2024

디자인 시스템에 바텀시트 최대 높이 400 몇으로 제한 둔 거 풀어둘게요! 물론 아직 배포는 안해서 상관 없지만~~~

태그를 눌렀을 때 지울 수 있게 하는게 어떤지입니다. x 버튼이 너무 작아서요

눌러서 지우는 거 좋음!!

우선 태그는 바텀시트로 선택은 1~3개로 진행하겠슴다

예 일단 이렇게 진행하시고, 회의때 몇개 할 지 이야기 해보죠???

리뷰 작성 url은 더 좋은 의견 있음 주세요 🙏

이거 바뀌는게 상품 상세에 category 빠져서 그런거에여??

@Leejin-Yang
Copy link
Contributor Author

이거 바뀌는게 상품 상세에 category 빠져서 그런거에여??

옙 그것도 있고, 그냥 막 지은거라 ㅎㅎ..

@hae-on
Copy link
Contributor

hae-on commented Apr 11, 2024

옙 그것도 있고, 그냥 막 지은거라 ㅎㅎ..

전 카테고리만 빠지면 나쁘지 않은 거 같은데....
얘가 레시피 등록이랑 리뷰 등록이랑 두개가 있으니까 그걸 '-'말고 '/'로 나눠야하나...?
모르겠습니다!!!

지금 리뷰 태그 바텀시트는 높이 어떻게 제한하고 있죠?
일단 dialog에 최대 높이 지정할 수 있도록 바꾸긴 했는데 머지는 안 함...
이거 머지하면 dialog 여는 방식 싹 다 수정해야해서...
일단 급한 거 아니면 PR들 다 머지 들어가고 맨 마지막에 작업하겠음!

@Leejin-Yang
Copy link
Contributor Author

@hae-on @xodms0309

스크린샷 2024-04-14 15 05 29

리뷰 반영 및 추가 작업 했슴다
TopBar 머지되면 타이틀 추가하고, 상품 상세 페이지 머지되면 페이지 이동하는 작업하면 됩니다.

Copy link

🔗 배포된 storybook 주소: https://65f015a4aed45406385006ee-yowmzduuio.chromatic.com/

Copy link
Contributor

@hae-on hae-on left a comment

Choose a reason for hiding this comment

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

굿굿~~
근데 궁금한게 text area 유효성 검사 관련한 거 n자 이상 저것만 하나요??
디자이너분도 어떤거 있는지 몰라서 저 문구만 만드신 거 같은데
예전에 제가 짜둔 로직은 스페이스나 줄바꿈 몇 회 이상이면 안 되게 만들어놨거든요?? (물론 연결은 안 해둠)
이거 관련해서 우리 어디까지 잡을 지, 문구 뭐로 할 지 얘기 해봐야할 거 같은디???

그리고 TopBar는 제가 전체 다 한번에 바꾸려고 이슈 파두긴 했는데
여기서 사용하는 TopBar는 이 PR에서 작업하실건가요???

@Leejin-Yang
Copy link
Contributor Author

@hae-on

이거 관련해서 우리 어디까지 잡을 지, 문구 뭐로 할 지 얘기 해봐야할 거 같은디???

이거는 나중에 더 이야기해보죠.!

그리고 TopBar는 제가 전체 다 한번에 바꾸려고 이슈 파두긴 했는데 여기서 사용하는 TopBar는 이 PR에서 작업하실건가요???

그러면 이거 머지되면 한번에 해주세요 :)

Copy link
Member

@xodms0309 xodms0309 left a comment

Choose a reason for hiding this comment

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

구우우우웃 고생했어요 황펭~~~~

@@ -179,6 +179,15 @@ const router = createBrowserRouter([
return { Component: ProductDetailPage };
},
},
{
path: `${PATH.PRODUCT_LIST}/detail/:productId/review-register`,
Copy link
Member

Choose a reason for hiding this comment

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

굿 좋아요

@Leejin-Yang Leejin-Yang merged commit 0c9edba into feat/v2 Apr 14, 2024
2 of 3 checks passed
@Leejin-Yang Leejin-Yang deleted the feat/issue-76 branch April 14, 2024 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

리뷰 작성 폼 개편
3 participants