Skip to content

Conversation

@alejandronanez
Copy link
Member

Adding styled-components initial work! 🙌

Copy link
Member

@andrewda andrewda left a comment

Choose a reason for hiding this comment

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

👍

alejandronanez added a commit to alejandronanez/awesome-styled-components that referenced this pull request Oct 18, 2017
Waiting to merge gitpoint/git-point#495 this one before merging this change!
Thanks for creating this library, it's awesome!
@alejandronanez alejandronanez merged commit 80f383d into master Oct 18, 2017
@alejandronanez alejandronanez deleted the feat-styled-components branch October 18, 2017 02:43
});
const Container = styled.View`
align-items: center;
background-color: ${() => colors.white}
Copy link
Member

Choose a reason for hiding this comment

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

@alejandronanez I am not sure. Can it just be background-color: 'white'; ?

Copy link
Member

@andrewda andrewda Oct 18, 2017

Choose a reason for hiding this comment

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

Doing it this way makes it so we can change the white color everywhere by just changing a single value. However, why can't we just do ${colors.white} @alejandronanez?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hey @chinesedfan

  • We should avoid at all costs using plain strings for colors, that's not a good practice.
  • We already have a colors file that we should keep using.

Copy link
Member

Choose a reason for hiding this comment

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

@alejandronanez Yes, my point is the anonymous function, as @andrewda mentioned. (You merged so quickly. 😆 )

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I put a fix for that too @chinesedfan! :)

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.

4 participants