Skip to content

refactor(diff-blocks): convert diff-blocks to styled component #533

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

Closed
machour wants to merge 3 commits into from
Closed

refactor(diff-blocks): convert diff-blocks to styled component #533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a2616ca
chore(*): convert diff-blocks.component to styled component
machour Oct 21, 2017
bc85056
fix: use StyleName.withComponent() to inherit, extend Num
machour Oct 21, 2017
9545fbe
Merge remote-tracking branch 'upstream/master' into diff-blocks-styled
machour Oct 26, 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
133 changes: 72 additions & 61 deletions src/components/diff-blocks.component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// @flow

import React from 'react';
import { View, Text, StyleSheet, TouchableOpacity } from 'react-native';

import { colors, fonts } from 'config';
import { TouchableOpacity } from 'react-native';
import styled from 'styled-components/native';
import { colors, fontsStyled } from 'config';

type Props = {
additions: number,
Expand All @@ -12,42 +12,51 @@ type Props = {
onPress: Function,
};

const styles = StyleSheet.create({
container: {
flexDirection: 'row',
alignItems: 'center',
},
linesChanged: {
flexDirection: 'row',
alignItems: 'center',
},
numAdditions: {
marginRight: 3,
...fonts.fontPrimarySemiBold,
color: colors.green,
letterSpacing: 1,
},
numDeletions: {
marginRight: 2,
...fonts.fontPrimarySemiBold,
color: colors.red,
letterSpacing: 1,
},
block: {
width: 7,
height: 7,
marginLeft: 1,
},
greenBlock: {
backgroundColor: colors.green,
},
redBlock: {
backgroundColor: colors.darkRed,
},
greyBlock: {
backgroundColor: colors.greyMid,
},
});
const DiffBlocksViewContainer = styled.View`
flex-direction: row;
align-items: center;
`;
const DiffBlocksTouchableContainer = DiffBlocksViewContainer.withComponent(
TouchableOpacity
);

const Num = styled.Text`
letter-spacing: 1;
${fontsStyled.fontPrimarySemiBold};
`;

const NumAdditions = Num.extend`
margin-right: 3;
color: ${colors.green};
`;

const NumDeletions = Num.extend`
margin-right: 2;
color: ${colors.red};
`;

const LinesChanged = styled.View`
flex-direction: row;
align-items: center;
`;

const Block = styled.View`
width: 7;
height: 7;
margin-left: 1;
`;

const AddedBlock = Block.extend`
background-color: ${colors.green};
`;

const DeletedBlock = Block.extend`
background-color: ${colors.darkRed};
`;

const NeutralBlock = Block.extend`
background-color: ${colors.greyMid};
`;

export const DiffBlocks = ({
additions,
Expand All @@ -57,42 +66,44 @@ export const DiffBlocks = ({
}: Props) => {
const linesChanged = additions + deletions;

let greenBlocks = null;
let redBlocks = null;
let greyBlocks = null;
let addedBlocks = null;
let deletedBlocks = null;
let neutralBlocks = null;

if (linesChanged <= 5) {
greenBlocks = additions;
redBlocks = deletions;
greyBlocks = 5 - (greenBlocks + redBlocks);
addedBlocks = additions;
deletedBlocks = deletions;
neutralBlocks = 5 - (addedBlocks + deletedBlocks);
} else {
greenBlocks = Math.floor(additions / linesChanged * 5);
redBlocks = Math.floor(deletions / linesChanged * 5);
greyBlocks = 5 - (greenBlocks + redBlocks);
addedBlocks = Math.floor(additions / linesChanged * 5);
deletedBlocks = Math.floor(deletions / linesChanged * 5);
neutralBlocks = 5 - (addedBlocks + deletedBlocks);
}

const Component = onPress ? TouchableOpacity : View;
const Container = onPress
? DiffBlocksTouchableContainer
: DiffBlocksViewContainer;

return (
<Component style={styles.container} onPress={onPress}>
<Container onPress={onPress}>
{showNumbers && (
<View style={styles.linesChanged}>
<Text style={styles.numAdditions}>{`+${additions}`}</Text>
<Text style={styles.numDeletions}>{`-${deletions}`}</Text>
</View>
<LinesChanged>
<NumAdditions>{`+${additions}`}</NumAdditions>
<NumDeletions>{`-${deletions}`}</NumDeletions>
</LinesChanged>
)}

{[...Array(greenBlocks)].map((item, index) => {
return <View key={index} style={[styles.block, styles.greenBlock]} />; // eslint-disable-line react/no-array-index-key
{[...Array(addedBlocks)].map((item, index) => {
return <AddedBlock key={index} />; // eslint-disable-line react/no-array-index-key
})}

{[...Array(redBlocks)].map((item, index) => {
return <View key={index} style={[styles.block, styles.redBlock]} />; // eslint-disable-line react/no-array-index-key
{[...Array(deletedBlocks)].map((item, index) => {
return <DeletedBlock key={index} />; // eslint-disable-line react/no-array-index-key
})}

{[...Array(greyBlocks)].map((item, index) => {
return <View key={index} style={[styles.block, styles.greyBlock]} />; // eslint-disable-line react/no-array-index-key
{[...Array(neutralBlocks)].map((item, index) => {
return <NeutralBlock key={index} />; // eslint-disable-line react/no-array-index-key
})}
</Component>
</Container>
);
};
9 changes: 9 additions & 0 deletions src/config/fonts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,12 @@ export const fonts = {
fontPrimaryBold: { fontFamily: 'Nunito-Bold' },
fontCode: { fontFamily: 'Menlo' },
};

export const fontsStyled = {
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 @josenaranjo is working on something related to fonts as he mentioned here AFAIK he created a new file called styledFonts.js inside /config, maybe you can talk to him before merging this PR so you don't get any merge conflict in the future. FWIW I like the idea of having a new fresh file with fonts.

Copy link
Copy Markdown
Member Author

@machour machour Oct 21, 2017

Choose a reason for hiding this comment

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

styledFonts.js looks good to me. I'll wait on @josenaranjo PR to get merged before changing this one

Copy link
Copy Markdown
Member

@alejandronanez alejandronanez Oct 23, 2017

Choose a reason for hiding this comment

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

@machour ok, @josenaranjo's PR was merged #561 You can start using his font file.

Copy link
Copy Markdown
Member

@andrewda andrewda Oct 23, 2017

Choose a reason for hiding this comment

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

🤔 I actually like having all the fonts in one file. Not sure though.

Copy link
Copy Markdown
Member Author

@machour machour Oct 24, 2017

Choose a reason for hiding this comment

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

seconding @andrewda, keeping both variables in the same file will help making sure we change both when updating something.

@alejandronanez Is it okay of I merge it back in fonts.js ?

Copy link
Copy Markdown
Member

@alejandronanez alejandronanez Oct 25, 2017

Choose a reason for hiding this comment

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

Sounds good to me, do you mind removing the file @josenaranjo added on his PR and doing a small refactor on his component so we don't introduce more tech debt in here?

fontPrimaryLight: { 'font-family': 'Nunito-Light' },
fontPrimary: { 'font-family': 'Nunito-Regular' },
fontPrimaryItalic: { 'font-family': 'Nunito-Italic' },
fontPrimarySemiBold: { 'font-family': 'Nunito-SemiBold' },
fontPrimaryBold: { 'font-family': 'Nunito-Bold' },
fontCode: { 'font-family': 'Menlo' },
};