Styled components example consistency #15
Conversation
|
Thanks for the indentation! For the props usage, was this before or after I created |
| @@ -549,7 +549,7 @@ const Button = styled.button` | |||
| } | |||
|
|
|||
| &:focus { | |||
| outline: 3px solid ${props => props.theme.primaryHoverColor}; | |||
| outline: 3px solid ${defaultTheme.primaryHoverColor}; | |||
There was a problem hiding this comment.
This usage of props and the next one are being used after the defaultTheme is introduced, but before the extensible themes are introduced as a prop and the prop syntax is explained later down ~line 926.
There was a problem hiding this comment.
Ah so you think I should just clarify why we're doing these changes a bit more?
There was a problem hiding this comment.
You do a great job of explaining the props changes, just a little bit after this part of the example.
| @@ -943,11 +943,11 @@ const BUTTON_MODIFIERS = { | |||
| `, | |||
| warning: ({ theme }) => ` | |||
| background-color: ${theme.warningColor}; | |||
| color: ${props => props.theme.textColorInverted}; | |||
There was a problem hiding this comment.
Here, the props are being used after being introduced, but are being used sometimes in conjunction with the destructuring syntax which is a bit confusing:
background-color: ${theme.warningColorHover}; outline: 3px solid ${props => props.theme.warningColorHover};
But I can definitely understand if you want to expose the reader to both.
There was a problem hiding this comment.
OH you're right! I missed this :)
|
I tried to explain better by pointing out specific lines :). Let me know if it doesn't make any sense, or if you want to just scrap the pr! |
This PR requests a couple changes
Before the theme usage was introduced, there were a couple examples with colors being pulled from props instead of the default theme which was a little confusing.
When building the BUTTON_MODIFIERS object after themes were introduced, most of the colors were coming from the destructed
theme, but there were a couple examples that were pulling the colors off of props. I'm not sure if this was intended, but I found it a little confusing. I don't have any experience with modifiers and I was unsure if the two methods were separate concepts being introduced or two ways to do the same thing.Some of the modifiers code was missing indentation