-
Notifications
You must be signed in to change notification settings - Fork 10
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
Badgeコンポーネントの追加 #24
Badgeコンポーネントの追加 #24
Conversation
あー、命名は検討余地あるかも。 Badgeって、こっちのイメージもある。 |
TODO: padding変える。 |
Material-UI文脈のBadgeは別コンポーネントで提供すると良いかも。 |
↑あとで作ろうねという話に |
を自由に決められるようにする |
src/components/Badge/styled.ts
Outdated
|
||
export const Container = styled.span<Props>` | ||
display: inline-block; | ||
padding: 0.25em 0.7em; |
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.
themeのspace基準で当てて欲しい!
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.
当てました!(noroさんとdesign確認済み)
export const Container = styled.span<Props>` | ||
display: inline-block; | ||
padding: ${({ theme }) => `${theme.spacing / 2}px ${theme.spacing}px`}; | ||
border-radius: ${({ type }) => type === "pill" ? "10rem" : "4px"}; |
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.
pill型の場合は極端に大きな値を埋めることで、要素の大きさに依存することなく形を表示するようにした。with noroさん
color, | ||
type = "normal", | ||
component = "span", | ||
fontSize = "0.65em", |
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.
htmlに当たってるfont-size依存になるのでpx指定がいいかと!
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.
https://getbootstrap.com/docs/4.4/components/badge/
- bootstrapのように親要素からよしなにサイズを決めたい
- とはいえど細かい調整をしたいときもあるよね
みたいな流れでfontSize
propsが誕生したのですが、em指定に大きなデメリットとかってあったりしますかね...?
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.
なるほど!!そういう意図だったのか!
storybook見たらいい感じになってたので大丈夫そう!
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.
LGTMです!
No description provided.