Skip to content

refactor(user-profile): use styled-components (#534)#577

Closed
dillansimmons wants to merge 1 commit intogitpoint:masterfrom
dillansimmons:styled-user-profile
Closed

refactor(user-profile): use styled-components (#534)#577
dillansimmons wants to merge 1 commit intogitpoint:masterfrom
dillansimmons:styled-user-profile

Conversation

@dillansimmons
Copy link
Copy Markdown

A little unclear what the best method for passing styling to the ImageZoom component based on if user was logged in.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.4%) to 31.277% when pulling 16ecd48 on dillansimmons:styled-user-profile into a43d3bc on gitpoint:master.

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

@dillansimmons Hope my comments can help you. :)

color: ${colors.white};
font-family: ${styledFonts.fontPrimaryBold};
font-size: ${normalize(16)};
margin-bottom: 2px;
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.

We prefer unitless styles, except for shorthands properties.

(initialUser.type === 'User' || user.type === 'User') && {
borderColor: colors.white,
borderWidth: 2,
},
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.

@dillansimmons I think you can design a prop for StyledImageZoom. For example,

const StyledImageZoom = styled(ImageZoom)`
    // blabla
    ${({isUser}) => { /* additional styles */}}
`;
// in render
<StyledImageZoom isUser={initialUser.type === 'User' || user.type === 'User'}></StyledImageZoom>

@machour
Copy link
Copy Markdown
Member

machour commented Jan 15, 2018

Hey @dillansimmons, friendly ping, are you still up for this? We're trying to close the styled-components migration 😄

@dillansimmons
Copy link
Copy Markdown
Author

Hey @machour, yeah I've stalled out on this. I get the error: Cannot create styled-component for component: undefined when trying to extend the ImageZoom component.

const StyledImageZoom = styled(ImageZoom)`
    width: 75;
    height: 75;
    margin-bottom: 20;
    border-radius: 37.5;
    ${({isUser}) => {
    `border-color: ${colors.white};
    border-width: 2;
    `}}
`;

Happy to let someone else take a shot at this, not a styled components expert by any means.

@chinesedfan
Copy link
Copy Markdown
Member

I get the error: Cannot create styled-component for component: undefined when trying to extend the ImageZoom component.

@dillansimmons I finished your work in #724. The error is due to the import sequence. And we are always glad to see your other new PRs!

@machour
Copy link
Copy Markdown
Member

machour commented Feb 23, 2018

#724 got merged, closing this PR.

@machour machour closed this Feb 23, 2018
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