Skip to content

Conversation

@ZahraTee
Copy link
Contributor

@ZahraTee ZahraTee commented Dec 28, 2017

Screenshots

Before After
before after

Part of #532.

@machour
Copy link
Member

machour commented Dec 28, 2017

Since this is affecting the UI, could you provide us with two screenshots of the organization screen?
(One without the PR, and one with this PR applied)

const styles = StyleSheet.create({
listTitle: {
const DescriptionListItem = styled(ListItem).attrs({
titleStyle: {
Copy link
Member

Choose a reason for hiding this comment

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

I think this one can be removed (unused)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 40.544% when pulling f026676 on ZahraTee:styled-components-organization-screen into 5691f00 on gitpoint:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 40.544% when pulling 3dbc799 on ZahraTee:styled-components-organization-screen into 5691f00 on gitpoint:master.

@machour
Copy link
Member

machour commented Dec 28, 2017

LGTM, thanks!

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.

Looks awesome thanks so much 🙏 Just one tiny change, then looks good to me

...fonts.fontPrimary,
},
});
})``;
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove this? Was probably just added by prettier for some reason, but I think you can safely get rid of it.

Copy link
Member

Choose a reason for hiding this comment

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

It's needed by styled components when using attrs()

Copy link
Member

Choose a reason for hiding this comment

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

Ahh alright, yes you're right.

@andrewda andrewda dismissed their stale review December 28, 2017 19:33

not an issue

Copy link
Member

@housseindjirdeh housseindjirdeh left a comment

Choose a reason for hiding this comment

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

🕺

@housseindjirdeh housseindjirdeh merged commit 38f1318 into gitpoint:master Jan 3, 2018
@ZahraTee ZahraTee deleted the styled-components-organization-screen branch July 20, 2021 13:53
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.

7 participants