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.

Sign up for GitHub

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

Refactoring text component #769

Closed
wants to merge 7 commits into from
Closed

Refactoring text component #769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b11307a
changing Text styled-cmponent declaration format
caiokf Nov 14, 2019
201b00d
Merge branch 'master' into storybook-components
BenGamma Nov 15, 2019
72a447c
Merge branch 'storybook-components' of https://github.com/ONEARMY/com…
BenGamma Nov 17, 2019
a5297aa
Merge branch 'storybook-components' into refactoring-text-component
BenGamma Nov 17, 2019
9d15560
changing Text styled-cmponent declaration format
caiokf Nov 14, 2019
4e6aab5
Merge branch 'refactoring-text-component' of https://github.com/ONEAR…
BenGamma Nov 20, 2019
d4d2e3e
add custom props to Text - remove uppercase import
BenGamma Nov 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
193 changes: 105 additions & 88 deletions src/components/Text/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ export interface ITextProps {
medium?: boolean
small?: boolean
superSmall?: boolean

// keyof colors returns full object prototype, include typeof for just named keys (i.e. color list)
color?: keyof typeof theme.colors

// clip forces text to fill max 1 line and add '...' for overflow
clipped?: boolean
preLine?: boolean
Expand All @@ -29,94 +31,109 @@ export interface ITextProps {
paragraph?: boolean
}

export const uppercase = props =>
props.uppercase
? {
textTransform: 'uppercase',
}
: null

export const capitalize = props =>
props.capitalize
? {
textTransform: 'capitalize',
}
: null

export const inline = (props: ITextProps) =>
props.inline ? { display: 'inline-block' } : null

export const txtcenter = (props: ITextProps) =>
props.txtcenter ? { textAlign: 'center' } : null

export const txtRight = (props: ITextProps) =>
props.txtRight ? { textAlign: 'right' } : null

export const regular = (props: ITextProps) =>
props.regular ? { fontWeight: 400 } : null

export const bold = (props: ITextProps) =>
props.bold ? { fontWeight: 600 } : null

export const large = (props: ITextProps) =>
props.large ? { fontSize: theme.fontSizes[3] } : null

export const tags = (props: ITextProps) =>
props.tags ? { fontSize: '12px', color: theme.colors.blue } : null

// TODO : change auxiliary & paragaph prop to theme variant
export const auxiliary = (props: ITextProps) =>
props.auxiliary
? {
fontFamily: '"Inter", Helvetica Neue, Arial, sans-serif;',
fontSize: '12px',
color: '#686868',
}
: null

export const paragraph = (props: ITextProps) =>
props.paragraph
? {
fontFamily: '"Inter", Helvetica Neue, Arial, sans-serif;',
fontSize: '16px',
color: theme.colors.grey,
}
: null

export const medium = (props: ITextProps) =>
props.medium ? { fontSize: theme.fontSizes[2] } : null

export const small = (props: ITextProps) =>
props.small ? { fontSize: theme.fontSizes[1] } : null

export const superSmall = (props: ITextProps) =>
props.superSmall ? { fontSize: theme.fontSizes[0] } : null

export const clipped = (props: ITextProps) =>
props.clipped
? { whiteSpace: 'nowrap', textOverflow: 'ellipsis', overflow: 'hidden' }
: null

export const preLine = (props: ITextProps) =>
props.preLine ? { whiteSpace: 'pre-line' } : null

export const BaseText = styled(RebassText)`
${inline}
${uppercase}
${capitalize}
${regular}
${bold}
${txtcenter}
${large}
${medium}
${small}
${superSmall}
${clipped}
${preLine}
${tags}
${auxiliary}
${paragraph}
${txtRight}
export const BaseText = styled(RebassText)<ITextProps>`
${props =>
props.inline &&
`
display: 'inline-block';
`}

${props =>
props.txtcenter &&
`
textAlign: 'center';
`}

${props =>
props.txtRight &&
`
textAlign: 'right';
`}

${props =>
props.superSmall &&
`
font-size: ${theme.fontSizes[0]};
`}

${props =>
props.small &&
`
font-size: ${theme.fontSizes[1]};
`}

${props =>
props.medium &&
`
font-size: ${theme.fontSizes[2]};
`}

${props =>
props.large &&
`
font-size: ${theme.fontSizes[3]};
`}

${props =>
props.regular &&
`
font-weight: 400;
`}

${props =>
props.bold &&
`
font-weight: 600;
`}

${props =>
props.uppercase &&
`
text-transform: uppercase;
`}

${props =>
props.capitalize &&
`
text-transform: capitalize;
`}

${props =>
props.tags &&
`
font-size: 12px;
color: ${theme.colors.blue};
`}

${props =>
props.auxiliary &&
`
font-family: "Inter", Helvetica Neue, Arial, sans-serif;
font-size: 12px;
color: #686868;
`}

${props =>
props.paragraph &&
`
font-family: "Inter", Helvetica Neue, Arial, sans-serif;
font-size: 16px;
color: ${theme.colors.grey};
`}

${props =>
props.clipped &&
`
white-space: nowrap;
text-overflow: ellipsis;
overflow: hidden;
`}

${props =>
props.preLine &&
`
white-space: pre-line;
`}
Comment on lines +35 to +136
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep that's more clear thanks. I was wondering though if you had an idea to reduce the number of possible props to the text component ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BenGamma

What we have at the moment:

inline?: boolean
regular?: boolean
txtcenter?: boolean
capitalize?: boolean
bold?: boolean
txtRight?: boolean
large?: boolean
medium?: boolean
small?: boolean
superSmall?: boolean
color?: keyof typeof theme.colors
clipped?: boolean
preLine?: boolean
tags?: boolean
auxiliary?: boolean
paragraph?: boolean

What I think we can reduce it to:

color?: keyof typeof theme.colors
large?: boolean
medium?: boolean
small?: boolean
superSmall?: boolean
align?: string
bold?: boolean
uppercase?: boolean
capitalize?: boolean
clipped?: boolean

Plus, I would have the following separate components for the predefined styles:

ParagraphText
AuxiliaryText
TagsText

Those can simply be export default styled(Text)`color: blah; font-size: bleh;` exported as a different component.

Then we have left inline?: boolean which maybe every Text could be by default, and if needed to be a block element, just wrap it in a div.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes props make sense 👍
I like the separate component for ParaphText, AuxiliaryText, TagsText.

How I would see it is in the src/components/Text/index.ts having the Text component exported by default and then have exported const ParaphText, AuxiliaryText, TagsText.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i will go ahead with that

`

type TextProps = ITextProps & RebassTextProps
Expand Down
1 change: 0 additions & 1 deletion src/pages/Howto/Content/Howto/HowtoSummary/HowtoSummary.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { Container } from './elements'
import { IHowtoStep } from 'src/models/howto.models'
import { ListItem, ListItemText } from '@material-ui/core'
import Text from 'src/components/Text'
import { uppercase } from '../../../../../components/Text/index'

interface IProps {
steps: IHowtoStep[]
Expand Down