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/#60] 게시물 삭제 모달 구현 #64

Merged
merged 9 commits into from
Jan 16, 2025
Merged

Conversation

minjeoong
Copy link
Collaborator

@minjeoong minjeoong commented Jan 15, 2025

🔥 Related Issues

✅ 작업 리스트

  • 게시물 삭제 모달 구현
  • 추후 스프린트를 위한 수정하기 추가에 대한 구현

🔧 작업 내용

게시물 삭제 모달 구현했습니다.
해당 모달은 게시물을 삭제하거나, 댓글을 삭제하는 용도로 쓰일 예정입니다.
interaction 정의된 게 없어서, 디자이너께 button 클릭 시 디자인을 요청 드렸습니다.
만들어주신 해당 디자인에 대해 :active css 주었습니다.
... 버튼의 위치가 변경되어도 그 것에 맞게 따라다니도록 구현하였습니다! (영상 확인)

스크린샷 2025-01-16 06 42 25

📣 리뷰어에게 어떠신가요?

[ 해당 컴포넌트가 쓰이는 곳, 수정해야하는 곳 ]

  1. 댓글 컴포넌트
  2. 대댓글 컴포넌트
  3. 헤더 컴포넌트

에 해당 컴포넌트로 변경 필요할 수 있을 것 같습니다.
그리고 유저가 쓴 게시물인 경우에만 해당 버튼이 보여야 하기 때문에,
헤더에서도 props 수정이 불가피해보입니다.
댓글 컴포넌트도 마찬가지로 불가피해보입니다.
-> 추후 디테일 페이지 구현하면서 해당 내용 추가로 수정해두겠습니다!

[ 컴포넌트 @params ]
스크린샷 2025-01-16 06 49 06

다음과 같이 아이콘 사이즈 변경 가능합니다.
현재는 2.4rem, 2.0rem 두 가지 설정해두었습니다.

export const moreIcon = recipe({
  base: {
    width: "2.4rem",
    height: "2.4rem",
  },
  variants: {
    iconSize: {
      "24": {
        width: "2.4rem",
        height: "2.4rem",
      },
      "20": {
        width: "2.0rem",
        height: "2.0rem",
      },
    },
  },
});

[ 해당 컴포넌트 사용 방법 ]
스크린샷 2025-01-16 06 52 02

이렇게 useMoreModal 훅을 만들었습니다.
위치는 @shared/hook/useMoreModal 입니다.
위치나 다양한 피드백 환영합니다.

[수정하기 기능 ]
추후 스프린트를 위해 수정하기 기능을 추가 했습니다.
onEdit 함수가 들어가면, 수정하기가 있다고 인식하도록 구현했습니다.
근데 코드를 작성하면 작성할 수록 이게 맞나 싶은 생각이 들었어요..
다들 한번씩 봐주시고 말씀 주시면 감사할 것 같습니다.

📸 스크린샷 / GIF / Link

2025-01-16.06.44.27.mov

@minjeoong minjeoong added ✨ feat 기능 구현 💄 style 기능에 영향을 주지 않는 커밋, 코드 순서, css등의 포맷에 관한 커밋 labels Jan 15, 2025
Copy link
Collaborator

@Leeyoonji23 Leeyoonji23 left a comment

Choose a reason for hiding this comment

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

일단 어푸 먼저 하겠씁니다! 코드는 나중에 찬찬히 보고 코리 달겠습니다🥲 구현하느라 고생 많으셨어요🥹🥹

Copy link
Collaborator

@ocahs9 ocahs9 left a comment

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 22
iconSize: {
"24": {
width: "2.4rem",
height: "2.4rem",
},
"20": {
width: "2.0rem",
height: "2.0rem",
},
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

p2) 이렇게 하면 iconSize가 2가지의 경우로밖에 분리가 안되고, 매번 다른 사이즈의 아이콘이 필요하다면
그때마다 스타일링 variant를 작성해주어야한다는 불편함이 있어요.

export const iconSizeVar = createVar();

export const container = style({
  margin: "20rem",
  position: "relative",
  width: iconSizeVar, // 동적 크기 설정
  height: iconSizeVar, // 동적 크기 설정
});

이런 형식으로 만들어두고, 실제로 사용할 떄는

 <div
      className={styles.container}
      style={{
        [styles.iconSizeVar]: `${size}rem`, // CSS 변수에 동적 값 설정
      }}
    >
      Icon
    </div>

과 같은 방식으로 개선해보는건 어떨까요?

Copy link
Collaborator Author

@minjeoong minjeoong Jan 16, 2025

Choose a reason for hiding this comment

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

피드백 감사합니다!
해당 방법은 다양한 사이즈가 사용 될 때 유용한 것 같아요!

해당 아이콘은 두가지 사이즈(20,24) 만 쓰이고 있기도 하고,
20을 받으면 2 로 변환하는 것
24를 받으면 2.4 로 변환하는 것

해당 두가지 이슈가 있어서
props로 2 or 2.4를 받는 게 맞을까 생각해봤을 때,, 불편할 것 같다는 생각이 들었어요.

또한 해당 코드 기반으로 방금 변경 사항 적용을 해보았는데, CSS 변수(createVar)와 React 스타일 속성 사용의 혼용으로 스타일이 안 먹는 것 같습니다...

어떻게 하면 좋을지 얘기해보면 좋을 것 같아요

Copy link
Collaborator

Choose a reason for hiding this comment

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

assginInLineVars 함수를 사용해서 할당을 해야해요!
구두로 설명드리겠습니다 ~

Comment on lines 31 to 41
variants: {
iconSize: {
"24": {
width: "2.4rem",
height: "2.4rem",
},
"20": {
width: "2.0rem",
height: "2.0rem",
},
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

p2) 이것도! 위에서 언급했으니 자세한 설명은 생략할게요~


export const container = recipe({
base: {
margin: "20rem",
Copy link
Collaborator

Choose a reason for hiding this comment

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

p4) 혹시 이 마진은 왜 존재하는건가요? 20rem 이면 꽤 큰 거 같은데,
MoreModal 을 사용하려면 항상 상,하,좌,우 모달 주변에 마진이 필요했던 이유가 있나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

마진은 없는 게 맞습니다.
테스트 하려다 모달이 안 보여서 움직여둔 겁니다!
삭제 후 푸시 했습니다

Copy link
Collaborator

@yarimu yarimu left a comment

Choose a reason for hiding this comment

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

간단하지만 효율적인 hook 잘 확인했습니다!!0! 저도 이렇게 만들어 봐야 겠어요!!

위에서 준혁님이 언급해주신 사이즈 개선 방향 너무 좋은데요?? 이번에 만든 공컴에서 사이즈 이슈를 몇 번 겪어서 더 유용하네 느껴지네욥!!

@minjeoong minjeoong merged commit 604c429 into develop Jan 16, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feat 기능 구현 💄 style 기능에 영향을 주지 않는 커밋, 코드 순서, css등의 포맷에 관한 커밋
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ Feat ] 삭제하기 버튼 컴포넌트
4 participants