Skip to content

Fix Browser Injection for both platforms#113

Merged
brunobar79 merged 12 commits intomasterfrom
browser-injection-android
Sep 1, 2018
Merged

Fix Browser Injection for both platforms#113
brunobar79 merged 12 commits intomasterfrom
browser-injection-android

Conversation

@brunobar79
Copy link
Copy Markdown
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
Copy Markdown
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.

window.ethereum = new InpageBridge();

window.originalPostMessage({ type: 'ETHEREUM_PROVIDER_SUCCESS' }, '*');
function safePostMessage(msg) {
Copy link
Copy Markdown
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
Copy Markdown
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.

sed -i '' -e 's/26.0.2/27.0.3/' $TARGET;
echo "Done"

echo "9. Fix all android build warnings..."
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
github-merge-queue bot pushed a commit that referenced this pull request Mar 23, 2026
## **Description**

Split from #26651 to reduce CODEOWNERS fanout.

Batch: assets
Source branch: \`origin/remove-static-hex-from-tests\`

When \`@metamask/design-tokens\` upgrades change color values, tests
that hardcode hex literals (e.g. \`'#ffffff'\`, \`'#457a39'\`) break
because the component renders the new token value while the test still
asserts the old one — even though nothing is actually wrong. This PR
fixes that brittleness across the assets scope.

**Two strategies are applied:**

1. **Replace hardcoded hex theme mocks with \`mockTheme\`** — test mocks
that hand-rolled partial theme objects (e.g. \`{ colors: { primary: {
default: '#0376C9' } } }\`) are replaced with \`jest.requireActual\` to
pull the real \`mockTheme\`. Test assertions that checked against hex
literals (e.g. \`expect(color).toBe('#457a39')\`) now reference
\`mockTheme.colors.success.default\`, so both the component and the test
always resolve the same value regardless of token package version.

2. **Add targeted \`eslint-disable\` comments** — strings like
\`'#113'\` or \`'#6904'\` are NFT token IDs, not CSS colors. The
\`color-no-hex\` rule can't distinguish them, so inline suppressions are
added to allow these legitimate domain strings without weakening the
rule elsewhere.

Together these changes make the \`@metamask/design-tokens/color-no-hex\`
lint rule enforceable across this scope, so future hex literals won't
accidentally creep back in and cause brittle tests.

## **Changelog**

CHANGELOG entry: null

## **Related issues**

Fixes:

## **Manual testing steps**

\`\`\`gherkin
Feature: color-no-hex lint compliance (assets batch)

  Scenario: user runs lint and tests
    Given the asset test files have been updated
    When user runs lint and test checks
    Then no color-no-hex violations are reported
    And all tests pass
\`\`\`

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I've included tests if applicable
- [x] I've documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I've applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Low risk: changes are confined to ESLint configuration and test
mocks/assertions to avoid hardcoded hex values, with no runtime logic
impact.
> 
> **Overview**
> **Enables `@metamask/design-tokens/color-no-hex` enforcement for the
Assets-owned UI/hooks surface** by expanding the ESLint override to
treat hex literals as errors in those directories.
> 
> Updates multiple Assets-related tests to remove hardcoded hex color
mocks and assertions by sourcing colors from the real `mockTheme` via
`jest.requireActual`, and adds targeted inline disables where strings
like `#113`/`#6904` are NFT identifiers (not color literals).
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
02aaba1. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
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