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

Fix Browser Injection for both platforms #113

Merged
merged 12 commits into from
Sep 1, 2018

Conversation

brunobar79
Copy link
Contributor

Description

  • Added progress bar for Android

  • Fixed all android build warnings

  • Migrated to react-native-web3-webview which handles both iOS and android injection correctly.

    • For iOS there are no major changes, except we now own the package we use and I've created a standard API for both platforms.

    • For Android this allows to intercept http requests which is a much more solid approach than before.

Web3 injection + provider now works reliably on both platforms.

DEMO:

tgri0haz88

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

Issue

Resolves #97, #3 and #22

@brunobar79 brunobar79 requested a review from bitpshr September 1, 2018 21:09
Copy link
Contributor

@bitpshr bitpshr left a comment

Choose a reason for hiding this comment

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

I love that you externalized the custom webview stuff. This is huge. Awesome work.

@@ -118,4 +118,11 @@ class InpageBridge {

window.ethereum = new InpageBridge();

window.originalPostMessage({ type: 'ETHEREUM_PROVIDER_SUCCESS' }, '*');
function safePostMessage(msg) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Was there a race condition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, there was on android. We need to wait for the RN bridge to be ready to apply the polyfill and sometimes it happens a little bit after the injection. Then it will trigger a JS exception and it will break everything else.

echo "Done"

echo "9. Fix all android build warnings..."
Copy link
Contributor

Choose a reason for hiding this comment

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

😦Android is killin' us.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those fixes are because I upgraded the project to use a more 'up-to-date' gradle version and build targets to use more modern webview features. They deprecated the way you require dependencies and it will break by the end of 2018, so better fixing it now until each pkg owner fixes it themselves

Copy link
Contributor

Choose a reason for hiding this comment

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

with these changes I'm not able to run npm install taking out the section 9. Fix all android build warnings... works fine.

@brunobar79 brunobar79 merged commit 2626a65 into master Sep 1, 2018
@brunobar79
Copy link
Contributor Author

brunobar79 commented Sep 2, 2018 via email

@brunobar79 brunobar79 deleted the browser-injection-android branch February 11, 2019 21:41
rickycodes pushed a commit that referenced this pull request Jan 31, 2022
* android working with request interception

* fix reload

* progress bar working on android

* working like a charm

* migrate to react-native-web3-webview

* fix unit tests

* update unit tests

* working

* clean up

* cleanup

* use mac osx for builds

* fix circle config
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.

3 participants