-
Notifications
You must be signed in to change notification settings - Fork 2
feat(auth): offer 'Open Dash Wallet' on desktop, not just touch devices #354
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
base: staging
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -19,9 +19,15 @@ interface KeyExchangeQRProps { | |||||
| * QR code component for key exchange URI. | ||||||
| * | ||||||
| * Displays a dash-key:/dash-st: URI as a QR code that can be scanned by a | ||||||
| * wallet app. On touch devices (where scanning your own screen is impossible) | ||||||
| * it also offers an "Open in wallet app" deep link into a wallet registered | ||||||
| * for the URI scheme. Includes copy-to-clipboard functionality for manual entry. | ||||||
| * wallet app, and offers an "Open Dash Wallet" deep link into whichever wallet | ||||||
| * is registered for the URI scheme on this device. The deep link matters on | ||||||
| * desktop too — Dash Evo Tool runs on the same machine as the browser, so | ||||||
| * scanning is a detour — but the browser gives no signal when no handler is | ||||||
| * registered, so a fallback hint appears once the link has been clicked. | ||||||
| * | ||||||
| * Copy-to-clipboard is offered on desktop only: on a touch device the deep | ||||||
| * link already hands the URI to the wallet, and pasting it by hand is the | ||||||
| * path the deep link exists to avoid. | ||||||
| */ | ||||||
| export function KeyExchangeQR({ | ||||||
| uri, | ||||||
|
|
@@ -30,13 +36,19 @@ export function KeyExchangeQR({ | |||||
| }: KeyExchangeQRProps) { | ||||||
| const [copied, setCopied] = useState(false) | ||||||
| const [isTouchDevice, setIsTouchDevice] = useState(false) | ||||||
| const [launchAttempted, setLaunchAttempted] = useState(false) | ||||||
|
|
||||||
| // Coarse-pointer detection has to run client-side; the static export renders | ||||||
| // the desktop (QR-first) layout until hydration. | ||||||
| // the desktop variant until hydration. | ||||||
| useEffect(() => { | ||||||
| setIsTouchDevice(window.matchMedia('(pointer: coarse)').matches) | ||||||
| }, []) | ||||||
|
|
||||||
| // A new request (retry, or expiry-driven regeneration) starts a fresh attempt | ||||||
| useEffect(() => { | ||||||
| setLaunchAttempted(false) | ||||||
| }, [uri]) | ||||||
|
|
||||||
| const handleCopy = async () => { | ||||||
| try { | ||||||
| await navigator.clipboard.writeText(uri) | ||||||
|
|
@@ -56,16 +68,15 @@ export function KeyExchangeQR({ | |||||
|
|
||||||
| return ( | ||||||
| <div className="flex flex-col items-center gap-4"> | ||||||
| {/* Deep link for wallets installed on this device */} | ||||||
| {isTouchDevice && ( | ||||||
| <a | ||||||
| href={uri} | ||||||
| className={cn(buttonVariants({ size: 'lg' }), 'w-full gap-2')} | ||||||
| > | ||||||
| <ArrowTopRightOnSquareIcon className="w-5 h-5" /> | ||||||
| Open in Wallet App | ||||||
| </a> | ||||||
| )} | ||||||
| {/* Deep link into a wallet registered for the scheme on this device */} | ||||||
| <a | ||||||
| href={uri} | ||||||
| onClick={() => setLaunchAttempted(true)} | ||||||
| className={cn(buttonVariants({ size: 'lg' }), 'w-full gap-2')} | ||||||
| > | ||||||
| <ArrowTopRightOnSquareIcon className="w-5 h-5" /> | ||||||
| Open Dash Wallet | ||||||
| </a> | ||||||
|
|
||||||
| {/* QR Code */} | ||||||
| <div className="p-4 bg-white rounded-xl shadow-sm border-2 border-blue-500"> | ||||||
|
|
@@ -83,8 +94,8 @@ export function KeyExchangeQR({ | |||||
| <div className="text-center"> | ||||||
| <p className="text-sm text-gray-600 dark:text-gray-400"> | ||||||
| {isTouchDevice | ||||||
| ? 'Open in a wallet app on this device, or scan the QR code with a wallet on another device' | ||||||
| : 'Scan with a compatible Dash wallet, such as Dash Evo Tool'} | ||||||
| ? 'Open a wallet app on this device, or scan the QR code with a wallet on another device' | ||||||
| : 'Open a wallet on this computer, such as Dash Evo Tool, or scan the QR code with a wallet on your phone'} | ||||||
| </p> | ||||||
| {remainingTime !== null && remainingTime !== undefined && ( | ||||||
| <p className="text-xs text-gray-500 dark:text-gray-500 mt-1"> | ||||||
|
|
@@ -93,23 +104,34 @@ export function KeyExchangeQR({ | |||||
| )} | ||||||
| </div> | ||||||
|
|
||||||
| {/* Copy button */} | ||||||
| <button | ||||||
| onClick={handleCopy} | ||||||
| className="flex items-center gap-2 px-4 py-2 text-sm rounded-lg border border-gray-200 dark:border-gray-700 hover:bg-gray-50 dark:hover:bg-neutral-800 transition-colors" | ||||||
| > | ||||||
| {copied ? ( | ||||||
| <> | ||||||
| <CheckIcon className="w-4 h-4 text-green-500" /> | ||||||
| <span className="text-green-600 dark:text-green-400">Copied!</span> | ||||||
| </> | ||||||
| ) : ( | ||||||
| <> | ||||||
| <ClipboardIcon className="w-4 h-4" /> | ||||||
| <span>Copy URI</span> | ||||||
| </> | ||||||
| )} | ||||||
| </button> | ||||||
| {/* Copy button — desktop only; the deep link supersedes it on touch */} | ||||||
| {!isTouchDevice && ( | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Blocking: Preserve the same-device fallback after a touch launch attempt The
Suggested change
source: ['claude', 'codex'] |
||||||
| <button | ||||||
| onClick={handleCopy} | ||||||
| className="flex items-center gap-2 px-4 py-2 text-sm rounded-lg border border-gray-200 dark:border-gray-700 hover:bg-gray-50 dark:hover:bg-neutral-800 transition-colors" | ||||||
| > | ||||||
| {copied ? ( | ||||||
| <> | ||||||
| <CheckIcon className="w-4 h-4 text-green-500" /> | ||||||
| <span className="text-green-600 dark:text-green-400">Copied!</span> | ||||||
| </> | ||||||
| ) : ( | ||||||
| <> | ||||||
| <ClipboardIcon className="w-4 h-4" /> | ||||||
| <span>Copy URI</span> | ||||||
| </> | ||||||
| )} | ||||||
| </button> | ||||||
| )} | ||||||
|
|
||||||
| {/* Browsers stay silent when no app handles the scheme, so say what to do */} | ||||||
| {launchAttempted && ( | ||||||
| <p className="text-xs text-center text-gray-500 dark:text-gray-500"> | ||||||
| {isTouchDevice | ||||||
| ? 'Nothing opened? No wallet app on this device is registered to handle Dash login links. Scan the QR code with a wallet on another device instead.' | ||||||
| : 'Nothing opened? No wallet on this device is registered to handle Dash login links. Scan the QR code with a wallet on another device, or copy the URI and paste it into your wallet.'} | ||||||
| </p> | ||||||
| )} | ||||||
| </div> | ||||||
| ) | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Suggestion: Add smoke coverage for the desktop wallet deep link
The PR's primary desktop behavior has no automated regression coverage even though
e2e/smoke/public-pages.spec.tsalready opens/login/. Extend that test to click “Login with Wallet (QR),” wait for the locally generated request, assert that “Open Dash Wallet” has adash-key:href, and confirm that “Copy URI” is present in the desktop project. URI generation completes before response polling and requires no write credentials, so this exercises the new rendering without performing a Platform write.source: ['codex']