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: 프로필 내 카드 구현 #1272

Merged
merged 7 commits into from
Jan 25, 2024
Merged

feat: 프로필 내 카드 구현 #1272

merged 7 commits into from
Jan 25, 2024

Conversation

seojisoosoo
Copy link
Member

🤫 쉿, 나한테만 말해줘요. 이슈넘버

🧐 어떤 것을 변경했어요~?

  • 프로필 내에서 프로젝트/모임에 사용되는 콘텐츠 카드를 구현했어요.

🤔 그렇다면, 어떻게 구현했어요~?

  • ContentsCard로 네이밍하고, thumbnail, title, top, bottom을 prop으로 받아왔어요.
  • title은 제목, top은 제목 위에 들어가는 내용, bottom은 제목 아래에 들어가는 내용이에요.

❤️‍🔥 당신이 생각하는 PR포인트, 내겐 매력포인트.

  • MemberProjectCard, MemberGroupCard컴포넌트를 따로 만든 뒤, 지금 만든 ContentsCard를 사용하려고 해요.

📸 스크린샷, 없으면 이것 참,, 섭섭한데요?

스크린샷 2024-01-24 오전 1 01 33

@seojisoosoo seojisoosoo self-assigned this Jan 23, 2024
@github-actions github-actions bot requested a review from juno7803 January 23, 2024 16:02
Copy link

github-actions bot commented Jan 23, 2024

✨✨ 스토리북으로 확인하기 ✨✨

Copy link

github-actions bot commented Jan 23, 2024

🚀 프리뷰 배포 확인하기 🚀

https://bf0b9d3d.sopt-internal-dev.pages.dev

Comment on lines 36 to 41
const Thumbnail = styled.img`
border-radius: 14px;
width: 84px;
height: 84px;
object-fit: cover;
`;
Copy link
Member

Choose a reason for hiding this comment

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

p2; <img> 대신 <ResizedImage> 를 사용해서 용량 최적화를 하면 좋을 것 같아요!

`;

const Title = styled.h1`
max-width: 227px;
Copy link
Member

Choose a reason for hiding this comment

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

ask; 227px는 어떻게 나온 숫자인가요..?

Copy link
Member Author

Choose a reason for hiding this comment

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

피그마에서 확인해보았을 때, 20자 초과되는 지점이 227px이더라구요..! 그래서 227px로 max-width를 주고 초과되면 ellipsis가 되도록 했습니다!

Copy link
Member

Choose a reason for hiding this comment

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

가로 사이즈에 상관 없이 20자만 노출되는게 현재 스펙인건가용?

Copy link
Member Author

Choose a reason for hiding this comment

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

네!! 디스크립션은 20자, 타이틀은 14자 초과 시 ellipsis되는것으로 명기되어있더라구요..!!

스크린샷 2024-01-24 오전 2 17 26

Copy link
Member

Choose a reason for hiding this comment

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

명확히 20자 제한이면 픽셀을 주는 것 보다 js로 처리하는게 안전하지 않을까요..? (컴퓨터 폰트 상황에 따라 달라질 수 있음)
추가로 너비가 아니라 글자수로 스펙이 정해진 이유를 물어보면 좋을 것 같아요! 글자수로 제한한 특별한 이유가 없다면, 너비 기반으로 ellipsis 하는게 좀 더 자연스러우면서 유연한 것 같아서요

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 너비 기반으로 하는 게 좀 더 자연스러울 것 같다고 생각합니다!! 디자인이랑 한 번 소통해볼게요~!!

Copy link
Member Author

@seojisoosoo seojisoosoo Jan 24, 2024

Choose a reason for hiding this comment

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

슬랙으로 소통한 결과, 반응형 너비에 따라 ellipsis 처리하는 것으로 스펙 변경되었습니다!
max-width: calc(100% - 132px);로, 카드 내에서 텍스트부분의 max-width를 구해주었고, 이를 넘어가면 ellipsis처리되도록 했습니다!!

package.json Outdated
@@ -8,7 +8,7 @@
"scripts": {
"analyze": "cross-env ANALYZE=true next build",
"build": "next build",
"build-storybook": "storybook build",
"build-storybook": "NODE_OPTIONS='--max-old-space-size=4096' storybook build",
Copy link
Member Author

Choose a reason for hiding this comment

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

스토리북관련해서

OOM command not allowed when used memory > 'maxmemory'. script: 6c25fa0b8bf82ab072e19d624f145540fe35ec57, on @user_script:172.
    → Failed to publish build
Error: non-zero exit code

이 에러가 나서, 메모리 옵션을 조정해주었는데, 괜찮을까요!

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
Member

Choose a reason for hiding this comment

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

다만 윈도우에서 문제가 발생할 수 있으니 위의 analyze 스크립트처럼 cross-env 를 사용하면 좋을 것 같아요

Copy link
Member Author

Choose a reason for hiding this comment

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

오호 G선생님과 cross-env 학습하고 왔습니당 신기하네요오

Copy link
Collaborator

Choose a reason for hiding this comment

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

다만 윈도우에서 문제가 발생할 수 있으니 위의 analyze 스크립트처럼 cross-env 를 사용하면 좋을 것 같아요

@Tekiter 윈도우에서 어떤 문제가 발생하나요?! "윈도우" 를 가지고 분기도 할 수 있군요..!

Copy link
Collaborator

@juno7803 juno7803 left a comment

Choose a reason for hiding this comment

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

시멘틱 태그들도 아주 맛있게 써주었군요 👍
빠른 작업 감사합니다!

@seojisoosoo seojisoosoo added this pull request to the merge queue Jan 25, 2024
Merged via the queue into main with commit 24cead1 Jan 25, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: 프로필 내 카드 구현
3 participants