-
Notifications
You must be signed in to change notification settings - Fork 4.6k
🪟 🎨 Add Text component #15570
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
🪟 🎨 Add Text component #15570
Changes from all commits
caea515
8256b28
9438826
cea10f8
bf2df7e
7307790
bbb58ce
f50472c
69c6210
00d2c00
74f66dd
da8093e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
import classNames from "classnames"; | ||
import React from "react"; | ||
|
||
import headingStyles from "./heading.module.scss"; | ||
import textStyles from "./text.module.scss"; | ||
|
||
type TextSize = "sm" | "md" | "lg"; | ||
type HeadingSize = TextSize | "xl"; | ||
type TextElementType = "p" | "span" | "div"; | ||
type HeadingElementType = "h1" | "h2" | "h3" | "h4" | "h5" | "h6"; | ||
|
||
interface BaseProps { | ||
className?: string; | ||
centered?: boolean; | ||
} | ||
|
||
interface TextProps extends BaseProps { | ||
as?: TextElementType; | ||
size?: TextSize; | ||
bold?: boolean; | ||
} | ||
|
||
interface HeadingProps extends BaseProps { | ||
as: HeadingElementType; | ||
size?: HeadingSize; | ||
} | ||
|
||
const getTextClassNames = ({ size, centered, bold }: Required<Pick<TextProps, "size" | "centered" | "bold">>) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
To confirm my understanding - the purpose of this is basically to remove the If so, would it potentially be better to do something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You'd need to add the new property to the destructure anyway. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In case that the props were to expand, this would continue to work with Pick over using Omit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, good point that the new props would need to be added to the destructure, I forgot about that. This looks good to me then! |
||
const sizes: Record<TextSize, string> = { | ||
sm: textStyles.sm, | ||
md: textStyles.md, | ||
lg: textStyles.lg, | ||
}; | ||
|
||
return classNames(textStyles.text, sizes[size], centered && textStyles.centered, bold && textStyles.bold); | ||
}; | ||
|
||
const getHeadingClassNames = ({ size, centered }: Required<Pick<HeadingProps, "size" | "centered">>) => { | ||
const sizes: Record<HeadingSize, string> = { | ||
sm: headingStyles.sm, | ||
md: headingStyles.md, | ||
lg: headingStyles.lg, | ||
xl: headingStyles.xl, | ||
}; | ||
|
||
return classNames(headingStyles.heading, sizes[size], centered && headingStyles.centered); | ||
}; | ||
|
||
const isHeadingType = (props: TextProps | HeadingProps): props is HeadingProps => { | ||
return props.as ? /^h[0-6]$/.test(props.as) : false; | ||
}; | ||
|
||
export const Text: React.FC<TextProps | HeadingProps> = React.memo((props) => { | ||
const isHeading = isHeadingType(props); | ||
const { as = "p", centered = false, children } = props; | ||
|
||
const className = classNames( | ||
isHeading | ||
? getHeadingClassNames({ centered, size: props.size ?? "md" }) | ||
: getTextClassNames({ centered, size: props.size ?? "md", bold: props.bold ?? false }), | ||
props.className | ||
); | ||
|
||
return React.createElement(as, { | ||
className, | ||
children, | ||
}); | ||
edmundito marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
@use "../../../scss/colors"; | ||
|
||
.heading { | ||
font-weight: 700; | ||
|
||
color: colors.$dark-blue; | ||
margin: 0; | ||
padding: 0; | ||
} | ||
|
||
.sm { | ||
font-size: 16px; | ||
line-height: 1.2em; | ||
} | ||
|
||
.md, | ||
.lg, | ||
.xl { | ||
line-height: 1.3em; | ||
} | ||
|
||
.md { | ||
font-size: 20px; | ||
} | ||
|
||
.lg { | ||
font-size: 24px; | ||
} | ||
|
||
.xl { | ||
font-size: 32px; | ||
} | ||
|
||
.centered { | ||
text-align: center; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import { ComponentStory, ComponentMeta } from "@storybook/react"; | ||
|
||
import { Text } from "./Text"; | ||
|
||
export default { | ||
title: "Ui/Text", | ||
component: Text, | ||
} as ComponentMeta<typeof Text>; | ||
|
||
const Template: ComponentStory<typeof Text> = (args) => <Text {...args} />; | ||
|
||
export const Primary = Template.bind({}); | ||
Primary.args = { | ||
size: "md", | ||
children: | ||
"Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.", | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { Text } from "./Text"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
@use "../../../scss/colors"; | ||
|
||
.text { | ||
color: colors.$dark-blue; | ||
margin: 0; | ||
padding: 0; | ||
} | ||
|
||
.sm, | ||
.md { | ||
font-weight: 400; | ||
line-height: 1.2em; | ||
} | ||
|
||
.sm { | ||
font-size: 12px; | ||
} | ||
|
||
.md { | ||
font-size: 13px; | ||
} | ||
|
||
.lg { | ||
font-size: 14px; | ||
font-weight: 500; | ||
line-height: 1.3em; | ||
} | ||
|
||
.centered { | ||
text-align: center; | ||
} | ||
|
||
.bold, | ||
.text > strong { | ||
font-weight: 600; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,11 @@ | ||
@use "../../../../../scss/colors"; | ||
@use "../../../../../scss/variables"; | ||
|
||
.title { | ||
font-weight: 700; | ||
font-size: 32px; | ||
line-height: 39px; | ||
width: 250px; | ||
color: colors.$dark-blue-900; | ||
margin-bottom: variables.$spacing-md; | ||
} | ||
|
||
.highlight { | ||
color: colors.$blue-400; | ||
color: colors.$blue; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,5 @@ | ||
@use "../../../../../../scss/colors"; | ||
|
||
.title { | ||
font-weight: 700; | ||
font-size: 32px; | ||
line-height: 39px; | ||
color: colors.$blue-400; | ||
color: colors.$blue; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,11 @@ | ||
import React from "react"; | ||
|
||
import { Text } from "components/base/Text"; | ||
|
||
import styles from "./FormTitle.module.scss"; | ||
|
||
export const FormTitle: React.FC = ({ children }) => <h1 className={styles.title}>{children}</h1>; | ||
export const FormTitle: React.FC = ({ children }) => ( | ||
<Text as="h1" size="xl" className={styles.title}> | ||
{children} | ||
</Text> | ||
); |
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.
I was inspired by styled components to call this
as
but is the name clear? Alternatively, I thought ofelement
ortype
as well. Thoughts?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.
as
feels fairly standard, though some other opinions would definitely be useful :)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.
I like
as
hereThere 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.
++ for
as
as well, as it feels pretty standard as of today.