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: user profile Avatar 공통 컴포넌트 구현 #15

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Yeonseo-Jo
Copy link

@Yeonseo-Jo Yeonseo-Jo commented Jan 22, 2025

close #13

☑️ 완료 태스크


🔎 PR 내용

UserProfileAvatar component

  • userProfile을 구현할 때 사용할, Avatar UI component를 추상화 한 공통 컴포넌트라는 의미로 <UserProfileAvatar>라고 네이밍 했습니다!
  • prop : radix의 Avatar 컴포넌트 props 중, 공통으로 고정해두고 사용할 속성(radius, varaint, color, fallback) 제외한 prop들
  • [ > Avatr prop 세부 정보 ]

(radix) Avatar 컴포넌트의 세부 동작

  • Avatar 컴포넌트 : span 태그로 감싸진 img태그
    ➡ img 관련 attribute들 (ex. alt 등) 적용 가능
  • loading 시에는 img 태그를 감싸는 span 태그만 보여짐
    ➡ 이 span 태그의 background === color prop에 의존하므로, 로딩 시 color prop으로 지정해준 색상의 태그가 보여지게 됨 (스켈레톤처럼 동작!)
  • img 로드 실패 시, fallback prop으로 넘겨준 element가 보여짐
  • [ > 코드가 궁금하다면 여기를 참고하세용 ]

🙋🏻‍♀️ 논의가 필요한 점

  1. color prop으로 gray가 적절할지?
    : Avatar 컴포넌트 설계 구조상, 스켈레톤 동작처럼 이미지 source가 로드되기 전까지 color에 넘겨준 color 값으로 보여지게 되어요.
    이때 넘길 수 있는 값이 enum으로 제한되어 있어, gray scale 색상으로 사용하기로 한 Slate 색상을 사용할수 없어 gray로 지정했는데 어떤것 같나욤?
    다른 main color를 사용하는것보다 gray 계열을 사용하는 것이 자연스러운것 같아 우선 gray로 지정해놨어요

  2. fallback img로 어떤걸 보여줄지
    : 요건 구두로 짧게 논의해보면 좋을것 같아 회의 안건으로 올려두겠습니둥~


📷 스크린샷

menu 페이지에 테스트 코드를 추가해두었습니당 (fallback인 경우 / 이미지를 성공적으로 받아왔을 경우)

2025-01-22.22.30.46.mov

Copy link
Member

@seobbang seobbang left a comment

Choose a reason for hiding this comment

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

고생하셨습니당 !! 👍✨

color prop으로 gray가 적절할지?

저는 좋아요!! 혹여나 나중에 어색하게 느껴지는 경우가 있으면 그때 바꿔도 좋을 것 같습니당

Comment on lines +3 to +6
type UserProfileAvatarProps = Omit<
AvatarProps,
"radius" | "variant" | "color" | "fallback"
>;
Copy link
Member

Choose a reason for hiding this comment

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

연서언니가 달아준 제 pr 코리 내용에서의 이점과 결론처럼 interface 사용하는건 어떤가용?

Suggested change
type UserProfileAvatarProps = Omit<
AvatarProps,
"radius" | "variant" | "color" | "fallback"
>;
interface UserProfileAvatarProps extends Omit<
AvatarProps,
"radius" | "variant" | "color" | "fallback"
>{};

이것도 강제할 필요까지는 없겠지만 컨벤션 얘기할 때 한 번 얘기하고 가면 좋겠네요 ㅎㅎ

"radius" | "variant" | "color" | "fallback"
>;

const UserProfileAvatar = ({ ...props }: UserProfileAvatarProps) => {
Copy link
Member

@seobbang seobbang Jan 24, 2025

Choose a reason for hiding this comment

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

여긴 다른 prop이 없어서 이렇게 해도 괜찮을 것 같네용
개인 취향이니 참고만 해주셔요!

Suggested change
const UserProfileAvatar = ({ ...props }: UserProfileAvatarProps) => {
const UserProfileAvatar = (props: UserProfileAvatarProps) => {

@jungwoo3490 jungwoo3490 added ✨ Feature 기능 개발 🐹 연또 and removed feat labels Jan 24, 2025
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.

Feat: ProfileImage 공통 컴포넌트를 구현합니다.
3 participants