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

Theme can now be changed by changing the theme prop #2891

Merged
merged 10 commits into from
Apr 19, 2021

Conversation

hetanthakkar
Copy link
Contributor

What kind of change does this PR introduce?

Added ability to change the theme for a component tree via state change
Fixed issue #2754

@hetanthakkar
Copy link
Contributor Author

@flyingcircle Sorry for reopening this PR again. I have made the necessary changes and made the code much simpler. Can you please review!? I closed that PR because I accidentally messed up with git and deleted that branch. Will be cautious next time. Sorry for trouble

@codecov
Copy link

codecov bot commented Mar 21, 2021

Codecov Report

Merging #2891 (501d467) into next (a5c0931) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #2891      +/-   ##
==========================================
+ Coverage   88.20%   88.24%   +0.03%     
==========================================
  Files          50       50              
  Lines        1009     1012       +3     
  Branches      402      401       -1     
==========================================
+ Hits          890      893       +3     
  Misses         76       76              
  Partials       43       43              
Impacted Files Coverage Δ
src/config/ThemeProvider.tsx 100.00% <100.00%> (ø)
src/searchbar/SearchBar.tsx 100.00% <0.00%> (ø)
src/searchbar/SearchBar-ios.tsx 91.22% <0.00%> (ø)

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 a5c0931...501d467. Read the comment docs.

@flyingcircle flyingcircle linked an issue Mar 21, 2021 that may be closed by this pull request
@hetanthakkar
Copy link
Contributor Author

@flyingcircle yeah! you are right. We need to detect that. I think I need to get some sleep. I am making too many silly mistakes.

@flyingcircle
Copy link
Collaborator

CI is saying you're failing the unit tests. Please fix first.

@hetanthakkar
Copy link
Contributor Author

@flyingcircle I have also added isTheme condition in the if statement to check if the theme has been supplied. It is necessary to check as a test case in test file was failing as its style got changed in getDerivedStateFromProps because state.theme!==props.theme always holds true in the beginning if the theme is not supplied

@hetanthakkar
Copy link
Contributor Author

@flyingcircle Please review!

@hetanthakkar
Copy link
Contributor Author

@flyingcircle Can you please review!

@flyingcircle flyingcircle merged commit 75f02c5 into react-native-elements:next Apr 19, 2021
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.

Ability to change theme for a component tree via state change
2 participants