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

[Wip] 공통 컴포넌트 제작(4-1) UserContentBlock #23

Merged
merged 20 commits into from
Jan 2, 2024

Conversation

loevray
Copy link
Collaborator

@loevray loevray commented Jan 2, 2024

작업 사항

  • 프로필 이미지 + 닉네임 + 내용 + 서브컨텐츠(날짜 수정,삭제 등)을 추상화한 UserContentBlock을 제작하였습니다
  • Footer컴포넌트의 props를 rest로가져와서 Footer컴포넌트 최상단 컴포넌트인 Flex에 적용하였습니다 =>외부에서 스타일 주기 편하게
  • storybook과 react-router-dom의 호환을 위하여 addon도 추가하였습니다

관련 이슈

#7 의 4-1번 태스크입니다

PR Point

  • props가 뭔가 많은거같기도 하고 아닌것 같기도 합니다~ 너무 많다고 생각되시면 말씀해주세요
  • href를 props로 받아와서 navigate해줄수 있습니다
  • onSubContentClick을 props로 받아와서 삭제등에 대응할수 있게 하였습니다
  • onNavigate를 내부에서 구현하는게 맞는지 의문이듭니다. props로 밖에서 onClick으로 전달할수 있어서...고민이 됩니다

참고사항

green, z-index도 테마에 추가하였습니다.

- UserContentBlock은 프로필사진, 닉네임, 내용으로 구성되어있습니다.
- 메시지, 알람, 댓글리스트에서 사용 가능할 것으로 예상됨
- storybook-addon-react-router-v6패키지입니다
- 7일전 같이 날짜 나타낼때 쓰면 좋아보여서 넣었습니다
- 외부에서 차크라ui처럼 주입 가능
@loevray loevray added ✨feature 기능 개발 💄style UI/스타일 수정 👀 리뷰 필요 labels Jan 2, 2024
@loevray loevray self-assigned this Jan 2, 2024
Copy link
Collaborator

@JaeHyunGround JaeHyunGround left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 !! :)

width = DEFAULT_WIDTH,
height = DEFAULT_HEADER_HEIGHT,
}: FooterProps) => {
const Footer = ({ ...props }: FlexProps) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

FlexProps 로 받아주면 ...props 도 대처가 되는군요 !! 유익한 기능을 알아갑니다 :)

@@ -64,4 +67,7 @@ export const theme = extendTheme({
sizes: {
icon: '2.4rem',
},
zIndices: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 z-index를 의미하는 것이겠죠 ??!?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

네 맞습니다 chakra ui 링크에서 theme-key로 되어있는 부분이 테마변수명입니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

아하 확인했습니다 !!

Comment on lines +54 to +57
userImageSize && typeof userImageSize === 'string'
? userImageSize
: `${userImageSize}px`
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 부분이 하나의 함수로 처리되어도 좋을 것 같습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

좋습니다 numberToPixel 이런 함수로 만들어서 사용해보겠습니다 ㅎㅎ

Comment on lines 60 to 68
{isOnline ? (
<AvatarBadge
boxSize="14px"
border="2px solid white"
bg="green.100"
right="5%"
bottom="5%"
/>
) : null}
Copy link
Collaborator

Choose a reason for hiding this comment

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

isOnline && 이런 식으로 처리해도 좋을 것 같습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵 수정사항 반영하겠습니다

right="0"
zIndex="normal"
cursor={onSubContentClick && 'pointer'}
onClick={() => onSubContentClick && onSubContentClick()}
Copy link
Collaborator

Choose a reason for hiding this comment

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

지금 최상위 컴포넌트에도 click 이벤트가 있고, Text에도 이벤트가 있는데 관련해서 문제는 없는지 궁금합니다.
해당 컴포넌트 클릭 시 url 이동 말고 필요한 다른 이벤트가 뭔가 생각이 잘 안나네요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

기획중 둘을 동시에 사용하는 경우가 없었습니다. 아직 정해지지 않은 부분이기도 하구요

  1. 알림,메시지는 최상위 컴포넌트 클릭시 페이지 이동
  2. 댓글부분에서는 subcontent클릭시 삭제기능. => 이때는 아직 정해지지 않았지만, 아마 사용하진 않을 것 같습니다

Comment on lines 12 to 23
interface UserContentBlockProps extends FlexProps {
userImage?: string;
userImageSize?: string | number;
isOnline?: boolean;
username: string;
content: string;
subContent?: string;
usernameFontSize?: string | number;
contentFontSize?: string | number;
href?: string;
onSubContentClick?: () => void;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

정말 별건 아니지만... username , content 관련 속성들이 정렬되면 좋을 것 같습니다

{username,
usernameFontSize,
content,
contentFontsize
}

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

@whdgur5717 whdgur5717 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

  • props는 다 필요한거라 괜찮은 것 같습니다
  • 저도 뭔가 컴포넌트의 역할 상 이동까지 담당하는 게 좀 과도하지 않나 싶긴 합니다.. 나중에 이동이 아니라 다른 걸 할 수도 있기 때문에 props로 전달한 일만 하는 게 좀 좋아보이긴 합니다

@loevray loevray merged commit e410c29 into dev Jan 2, 2024
@loevray loevray deleted the feature/commonComponentUserContentBlock branch January 2, 2024 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👀 리뷰 필요 ✨feature 기능 개발 💄style UI/스타일 수정
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants