-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
fixed withTheme #2033
fixed withTheme #2033
Conversation
Can we add a test case to this scenario? @theboyWhoCriedWoolf |
Tests added, I also changed the determineTheme method to prevent it from throwing an error and stay consistent with previous functionality of just throwing a warning if no theme is there. Happy to make further changes if required. Thanks |
src/utils/determineTheme.js
Outdated
@@ -9,8 +9,8 @@ export default (props: Props, fallbackTheme: any, defaultProps: any) => { | |||
|
|||
/* eslint-disable react/prop-types */ | |||
const isDefaultTheme = defaultProps ? props.theme === defaultProps.theme : false; | |||
|
|||
const theme = props.theme && !isDefaultTheme ? props.theme : fallbackTheme || defaultProps.theme; | |||
const defaultPropsTheme = (defaultProps || {}).theme; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use EMPTY_OBJECT
from utils/empties
? And let's get rid of the extra variable allocation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Thank you so much for helping us improve styled-components! Based on our Community Guidelines every person that has a PR of any kind merged is offered an invitation to the styled-components organization—that is you right now! Should you accept, you'll get write access to the main repository. (and a fancy |
Updated withTheme to take default props from WrappedComponent. Fixes issues when ThemeProvider is not used but theme is defined in defaultProps.