Skip to content

React-styling Done #52

Open
Maciejnecka wants to merge 9 commits intodevmentor-pl:masterfrom
Maciejnecka:master
Open

React-styling Done #52
Maciejnecka wants to merge 9 commits intodevmentor-pl:masterfrom
Maciejnecka:master

Conversation

@Maciejnecka
Copy link

No description provided.

Copy link
Owner

@devmentor-pl devmentor-pl left a comment

Choose a reason for hiding this comment

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

Maćku,

Całość prezentuje się bardzo dobrze! 👍
Parę drobnych uwag zostawiłem w komentarzach :)

01/Task01.js Outdated
</Row>
)
}
const theme = {
Copy link
Owner

Choose a reason for hiding this comment

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

Często wrzuca się do osobnego pliku ;)

02/Task02.js Outdated
</Row>
)
}
const theme = {
Copy link
Owner

Choose a reason for hiding this comment

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

Można mieć też jeden motyw dla całego projektu :)

</Col>
</Row>
);
};
Copy link
Owner

Choose a reason for hiding this comment

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

👍

}
const Alert = (props) => {
return (
<ThemeProvider theme={props.theme || {}}>
Copy link
Owner

Choose a reason for hiding this comment

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

Rozumiem, że to zadanie itp. ale docelowo Provider definiujemy gdzieś na górze hierarchii komponentów, aby wygodnie stylować przepływem theme - to tak na przyszłość

color: #fff;
background-color: ${(props) =>
props.theme[props.variant] || props.theme.default};
`;
Copy link
Owner

Choose a reason for hiding this comment

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

👍

);
};

export default BreadcrumbItem;
Copy link
Owner

Choose a reason for hiding this comment

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

👍

}
`;

export { StyledBreadcrumbItem };
Copy link
Owner

Choose a reason for hiding this comment

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

👍

Comment on lines 22 to 26
props.size === 'lg' &&
css`
font-size: 18px;
padding: 1rem 1.5rem;
`}
Copy link
Owner

Choose a reason for hiding this comment

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

Może wygodniej byłoby pobierać z theme tj. co na zasadzie props.theme[props.size]

}
`;

export { StyledCard };
Copy link
Owner

Choose a reason for hiding this comment

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

👍

);
};

export default Tabs;
Copy link
Owner

Choose a reason for hiding this comment

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

👍

Copy link
Owner

@devmentor-pl devmentor-pl left a comment

Choose a reason for hiding this comment

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

Maćku,

Super, o to chodziło!

<Alert variant="secondary">
Uwaga! <em>Styled Components</em> nadchodzi!
</Alert>
</ThemeProvider>
Copy link
Owner

Choose a reason for hiding this comment

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

👍

</BreadcrumbItem>
<BreadcrumbItem active>Data</BreadcrumbItem>
</Breadcrumb>
</ThemeProvider>
Copy link
Owner

Choose a reason for hiding this comment

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

👍

</p>
</Tab>
<Tab eventKey="contact" title="Contact" disabled="disabled">
<Tab eventKey="contact" title="Contact" disabled={true}>
Copy link
Owner

Choose a reason for hiding this comment

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

👍

<Button variant="primary" size="medium">
Go somewhere
</Button>
</ThemeProvider>
Copy link
Owner

Choose a reason for hiding this comment

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

👍

size && theme.button[size].borderRadius};
text-align: center;
font-size: 1rem;
font-size: ${({ size, theme }) => size && theme.button[size].fontSize};
Copy link
Owner

Choose a reason for hiding this comment

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

👍

button: button,
};

export default theme;
Copy link
Owner

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments