Skip to content

Conversation

@simonhoyos
Copy link
Contributor

@simonhoyos simonhoyos commented Oct 27, 2017

Related #532

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 36.163% when pulling eb0aa73 on shmesa22:refactor-comment-input into 5836450 on gitpoint:master.

@andrewda andrewda changed the title Refactor comment input to styled components refactor: comment input to styled components Oct 28, 2017
@chinesedfan
Copy link
Member

@shmesa22 Welcome to GitPoint! Can you add some comments or screenshots to make sure component styles are not broken?

Copy link
Member

@alejandronanez alejandronanez left a comment

Choose a reason for hiding this comment

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

  • Add flow annotation to PostButtonIcon
  • Add screenshot(s) of your changes

View
);

const PostButtonIcon = ({ style }) => <Icon name="send" iconStyle={style} />;
Copy link
Member

@alejandronanez alejandronanez Oct 28, 2017

Choose a reason for hiding this comment

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

I think you need to add props validation to this function.
What I think is that you need to do something like:

type PostButtonIconProps = {
  style: Object
};

const PostButtonIcon = ({ style } : PostButtonIconProps)=> <Icon name="send" iconStyle={style} />;

PostButtonIconProps is a flow type annotation.

Welcome to Gitpoint! Let us know if you have any other question!

@simonhoyos
Copy link
Contributor Author

screen shot 2017-10-28 at 11 12 05

screen shot 2017-10-28 at 11 13 33

screen shot 2017-10-28 at 11 13 56

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 36.139% when pulling 7c8310d on shmesa22:refactor-comment-input into 5836450 on gitpoint:master.

Copy link
Member

@alejandronanez alejandronanez left a comment

Choose a reason for hiding this comment

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

This LGTM.
Any thoughts @andrewda @housseindjirdeh @machour

@alejandronanez alejandronanez merged commit 5176877 into gitpoint:master Oct 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants