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.

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

fix(styled-components): Fix react-native-elements related problems #619

Merged
merged 8 commits into from
Nov 7, 2017

Conversation

machour
Copy link
Member

@machour machour commented Nov 4, 2017

Spent the afternoon fixing up the problems introduced by the styled-components migration:

  • Upgrade to react-native-elements 0.18.1 which contains a fix for styled components supports
  • Fixed problems:
    1. <EntityInfo/>: The test on the company was inverted. Also renamed the prop.
    2. <IssueDescription/>: Fix the repo link header, the issue title & use InlineLabel instead of LabelButton for issues/pulls label
    3. <IssueEventListItem/>: Fix icon styling
    4. <*List /> : Adjusted the border bottom color, as the default changed in RNE from 0.7 to 0.17 and this is why we had bolder lines.

This should fix #615 #580 and several comments in #569

@machour
Copy link
Member Author

machour commented Nov 4, 2017

We're back to normal on a lot of things <3

screen shot 2017-11-04 at 10 06 29 pm
screen shot 2017-11-04 at 10 07 05 pm

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) to 39.347% when pulling bfd0ec3 on machour:fix-styled-components into 53089b6 on gitpoint:master.

@@ -78,7 +82,7 @@ export const EntityInfo = ({ entity, orgs, locale, navigation }: Props) => {
}}
subtitle={entity.company}
onPress={() => navigateToCompany(entity.company, orgs, navigation)}
unknown={companyInOrgs(entity.company, orgs)}
hideChevron={!companyInOrgs(entity.company, orgs)}
Copy link
Member

Choose a reason for hiding this comment

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

🙏

Copy link
Member

@chinesedfan chinesedfan left a comment

Choose a reason for hiding this comment

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

@machour It is really awesome!

@machour
Copy link
Member Author

machour commented Nov 6, 2017

Last commit upgrades RNE to 0.18.2 that fixes another small problem with padding when using hideChevron on an item 🕵️

Before:
screen shot 2017-11-06 at 4 07 26 pm

After:
screen shot 2017-11-06 at 4 13 16 pm

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 39.309% when pulling a1592c0 on machour:fix-styled-components into 53089b6 on gitpoint:master.

@chinesedfan
Copy link
Member

@gitpoint/maintainers Finally, I determined to merge this important PR. I have to admit that I only tested in iOS simulator by myself. But without it, there are so many bugs in our code base.

And personally, I trust @machour have tested thoroughly in both platforms, as well as @andrewda 's review. In the worst case, we can fix later. :)

@chinesedfan chinesedfan merged commit 100b864 into gitpoint:master Nov 7, 2017
@machour machour deleted the fix-styled-components branch November 7, 2017 13:55
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.

IssueDescription is not rendered very well
4 participants