-
Notifications
You must be signed in to change notification settings - Fork 1
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
design: Chips 및 상단 배너 컴포넌트 UI 구현 #120
Conversation
|
||
export const bannerSummary = css(typo.Body.Medium_1, { | ||
gap: '5px', | ||
color: color.Neutral[400], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
숫자 프로퍼티를 대괄호 표기법으로 접근할 때는 color.Neutral[400]
처럼 숫자로만 작성하는 것으로 통일하는게 어떨지 의견 여쭙고 싶습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
헉 전에 올려주신 부분인데 제가 실수했네요.. 꼼꼼히 체크했어야 했는데🥲
말씀해 주신 대로 숫자로 통일하여 다시 커밋 올리겠습니다 !
large: css({ | ||
padding: '12px 16px', | ||
borderRadius: '15px', | ||
fontSize: typo.Body.SemiBold_1.fontSize, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
large에는 폰트 관련해서 size만 지정하신 이유가 있을까요? weight, line-height 누락된 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
피그마에 올라온 UI와 비교했을 때 폰트가 너무 두껍다고 생각되어 fontSize만 지정했었습니다.
지금 보니 코드의 통일성이 떨어진다고 생각되긴 하네요😥
다음에 상의 후 수정할 필요가 있다고 생각되면 수정하겠습니다!
import { loadingStyle } from '@/styles/keyframes'; | ||
|
||
export const loadingStyling = css` | ||
animation: ${loadingStyle} 2s infinite; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
로딩 애니메이션 추가 좋습니다 👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chip, 상단 배너 컴포넌트 구현하시느라 고생 많으셨습니다! 로딩 상태까지 구현해 놓으시다니!👍👍넘 좋습니당
아! 그리고 PR명 보면서 든 생각입니다! 저는 기능 구현은 통상적으로 feat으로 짓는게 좋다고 생각했는데, 디자인 시스템 개발할 때는 아름님 PR 명처럼 design으로 짓는 것도 좋아보이네요!! 그럼 디자인 시스템 구축할 때는 design으로 명명할까요??
@@ -4,7 +4,7 @@ import type { Meta, StoryObj } from '@storybook/react'; | |||
import { storyContainer, storyInnerContainer } from '@/stories/story.styles'; | |||
|
|||
const meta = { | |||
title: 'Button', | |||
title: 'commons/Button', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
옮겨주셔서 감사합니다!😀
💡 작업 내용
💡 자세한 설명
✅ 상단 배너 컴포넌트 UI
width: 100%
으로 설정해두었습니다.backgroundColor: color.Neutral['800']
로 지정해두었습니다.keyframes.ts
파일에loadingStyle
을 추가하였습니다. 추후에 자유롭게 수정, 삭제하셔도 됩니다!TopBanner.stories.tsx
파일에exampleBanner
을 임시로 적어두었습니다.+) 기존의 Button 컴포넌트의 스토리북 경로를 commons 폴더로 이동시켜두었습니다!
📗 참고 자료 (선택)
📢 리뷰 요구 사항 (선택)
🚩 후속 작업 (선택)
✅ 셀프 체크리스트
closes #119