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

refactor: use styled components in Badge #981

Merged
merged 6 commits into from
Sep 21, 2019

Conversation

LeandroTorresSicilia
Copy link
Member

@LeandroTorresSicilia LeandroTorresSicilia commented Sep 21, 2019

Changes proposed in this PR:

@nexxtway/react-rainbow

@@ -0,0 +1,9 @@
import styled from 'styled-components';

const Truncate = styled.span`
Copy link
Collaborator

Choose a reason for hiding this comment

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

TruncatedText??

@gustavopch
Copy link

In case you don't know, you don't need to import those theme variables. Instead, you could do something like:

color: ${props => props.theme.COLOR_GRAY_4}

Of course you'd have to change the Application component to have a ThemeProvider. Then, when lib is fully refactored with styled-components, it'll be a matter of changing Application to take a theme prop in order to let each consumer customize their themes or implement a dark mode.

@LeandroTorresSicilia
Copy link
Member Author

@gustavopch
That's right, but I think first we should move forward refactoring all components to use styled-components and removing all scss files.
For theming we need to discuss its structure and what will be customizable so it is early to use:
props.theme.COLOR_GRAY_4
although I know that we will need to touch all components again

@TahimiLeonBravo TahimiLeonBravo merged commit eb6520f into master Sep 21, 2019
@TahimiLeonBravo
Copy link
Collaborator

#950

@reiniergs reiniergs deleted the badge-styled-components branch September 22, 2019 03:41
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.

None yet

4 participants