fix(ui,walletconnect): detect iPadOS desktop-mode as mobile (#16)#91
Open
Sanka-D wants to merge 3 commits into
Open
fix(ui,walletconnect): detect iPadOS desktop-mode as mobile (#16)#91Sanka-D wants to merge 3 commits into
Sanka-D wants to merge 3 commits into
Conversation
…mons#16) iPadOS 13+ defaults to a "desktop" user-agent (`Macintosh`), so the user-agent sniff in `isMobile()` classified iPads as desktop and routed them to the QR flow instead of the mobile deep-link / WalletConnect-modal flow. Pair the UA check with a touch probe — a `Macintosh` UA with `navigator.maxTouchPoints > 1` is an iPad, not a Mac (desktop Safari reports 0). Applied consistently to the UI helper (`packages/ui/src/utils.ts`, plus a new `BROWSER_PATTERNS.MAC`) and the WalletConnect adapter's own `isMobile()`, and guarded both for non-DOM environments. Adds `packages/ui/tests/utils.test.ts` covering phone/tablet/desktop detection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ect/core The iPadOS desktop-mode mobile detection was duplicated in packages/ui and the WalletConnect adapter, so the two copies could drift. Move the single implementation to @xrpl-connect/core, re-export it from @xrpl-connect/ui (preserving the UI public surface), and have the WalletConnect adapter import it from core. - Drop the now-unused BROWSER_PATTERNS.MOBILE/.MAC from packages/ui (SAFARI kept). - Relocate the exhaustive detection test matrix to core (incl. SSR/non-DOM), leaving a focused jsdom smoke test of the re-export in ui. - Require @xrpl-connect/core ^0.8.3 from ui's peerDependencies, since the re-exported isMobile is new in that release; the previous ^0.8.0 range would let a partial upgrade resolve isMobile to undefined at runtime.
Collaborator
Review: fix(ui,walletconnect): detect iPadOS desktop-mode as mobile (#16)Verdict: approve_with_nits — CI green (test-and-build + docs-build) Points positifs
Nits
TestsManquent : modalMode==='mobile-only' routing dans WalletService et WalletConnect adapter tests. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Relates to #16 ("wallet connect flow might not work on mobile").
isMobile()relied purely on a user-agent sniff. iPadOS 13+ defaults to a "desktop" user-agent (Macintosh), so iPads were classified as desktop and routed to the QR-code flow instead of the mobile deep-link / WalletConnect-modal flow — a concrete, reproducible way the connect flow misbehaves on a common mobile device.This was the most defensible, device-independent sub-issue of #16 (verifiable by mocking
navigator). The broader mobile flow (popup user-gesture timing,approval()timeout, deep-link handoff) still warrants on-device testing and is left for follow-up.Fix
Pair the UA check with a touch probe: a
MacintoshUA withnavigator.maxTouchPoints > 1is an iPad (desktop Safari reports0). Applied consistently to:packages/ui/src/utils.ts(isMobile) + newBROWSER_PATTERNS.MACinconstants.ts— this is the helperWalletServiceuses to choose the modal vs QR path.packages/adapters/walletconnect/src/walletconnect-adapter.ts(its ownisMobile) — gates themodalMode: 'mobile-only'default.Both are also guarded for non-DOM (
typeof navigator === 'undefined').Tests
Adds
packages/ui/tests/utils.test.ts(6 cases): iPhone, Android, desktop Mac (no touch), desktop Windows, iPad desktop-mode (Macintosh UA + touch) → mobile, and Mac UA withmaxTouchPoints <= 1→ not mobile. Fullbuild+test+lintgreen.🤖 Generated with Claude Code