Fix Browser Injection for both platforms#113
Conversation
bitpshr
left a comment
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
Was there a race condition?
There was a problem hiding this comment.
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..." |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
with these changes I'm not able to run npm install taking out the section 9. Fix all android build warnings... works fine.
|
@estebanmino You’re using linux. Sed has a different syntax. That’s why.
On Sat, Sep 1, 2018 at 20:29 Esteban Miño ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In scripts/postinstall.sh
<MetaMask/website#113 (comment)>:
> echo "Done"
+
+echo "9. Fix all android build warnings..."
with these changes I'm not able to run npm install taking out the section 9.
Fix all android build warnings... works fine.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<MetaMask/website#113 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABMKWpgNw4cfRbG7W5QOye4jc07abcndks5uWyZYgaJpZM4WWXrt>
.
--
*Bruno Barbieri* // Senior Javascript Engineer @ Metamask
49 Bogart St, Suite 22, Brooklyn NY 11206
Web <https://consensys.net/> | Twitter <https://twitter.com/brunobar79> |
Linkedin <https://www.linkedin.com/in/brunobar79/> | Github
<https://github.com/brunobar79>
|
* 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
## **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 -->
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:
Checklist
Issue
Resolves #97, #3 and #22