Feat(Text) Create universal Text component #54
Conversation
catalog/pages/typography/index.md
Outdated
| - Prop: children | ||
| Type: node | ||
| Default: | ||
| Notes: Render as the Button's children |
There was a problem hiding this comment.
I think we should update this.
catalog/pages/typography/index.md
Outdated
| @@ -34,65 +81,233 @@ const str = "Better is together"; | |||
| <Container> | |||
| <Row style={fontSizeRowStyle}> | |||
| <Column medium={2} >12 px</Column> | |||
| <Column medium={6} style={{fontSize: "12px"}}>{str}</Column> | |||
| <Column medium={6} style={{fontSize: "12px"}}> | |||
There was a problem hiding this comment.
we probably shouldn't force a font size since we are showcasing that <Text size=.. renders the requested size. Some for all others below.
There was a problem hiding this comment.
Good point, I will convert these styles over to the size prop for all applicable specimens.
src/components/Text/Base.js
Outdated
| "text--primary": !!primary || (!secondary && !disabled), | ||
| "text--secondary": !!secondary, | ||
| "text--disabled": !!disabled | ||
| }); |
There was a problem hiding this comment.
let's also accept a className prop in case there is a need for additional classes.
| TextBase.propTypes = { | ||
| tag: PropTypes.oneOf(["div", "span", "p"]), | ||
| variant: PropTypes.oneOf(["accent", "dark", "light"]), | ||
| accent: PropTypes.oneOf([ |
There was a problem hiding this comment.
maybe we can use Object.keys(theme.colors)
There was a problem hiding this comment.
I combed through the FXO style guide and ensured I only included colors that were designated as approved for text. There are a bunch of background colors in there I'd like to omit.
| "summerSky", | ||
| "turquoise" | ||
| ]), | ||
| size: PropTypes.oneOf(["uno", "hecto", "kilo", "giga", "tera"]), |
There was a problem hiding this comment.
here we can also use Object.keys(theme.typography.size)
There was a problem hiding this comment.
I combed through the FXO style guide and ensured I only included font-sizes that were designated as approved for text. There are a couple header font-sizes in there I'd like to omit.
| secondary: PropTypes.bool, | ||
| disabled: PropTypes.bool, | ||
| children: PropTypes.node.isRequired | ||
| }; |
There was a problem hiding this comment.
I think we also need to be able to determine font weights from Object.keys(theme.typography.weight)
There was a problem hiding this comment.
Fixed this up by adding regular and semiBold weights as valid props
Codecov Report
@@ Coverage Diff @@
## master #54 +/- ##
==========================================
+ Coverage 99.63% 99.65% +0.01%
==========================================
Files 86 89 +3
Lines 552 574 +22
Branches 89 95 +6
==========================================
+ Hits 550 572 +22
Misses 2 2
Continue to review full report at Codecov.
|
|
🎉 This PR is included in version 2.10.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
What: Create a universal text component, implementing the FXO design system requirements.
Why: In order to avoid creating many one-off text components in the future, which may or may not align with the FXO design system requirements.
How: Create a universal text component, implementing the FXO design system requirements.
Checklist:
Happy to discuss modifications to the API that would make this component more performant/easier to use!