Invisible webview rebased #27
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds an "invisible webview" capability to the wallet application by rebasing PR #25 onto the dev branch. The changes introduce a webview-based wallet management system that runs wallet operations in the background while maintaining the existing mobile UI.
Key changes:
- Created a complete wallet webview implementation with webpack-based build system
- Replaced direct wallet manager usage with webview-based communication
- Removed iOS notification service extension components
Reviewed Changes
Copilot reviewed 33 out of 39 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| wallet/* | New webview wallet implementation with TypeScript, webpack config, and authentication management |
| context/WalletWebViewContext.tsx | New context provider that manages webview communication for wallet operations |
| app/auth/* | Updated authentication flows to use webview events instead of direct manager calls |
| ios/* | Removed notification service extension and updated iOS configuration |
| app.json | Added new URL schemes and updated iOS entitlements |
Comments suppressed due to low confidence (1)
context/WalletWebViewContext.tsx:1
- Line 126 checks
this.setSelectedWabUrlinstead ofthis.selectedWabUrl. This should be checking the URL property, not the setter method.
const F = 'context/WalletWebViewContext'
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| setTimeout(() => { | ||
| shadowWebviewRef.current?.injectJavaScript(script); | ||
| }, 3000); |
There was a problem hiding this comment.
The hardcoded 3-second timeout is a magic number that could cause timing issues. Consider using a more reliable method to ensure webview readiness or make this timeout configurable.
wallet/src/eventManager.ts
Outdated
| } | ||
|
|
||
| export class EventManager { | ||
| listenners: { [key: string]: any[] } = {}; |
There was a problem hiding this comment.
Property name 'listenners' should be 'listeners' (missing 'e').
| position: 'absolute', | ||
| top: 50, | ||
| left: 0, | ||
| width: webviewShown ? '100%': 0, | ||
| height: webviewShown ? 400: 0, | ||
| zIndex: 100 |
There was a problem hiding this comment.
Hardcoded positioning values (top: 50, height: 400, zIndex: 100) make the layout brittle. Consider using responsive units or theme-based spacing constants.
context/WalletWebViewContext.tsx
Outdated
| import { logWithTimestamp } from '@/utils/logging' | ||
| import WebView from 'react-native-webview' | ||
| import { TouchableOpacity, View, Text } from 'react-native' | ||
| import webviewSource from '../wallet/dist/index.html'; |
There was a problem hiding this comment.
Importing from a build directory (dist) creates a dependency on the build process. Consider using a more robust asset management approach or ensure this path is documented as requiring a pre-build step.
|
OTP verification seems broken, lots of TODOs indicate this is probably just a draft still. |
|
@chad1blakely please take it from here, you should have access hereon. |
#25 rebased on dev