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

Add React Native Ratings dependency #1671

Merged
merged 10 commits into from
Jan 7, 2019
Merged

Add React Native Ratings dependency #1671

merged 10 commits into from
Jan 7, 2019

Conversation

Monte9
Copy link
Collaborator

@Monte9 Monte9 commented Dec 16, 2018

This PR imports Rating from react-native-ratings package and re-exports it through the index file.

Context:

I created the react-native-ratings and wanted it to be available through RNE. Hence at the time, I copied the source over into RNE. But this has led to a disconnect between the two packages and hence we see a lot of issues related to something that might have already been fixed in the other package.

Hence as a work around, I am removing the source code for ratings from RNE and will just be maintaining it over at react-native-ratings. This way, all issues can be directed to that repo and closed here since they are non-actionable through RNE.


Fixes: https://github.com/react-native-training/react-native-elements/issues/1630

src/index.js Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Dec 16, 2018

Codecov Report

Merging #1671 into next will increase coverage by 1.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            next    #1671      +/-   ##
=========================================
+ Coverage   86.8%   87.84%   +1.03%     
=========================================
  Files         34       33       -1     
  Lines        629      551      -78     
  Branches      81       66      -15     
=========================================
- Hits         546      484      -62     
+ Misses        70       56      -14     
+ Partials      13       11       -2
Impacted Files Coverage Δ
src/tile/Tile.js 100% <0%> (ø) ⬆️
src/input/Input.js 100% <0%> (ø) ⬆️
src/searchbar/SearchBar-android.js 100% <0%> (ø) ⬆️
src/tile/FeaturedTile.js 100% <0%> (ø) ⬆️

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 890d4c9...59cba3d. Read the comment docs.

@iRoachie
Copy link
Collaborator

Are there any docs that need to be updated?

@Monte9
Copy link
Collaborator Author

Monte9 commented Jan 5, 2019

Ah good catch. Will update docs too.

@iRoachie
Copy link
Collaborator

iRoachie commented Jan 5, 2019

Alright, only thing left would be typescript definitions. Instead of adding them here I'll PR over on your repo.

@iRoachie
Copy link
Collaborator

iRoachie commented Jan 5, 2019

PR is here Monte9/react-native-ratings#36. So the process is:

  1. Merge the typescript PR
  2. Release a new version of react-native-ratings
  3. Add new version here in package.json
  4. Delete local types and use the ones from your library directly (i can do this)

@Monte9
Copy link
Collaborator Author

Monte9 commented Jan 5, 2019

@iRoachie I have published a new version of react-native-ratings v6.3 and have updated this PR to use that version. fyi.

@iRoachie
Copy link
Collaborator

iRoachie commented Jan 5, 2019

Alright @Monte9, off to you now to add the docs part for AirbnbRating and that should be it

@iRoachie
Copy link
Collaborator

iRoachie commented Jan 5, 2019

Just tried it with the ThemeProvider and everything works.

const theme = {
  Rating: {
    type: 'bell',
  },
  AirbnbRating: {
    count: 3,
  },
};

export default () => (
  <ThemeProvider theme={theme}>
    <View style={styles.container}>
      <AirbnbRating />
      <Rating />
    </View>
  </ThemeProvider>
);

screen shot 2019-01-05 at 7 32 22 pm

@Monte9
Copy link
Collaborator Author

Monte9 commented Jan 6, 2019

Awesome! Great work! 💯

I'll follow up with the docs tomorrow and we can merge it in.

@iRoachie iRoachie merged commit ca5b517 into next Jan 7, 2019
@iRoachie iRoachie deleted the add-ratings-package branch January 7, 2019 01:28
@iRoachie
Copy link
Collaborator

iRoachie commented Jan 7, 2019

And we are merged!! 💯 Thanks for taking this on 🥇

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.

2 participants