Skip to content

refactor(badge): refactor badge component to use styled-components #535

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

Merged
alejandronanez merged 6 commits into from
Oct 22, 2017
Merged

refactor(badge): refactor badge component to use styled-components #535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8f04e76
refactor($component): refactor badge component to use styled-components
hirejohnsalcedo Oct 21, 2017
e0d9ab2
refactor($component): refactor badge component to use styled-components
hirejohnsalcedo Oct 21, 2017
674b184
refactor(component): use more concise syntax in BadgeText
hirejohnsalcedo Oct 21, 2017
8a83bd4
Merge branch 'master' into refactor/532-badge-component
hirejohnsalcedo Oct 21, 2017
16cf3ac
fix(component): import styled from native
hirejohnsalcedo Oct 21, 2017
e9dfb85
Merge branch 'refactor/532-badge-component' of https://github.com/jpl…
hirejohnsalcedo Oct 21, 2017
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
8 changes: 8 additions & 0 deletions .all-contributorsrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,14 @@
"contributions": []
},
{
"login": "jpls93",
"name": "John Patrick Salcedo",
"avatar_url": "https://avatars2.githubusercontent.com/u/17956698?v=4",
"profile": "https://github.com/jpls93",
"contributions": [
"code"
]
},
"login": "ocarreterom",
"name": "Óscar Carretero",
"avatar_url": "https://avatars2.githubusercontent.com/u/11599942?v=4",
Expand Down
56 changes: 23 additions & 33 deletions src/components/badge.component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { StyleSheet, View, Text } from 'react-native';
import styled from 'styled-components/native';

import { colors, fonts, normalize } from 'config';

Expand All @@ -10,39 +10,29 @@ type Props = {
largeText: boolean,
};

const styles = StyleSheet.create({
badge: {
flex: 1,
alignItems: 'center',
justifyContent: 'center',
borderRadius: 18,
width: 18,
height: 18,
borderColor: colors.alabaster,
borderWidth: 1,
},
badgeText: {
...fonts.fontPrimaryBold,
backgroundColor: 'transparent',
},
textSmall: {
fontSize: normalize(7),
},
textLarge: {
fontSize: normalize(9.5),
},
});
const BadgeContainer = styled.View`
flex: 1;
align-items: 'center';
justify-content: 'center';
border-radius: 18;
width: 18;
height: 18;
border-color: ${colors.alabaster};
border-width: 1;
${({ backgroundColor }) => `background-color: ${backgroundColor};`};
Copy link
Copy Markdown
Member

@alejandronanez alejandronanez Oct 21, 2017

Choose a reason for hiding this comment

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

I think you can do background-color: ${backgroundColor} instead,

`;

const BadgeText = styled.Text`
${{ ...fonts.fontPrimaryBold }};
background-color: 'transparent';
${({ largeText }) =>
Copy link
Copy Markdown
Member

@alejandronanez alejandronanez Oct 21, 2017

Choose a reason for hiding this comment

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

Same goes for this font-size: ${largeText ? normalize(9.5) : normalize(7)}

Copy link
Copy Markdown
Contributor Author

@hirejohnsalcedo hirejohnsalcedo Oct 21, 2017

Choose a reason for hiding this comment

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

Hi @alejandronanez , might not work since unlike border-color: ${colors.alabaster};, colors is a file-wide variable whereas backgroundColor and largeText are props passed down to the styled components. https://www.styled-components.com/docs/basics#adapting-based-on-props

Copy link
Copy Markdown
Contributor Author

@hirejohnsalcedo hirejohnsalcedo Oct 21, 2017

Choose a reason for hiding this comment

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

@alejandronanez , let me know if I can help you with anything else

Copy link
Copy Markdown
Member

@alejandronanez alejandronanez Oct 22, 2017

Choose a reason for hiding this comment

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

Oh yeah, you're right. I missed that, sorry.
Thanks!

Copy link
Copy Markdown
Contributor

@MrLoh MrLoh Oct 22, 2017

Choose a reason for hiding this comment

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

the usual way would be this though:

...
    font-size: ${({largeText}) => largeText ? normalize(9.5) : normalize(7)};

Copy link
Copy Markdown
Contributor Author

@hirejohnsalcedo hirejohnsalcedo Oct 22, 2017

Choose a reason for hiding this comment

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

I agree that would be more concise. I guess I just made the code style more consistent by making it parallel to how I wrote BadgeContainer's backgroundColor which doesn't have a default value. Since writing something without a default value that way would make it background-color: undefined which, without tree-shaking, would yield dead code.

`font-size: ${largeText ? normalize(9.5) : normalize(7)};`};
`;

export const Badge = ({ color, backgroundColor, text, largeText }: Props) => (
<View style={StyleSheet.flatten([styles.badge, { backgroundColor }])}>
<Text
style={[
styles.badgeText,
largeText ? styles.textLarge : styles.textSmall,
{ color },
]}
>
<BadgeContainer backgroundColor={backgroundColor}>
<BadgeText largeText={largeText} color={color}>
{text}
</Text>
</View>
</BadgeText>
</BadgeContainer>
);