Skip to content

refactor: migrate organization screen to styled-components#682

Merged
housseindjirdeh merged 2 commits intogitpoint:masterfrom
ZahraTee:styled-components-organization-screen
Jan 3, 2018
Merged

refactor: migrate organization screen to styled-components#682
housseindjirdeh merged 2 commits intogitpoint:masterfrom
ZahraTee:styled-components-organization-screen

Conversation

@ZahraTee
Copy link
Copy Markdown
Contributor

@ZahraTee ZahraTee commented Dec 28, 2017

Screenshots

Before After
before after

Part of #532.

@machour
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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
Copy Markdown

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
Copy Markdown
Member

machour commented Dec 28, 2017

LGTM, thanks!

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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