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

Added replaceTheme function #1920

Merged
merged 2 commits into from Aug 1, 2019
Merged

Conversation

kuket15
Copy link
Contributor

@kuket15 kuket15 commented Jun 15, 2019

to merge the theme passed in with the default theme.

The standard updateTheme function only allows to merge the passed in theme with the current theme.

@codecov
Copy link

codecov bot commented Jun 15, 2019

Codecov Report

Merging #1920 into next will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             next   #1920      +/-   ##
=========================================
+ Coverage   95.88%   95.9%   +0.01%     
=========================================
  Files          34      34              
  Lines         681     683       +2     
  Branches       73      73              
=========================================
+ Hits          653     655       +2     
  Misses         24      24              
  Partials        4       4
Impacted Files Coverage Δ
src/config/ThemeProvider.js 100% <100%> (ø) ⬆️
src/config/withTheme.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3c78e0...d88dbbb. Read the comment docs.

@iRoachie
Copy link
Collaborator

What’s the benefit of adding this? Since it does a merge can’t you specify the properties you want to override?

@kuket15
Copy link
Contributor Author

kuket15 commented Jul 29, 2019

In my app I need to completely replace the current theme with a new one.
The standard updateTheme does a merge, so it keeps the existing properties that I don't overwrite.

E.g.

var currentTheme = {
    Button: {
      containerStyle: {
        marginHorizontal: 20,
        marginVertical: 10,
      },
      titleStyle: {
        color: 'rgb(255,255,255)',
        fontFamily: 'regular',
        fontSize: 16,
      },
    },
}

var newTheme = {
    Button: {
      containerStyle: {
        marginHorizontal: 50,
        marginVertical: 20,
      },
    },
}

In this case the updateTheme function would keep the titleStyle properties in the updated theme. I don't want to manually set the titleStyle properties to the default values for the Button component.

@iRoachie iRoachie added this to the 1.2.0 milestone Aug 1, 2019
to merge the theme passed in with the default theme.
@iRoachie iRoachie merged commit e2f2e6e into react-native-elements:next Aug 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants