react-theme-provider icon indicating copy to clipboard operation
react-theme-provider copied to clipboard

Adds ability to override theme as second argument in withTheme

Open peterpme opened this issue 5 years ago • 1 comments

Summary

Hi, I noticed that useTheme allows you to (partially) override the theme. There are situations where I'd like to be able to do that with withTheme, ie:

export default withTheme(MyScreen, { primary: 'orange' })

I have tried to look at ways of solving this otherwise but we don't support hooks yet and this felt like the right way.

The complicated part is how this line is handled:

const result = a && b && a !== b ? deepmerge(a, b) : a || b;

My simple solution is;

let result = a && b && a !== b ? deepmerge(a, b) : a || b;
result = c && result !== c ? deepmerge(result, c) : result;

but I know I can improve on this

Test plan

My solution is not 100% yet but it's in a working state so I wanted to open up the PR early and get your thoughts

Thank you!

peterpme avatar Jun 19 '20 15:06 peterpme

@peterpme What's your use case?

When you wrap your component with withTheme HOC then the component automatically gets the ability to accept a theme prop where you can pass whole theme or part of the theme to override it. useTheme hook accepts theme as an argument just to imitate that functionality which we get out of the box with HOC.

If you need the same component to use a different theme then I would suggest creating another component on top of the first one and passing theme props you'd like to change.

Trancever avatar Dec 18 '20 11:12 Trancever