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 plugin configuration for "[email protected]" #136

Merged
merged 2 commits into from
Sep 12, 2019

Conversation

deepueg
Copy link
Contributor

@deepueg deepueg commented Sep 11, 2019

This PR adds support for react-native-gesture-handler.

https://kmagiera.github.io/react-native-gesture-handler/docs/getting-started.html
https://github.com/kmagiera/react-native-gesture-handler.

Android requires additional code change on the delegate level once the package is added. This can be added inside a new delegate inside ern-navigation-impl

Close #115
The proposed solution in the above PR with replaceInFile can be dangerous as it changes the behavior of the base code and there is no easy way to make sure the file does not have compilation issues after the change. Also, if a plugin makes a change to the base file it can have an undesirable impact on other apps/components that do not intend to use this plugin. Please see the Readme section to see how the root view creation can be overridden.

Copy link
Contributor

@krunalsshah krunalsshah left a comment

Choose a reason for hiding this comment

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

If you wish to test it locally.

  • ern cauldron repo add local-cauldron /Code/local-cauldron
  • ern cauldron add native app test:android:1000.0.0
  • add a config/default.json
  "manifest": {
    "override": {
      "url": "<usr>/Code/electrode-native-manifest",
      "type": "partial"
    }
  }
}
  • After that any new plugin config you add to manifest and can test locally, you can use your local cauldron/manifest.
  • Sample Mini-App
    ern create miniapp TestGesture
  • Add the plugin you want to use locally
    ern add react-native-gesture-handler
  • Lastly
    ern run-android

@belemaire belemaire self-requested a review September 11, 2019 23:15
Copy link
Member

@belemaire belemaire left a comment

Choose a reason for hiding this comment

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

Could you just add a README.md file in react-native-gesture-handler_v1.4.1+ directory (as I did for realm plugin for example).
You don't have to create a demo MiniApp as I did, but just give specific instructions on what extra steps are needed to get this plugin working. We should do this for any native module that require additional steps after running ern add, otherwise users using this plugin will have no idea why it's not working and how to make it work (unless they dig into the PR that introduced the plugin and look at your comment).
If we start having more native modules with extra steps (listed in an README.md of plugin directory) we could also even easily have ern detect that the directory contains a README.md and let the user know after running ern add ... that the plugin need extra steps and log a link to the readme (or even print it in console).

@deepueg deepueg merged commit 2bebb46 into electrode-io:master Sep 12, 2019
@deepueg deepueg deleted the gesture-manager branch September 12, 2019 00:29
Copy link
Member

Choose a reason for hiding this comment

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

Trailing whitespace here. Please remove :)

@friederbluemle
Copy link
Member

@friederbluemle
Copy link
Member

Also - Did this PR supersede and close #115?

@deepueg
Copy link
Contributor Author

deepueg commented Sep 12, 2019

Merged while reviewing :P

Also useful extension: https://chrome.google.com/webstore/detail/render-whitespace-on-gith/ifdbipohclgnokjgpejhnbjdlgjkkhom?hl=en

Sorry, lets self add to the reviewer next time. Helps to know when reviewing. :-)

@deepueg
Copy link
Contributor Author

deepueg commented Sep 12, 2019

Also - Did this PR supersede and close #115?

Yes, it does.

@friederbluemle
Copy link
Member

Merged while reviewing :P
Also useful extension: chrome.google.com/webstore/detail/render-whitespace-on-gith/ifdbipohclgnokjgpejhnbjdlgjkkhom?hl=en

Sorry, lets self add to the reviewer next time. Helps to know when reviewing. :-)

Yeah no worries.. I wish GitHub would somehow make it visible in the PR "Review in progress". ;)

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.

4 participants