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

Update the iOS and Android Catalog for React Native 0.59.9 #193

Merged
merged 10 commits into from
Jun 14, 2019

Conversation

radazzouz
Copy link
Contributor

@radazzouz radazzouz commented Mar 19, 2019

Details:

IMPORTANT: I branched off of the reinhard/update-rn-0.59.0 branch as it already had some changes which were required for iOS.

TODO:

  • Fix the "Split PDF" example.

Acceptance Criteria:

  • The iOS Catalog should work with React Native 0.59.9
  • When approved, right before merging, rebase with master and increment the package version in package.json, package-lock.json, and samples/Catalog/package.json (see example commit: 1bf805f).
  • Create a new release (and tag) with the new package version (see https://github.com/PSPDFKit/react-native/releases).

@radazzouz radazzouz changed the title [WIP] Update the iOS Catalog for React Native 0.59.0 Update the iOS Catalog for React Native 0.59.0 Mar 21, 2019
Copy link
Contributor

@steviki steviki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Not a huge fan of how the new examples with the view component look now, since we can't hijack the navigation bar anymore, as react-navigation doesn't use a native UINavigationController, but fakes the navigation UI. Too bad NavigatorIOS got removed, which used the native navigation controller. But nothing we could do about at the moment, so this is good to go!

Copy link
Contributor

@irgendeinich irgendeinich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good, but the Android Catalog doesn't run with react-native 0.59 since the react-native-camera has dependency conflicts with react-native 0.59. Our wrapper it self works and doesn't require anymore changes to work.

@radazzouz radazzouz added the HOLD label Mar 22, 2019
@radazzouz
Copy link
Contributor Author

Thanks @irgendeinich! I've labeled this PR with HOLD.

irgendeinich added 2 commits June 13, 2019 17:10
@irgendeinich
Copy link
Contributor

react-native-camera was updated and the Android catalog now works with 0.59.9.
I'll fix this https://github.com/PSPDFKit/react-native/issues/234 which happens starting with 0.58 in here before merging.

Copy link
Contributor

@nickwinder nickwinder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly, windows is still going to be lagging behind here. They'll be a migration in react-native-windows at some point when they've reimplemented the backend in C++ to support more than just UWP. But we're stuck until that work is done.
So. LGTM

@radazzouz radazzouz changed the title Update the iOS Catalog for React Native 0.59.0 Update the iOS Catalog for React Native 0.59.9 Jun 13, 2019
@radazzouz radazzouz changed the title Update the iOS Catalog for React Native 0.59.9 Update the iOS and Android Catalog for React Native 0.59.9 Jun 14, 2019
@radazzouz radazzouz added Android and removed HOLD labels Jun 14, 2019
…ManagerConfig('RCTPSPDFKitView') instead

Update testing project to use FlatList instead of ListView
Copy link
Contributor

@irgendeinich irgendeinich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@radazzouz

It's now all good from my side. Feel free to merge whenever you're ready.

@radazzouz radazzouz merged commit 4d24321 into master Jun 14, 2019
@radazzouz radazzouz deleted the rad/update-rn-0.59.0 branch June 14, 2019 11:26
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.

4 participants