From 0876a9db2dd36ff20ef04d2904b0eec14bfa7f81 Mon Sep 17 00:00:00 2001
From: tiennm99 <tiennm99@outlook.com>
Date: Tue, 1 Sep 2026 17:16:53 +0700
Subject: [PATCH] feat(ui): username flow rework

Defer username prompt to first Play click, make username editable in header chip, generate random Player-xxxxxx name on skip or deep-link, update tests accordingly.
---
 src/app/components/RegionPicker.js  | 41 +++++++++++----
 src/app/components/UsernameModal.js | 39 +++++++++++---
 src/app/page.js                     | 79 ++++++++++++++++++++++-------
 src/lib/username.js                 | 17 +++++++
 tests/e2e/helpers.js                | 14 +++++
 tests/e2e/username.spec.js          | 60 +++++++++++++++++-----
 tests/username.test.js              | 19 +++++++
 7 files changed, 220 insertions(+), 49 deletions(-)
 create mode 100644 tests/username.test.js

diff --git a/src/app/components/RegionPicker.js b/src/app/components/RegionPicker.js
index 1def547..4d0a473 100644
--- a/src/app/components/RegionPicker.js
+++ b/src/app/components/RegionPicker.js
@@ -44,13 +44,23 @@ function unavailableLabel(code) {
  * @param {string} props.code Region code.
  * @param {string} props.label Text to show.
  * @param {boolean} props.emphasis True to style as the primary action.
+ * @param {Function} props.onPlayClick Pre-navigation hook; returning true
+ *   cancels the navigation (the caller resumes it itself).
  */
-function PlayRow({ code, label, emphasis }) {
+function PlayRow({ code, label, emphasis, onPlayClick }) {
   const { panos } = coverageOf(code);
+  const href = `/game?region=${code}`;
 
   return (
     <Link
-      href={`/game?region=${code}`}
+      href={href}
+      onClick={(e) => {
+        // Modified clicks (new tab, new window) keep native behavior: the
+        // deep-linked game page can name the player itself, so hijacking the
+        // gesture would cost more than the prompt is worth.
+        if (e.metaKey || e.ctrlKey || e.shiftKey || e.altKey || e.button !== 0) return;
+        if (onPlayClick && onPlayClick(href)) e.preventDefault();
+      }}
       className={`group flex min-h-14 items-center justify-between gap-3 rounded-xl border p-4 shadow-xs transition-all duration-150 hover:border-brand/40 hover:shadow-md focus-visible:outline-none focus-visible:ring-[3px] focus-visible:ring-ring focus-visible:ring-offset-2 focus-visible:ring-offset-background ${
         emphasis ? 'city-card-accent border-border bg-card' : 'border-border/60 bg-card/60'
       }`}
@@ -60,13 +70,21 @@ function PlayRow({ code, label, emphasis }) {
           {label}
         </span>
         {isThin(code) && (
-          <span className="rounded bg-muted px-1.5 py-0.5 text-[10px] font-medium uppercase tracking-wide text-muted-foreground">
+          <span
+            className="rounded bg-muted px-1.5 py-0.5 text-[10px] font-medium uppercase tracking-wide text-muted-foreground"
+            title="Limited street imagery — repeats are likely"
+          >
             few streets
           </span>
         )}
       </span>
       <span className="flex items-center gap-3">
-        <span className="text-xs tabular-nums text-muted-foreground">{count(panos)}</span>
+        <span
+          className="text-xs tabular-nums text-muted-foreground"
+          title={`${count(panos)} street panoramas to guess from`}
+        >
+          {count(panos)} spots
+        </span>
         <span className="inline-flex items-center gap-1.5 rounded-lg bg-brand-subtle px-3 py-1.5 text-sm font-semibold text-brand-subtle-foreground transition-colors group-hover:bg-brand group-hover:text-brand-foreground">
           Play
           <ArrowRight className="size-4" aria-hidden="true" />
@@ -99,7 +117,7 @@ function UnavailableRow({ code }) {
  * The province row itself plays that province, and only the chevron expands it,
  * so the common case stays one click while 61 districts stay reachable.
  */
-export default function RegionPicker() {
+export default function RegionPicker({ onPlayClick }) {
   // Read in an effect, not during render: localStorage does not exist on the
   // server and a hydration mismatch is worse than the row appearing a frame
   // late. Only one row keeps the accent style -- two competing primary
@@ -115,10 +133,10 @@ export default function RegionPicker() {
 
   return (
     <div className="grid gap-3">
-      <PlayRow code={COUNTRY_CODE} label="Play anywhere in Vietnam" emphasis />
+      <PlayRow code={COUNTRY_CODE} label="Play anywhere in Vietnam" emphasis onPlayClick={onPlayClick} />
 
       {lastRegion && (
-        <PlayRow code={lastRegion} label={`Continue in ${getRegion(lastRegion).name}`} />
+        <PlayRow code={lastRegion} label={`Continue in ${getRegion(lastRegion).name}`} onPlayClick={onPlayClick} />
       )}
 
       <Accordion type="multiple" className="grid gap-2">
@@ -138,7 +156,10 @@ export default function RegionPicker() {
                   <span className="text-base font-semibold text-foreground">{region.name}</span>
                   <span className="flex items-center gap-2 text-xs text-muted-foreground">
                     {region.partialCoverage && (
-                      <span className="rounded bg-muted px-1.5 py-0.5 font-medium uppercase tracking-wide">
+                      <span
+                        className="rounded bg-muted px-1.5 py-0.5 font-medium uppercase tracking-wide"
+                        title="Street imagery covers only part of this province"
+                      >
                         partial
                       </span>
                     )}
@@ -149,10 +170,10 @@ export default function RegionPicker() {
               </AccordionTrigger>
 
               <AccordionContent className="grid gap-2">
-                <PlayRow code={province} label={`Play anywhere in ${region.name}`} />
+                <PlayRow code={province} label={`Play anywhere in ${region.name}`} onPlayClick={onPlayClick} />
                 {districts.map((district) =>
                   isPlayable(district) ? (
-                    <PlayRow key={district} code={district} label={getRegion(district).name} />
+                    <PlayRow key={district} code={district} label={getRegion(district).name} onPlayClick={onPlayClick} />
                   ) : (
                     <UnavailableRow key={district} code={district} />
                   )
diff --git a/src/app/components/UsernameModal.js b/src/app/components/UsernameModal.js
index a31dacc..42e8c1b 100644
--- a/src/app/components/UsernameModal.js
+++ b/src/app/components/UsernameModal.js
@@ -1,6 +1,6 @@
 "use client";
 
-import { useState } from 'react';
+import { useEffect, useState } from 'react';
 import {
   Dialog,
   DialogContent,
@@ -13,9 +13,23 @@ import { Input } from '@/components/ui/input';
 import { Label } from '@/components/ui/label';
 import { Alert, AlertDescription } from '@/components/ui/alert';
 
-export default function UsernameModal({ isOpen, onSubmit, onClose }) {
+// Set or change the leaderboard name. With no saved name yet, the secondary
+// action skips into a generated name; when editing an existing name it is a
+// plain Cancel, because "skip" would read as discarding the current name.
+export default function UsernameModal({ isOpen, onSubmit, onSkip, onClose, initialValue }) {
   const [username, setUsername] = useState('');
   const [error, setError] = useState('');
+  const hasExistingName = Boolean(initialValue);
+
+  // Re-seed the field each time the dialog opens: it stays mounted between
+  // opens, and an edit session must start from the saved name, not the
+  // leftovers of the previous visit.
+  useEffect(() => {
+    if (isOpen) {
+      setUsername(initialValue || '');
+      setError('');
+    }
+  }, [isOpen, initialValue]);
 
   const handleSubmit = (e) => {
     e.preventDefault();
@@ -48,13 +62,22 @@ export default function UsernameModal({ isOpen, onSubmit, onClose }) {
     setUsername('');
   };
 
+  const handleSecondary = () => {
+    if (!hasExistingName && onSkip) onSkip();
+    else onClose();
+  };
+
   return (
     <Dialog open={isOpen} onOpenChange={onClose}>
       <DialogContent className="sm:max-w-sm">
         <DialogHeader>
-          <DialogTitle className="text-xl font-bold">Welcome to VNGeoGuessr</DialogTitle>
-          <DialogDescription>
-            Enter a username for the leaderboard
+          <DialogTitle className="text-2xl text-center font-bold">
+            {hasExistingName ? 'Change your name' : 'Welcome to VNGeoGuessr'}
+          </DialogTitle>
+          <DialogDescription className="text-center">
+            {hasExistingName
+              ? 'Pick a new name for the leaderboard'
+              : 'Enter a username for the leaderboard'}
           </DialogDescription>
         </DialogHeader>
 
@@ -90,16 +113,16 @@ export default function UsernameModal({ isOpen, onSubmit, onClose }) {
             <Button
               type="button"
               variant="ghost"
-              onClick={onClose}
+              onClick={handleSecondary}
               className="flex-1"
             >
-              Skip
+              {hasExistingName ? 'Cancel' : 'Skip — random name'}
             </Button>
             <Button
               type="submit"
               className="flex-1"
             >
-              Start Playing
+              Save name
             </Button>
           </div>
         </form>
diff --git a/src/app/page.js b/src/app/page.js
index 6f57433..4a37d76 100644
--- a/src/app/page.js
+++ b/src/app/page.js
@@ -1,7 +1,8 @@
 "use client";
 
 import Link from "next/link";
-import { Wrench } from "lucide-react";
+import { useRouter } from "next/navigation";
+import { Beer, Wrench } from "lucide-react";
 import React, { useState, useEffect } from "react";
 import { Button } from '@/components/ui/button';
 import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card';
@@ -10,7 +11,7 @@ import UsernameModal from './components/UsernameModal';
 import DonateQRModal from './components/DonateQRModal';
 import LeaderboardModal from './components/LeaderboardModal';
 import RegionPicker from './components/RegionPicker';
-import { getUsername, setUsername } from '../lib/username';
+import { generateRandomUsername, getUsername, setUsername } from '../lib/username';
 import { SCORE_BANDS, formatDistance } from '../lib/game';
 
 const STEP_LABELS = [
@@ -35,27 +36,50 @@ const SCORING_ROWS = [
 ];
 
 export default function Home() {
+  const router = useRouter();
   const [showDonateModal, setShowDonateModal] = useState(false);
   const [showUsernameModal, setShowUsernameModal] = useState(false);
   const [username, setUsernameState] = useState('');
+  // The Play destination held while the name prompt is up. The prompt appears
+  // at the first Play click -- when the name is about to matter -- rather than
+  // ambushing on landing, so the landing page introduces the game first.
+  const [pendingHref, setPendingHref] = useState(null);
 
   useEffect(() => {
-    const existingUsername = getUsername();
-    if (existingUsername) {
-      setUsernameState(existingUsername);
-    } else {
-      setShowUsernameModal(true);
-    }
+    setUsernameState(getUsername() || '');
   }, []);
 
-  const handleUsernameSubmit = (newUsername) => {
+  const saveUsername = (newUsername) => {
     setUsername(newUsername);
     setUsernameState(newUsername);
     setShowUsernameModal(false);
   };
 
+  const handleUsernameSubmit = (newUsername) => {
+    saveUsername(newUsername);
+    if (pendingHref) {
+      setPendingHref(null);
+      router.push(pendingHref);
+    }
+  };
+
+  // Skip = play under a generated name. Persisted like a typed name, so it
+  // shows on the leaderboard and stays editable from the header chip.
+  const handleUsernameSkip = () => {
+    handleUsernameSubmit(generateRandomUsername());
+  };
+
+  // Returns true when the click is intercepted: no saved name yet, so the
+  // prompt opens and navigation resumes after save/skip.
+  const handlePlayClick = (href) => {
+    if (getUsername()) return false;
+    setPendingHref(href);
+    setShowUsernameModal(true);
+    return true;
+  };
+
   return (
-    <div className="min-h-dvh vn-gradient-bg">
+    <div className="min-h-dvh vn-surface">
       <div className="min-h-dvh">
         <div className="container mx-auto px-4 py-6 max-w-5xl">
           {/* Header */}
@@ -63,19 +87,30 @@ export default function Home() {
             <Link href="/" className="text-2xl font-bold text-foreground tracking-wider">
               VNGeoGuessr
             </Link>
-            <div className="flex items-center gap-3">
+            <div className="flex flex-wrap items-center justify-end gap-3">
               <ThemeToggle />
-              {username && (
-                <span className="text-muted-foreground text-sm hidden sm:inline">
-                  Playing as <span className="font-semibold text-brand">{username}</span>
-                </span>
-              )}
+              {/* One chip whether or not a name exists: the only way to fix a
+                  typo'd name is reopening this modal, so the entry point must
+                  always be visible -- including on phones, where it truncates
+                  rather than overflowing the header row. */}
+              <button
+                type="button"
+                onClick={() => setShowUsernameModal(true)}
+                className="min-h-11 max-w-44 truncate rounded-lg px-2 text-sm text-muted-foreground transition-colors hover:bg-muted hover:text-foreground focus-visible:outline-none focus-visible:ring-[3px] focus-visible:ring-ring"
+                title={username ? 'Change your name' : 'Set your leaderboard name'}
+              >
+                {username ? (
+                  <>Playing as <span className="font-semibold text-brand">{username}</span></>
+                ) : (
+                  'Set name'
+                )}
+              </button>
               <LeaderboardModal currentUsername={username} />
               <Button
                 onClick={() => setShowDonateModal(true)}
                 variant="outline"
               >
-                <span className="text-base leading-none" aria-hidden="true">🍺</span>
+                <Beer className="size-4" aria-hidden="true" />
                 Buy me a beer
               </Button>
             </div>
@@ -130,7 +165,7 @@ export default function Home() {
                 <CardTitle className="text-xl font-bold text-card-foreground text-center">Where to Play</CardTitle>
               </CardHeader>
               <CardContent>
-                <RegionPicker />
+                <RegionPicker onPlayClick={handlePlayClick} />
               </CardContent>
             </Card>
           </div>
@@ -153,7 +188,13 @@ export default function Home() {
         </div>
 
         <DonateQRModal isOpen={showDonateModal} onClose={() => setShowDonateModal(false)} />
-        <UsernameModal isOpen={showUsernameModal} onSubmit={handleUsernameSubmit} onClose={() => setShowUsernameModal(false)} />
+        <UsernameModal
+          isOpen={showUsernameModal}
+          onSubmit={handleUsernameSubmit}
+          onSkip={handleUsernameSkip}
+          onClose={() => { setShowUsernameModal(false); setPendingHref(null); }}
+          initialValue={username}
+        />
 
         {/* Debug Button. Safe-area offset keeps it clear of the home
             indicator; hover/active states give the press the feedback the
diff --git a/src/lib/username.js b/src/lib/username.js
index 69baf05..0e39b09 100644
--- a/src/lib/username.js
+++ b/src/lib/username.js
@@ -13,3 +13,20 @@ export function setUsername(username) {
   if (typeof window === 'undefined') return;
   localStorage.setItem(USERNAME_STORAGE_KEY, username);
 }
+
+/**
+ * A name for a player who skipped choosing one. Must satisfy the same rules
+ * the modal enforces (2-20 chars of [a-zA-Z0-9_-]), because once generated it
+ * is stored and edited exactly like a typed name.
+ * Six suffix chars: the name is the leaderboard's member key, so a collision
+ * silently merges two players' totals -- 36^6 keeps that out of birthday-odds
+ * range at any plausible player count.
+ * @returns {string} e.g. "Player-x7k2m9"
+ */
+export function generateRandomUsername() {
+  let suffix = '';
+  for (let i = 0; i < 6; i += 1) {
+    suffix += Math.floor(Math.random() * 36).toString(36);
+  }
+  return `Player-${suffix}`;
+}
diff --git a/tests/e2e/helpers.js b/tests/e2e/helpers.js
index 63722c1..7246520 100644
--- a/tests/e2e/helpers.js
+++ b/tests/e2e/helpers.js
@@ -18,6 +18,8 @@ export const PANO_IMAGE_URL = 'https://pano.invalid/e2e-round.png';
 
 export const USERNAME_STORAGE_KEY = 'vngeoguessr_username';
 
+export const HINT_STORAGE_KEY = 'vngeoguessr_hint_seen';
+
 /** /api/new-game response for one TPHCM round. */
 export function newGameResponse(sessionId, round) {
   return {
@@ -153,3 +155,15 @@ export async function seedUsername(page, username) {
     [USERNAME_STORAGE_KEY, username]
   );
 }
+
+/**
+ * Mark the one-time how-to-play hint as already seen, so specs that are not
+ * about onboarding never race its banner for clicks.
+ * @param {import('@playwright/test').Page} page Playwright page.
+ */
+export async function seedHintSeen(page) {
+  await page.addInitScript(
+    (key) => window.localStorage.setItem(key, '1'),
+    HINT_STORAGE_KEY
+  );
+}
diff --git a/tests/e2e/username.spec.js b/tests/e2e/username.spec.js
index 833f3ef..5117ce0 100644
--- a/tests/e2e/username.spec.js
+++ b/tests/e2e/username.spec.js
@@ -1,36 +1,72 @@
 import { test, expect } from '@playwright/test';
-import { stubGameApis, USERNAME_STORAGE_KEY } from './helpers.js';
+import { stubGameApis, seedUsername, seedHintSeen, USERNAME_STORAGE_KEY } from './helpers.js';
 
-// First-visit username capture. No seeded name here: the modal appearing on a
-// clean profile is the behavior under test.
+// The name prompt appears at the first Play click -- when the name is about
+// to matter -- not on landing. No seeded name in the fresh-profile tests:
+// that timing is the behavior under test.
 
 test.beforeEach(async ({ page }) => {
   await stubGameApis(page, 'fresh-player');
+  await seedHintSeen(page);
   await page.goto('/');
 });
 
-test('asks for a username on first visit and remembers it', async ({ page }) => {
+test('landing shows no prompt; the first Play click asks, saves, and starts the game', async ({ page }) => {
+  // The landing page introduces the game first: no ambush modal, just an
+  // always-visible entry point for setting a name later.
+  await expect(page.getByRole('dialog')).toBeHidden();
+  await expect(page.getByRole('button', { name: 'Set name' })).toBeVisible();
+
+  await page.getByRole('link', { name: /Play anywhere in Vietnam/ }).click();
+
   const dialog = page.getByRole('dialog', { name: /Welcome to VNGeoGuessr/ });
   await expect(dialog).toBeVisible();
-
   await dialog.getByLabel(/Username/).fill('fresh-player');
-  await dialog.getByRole('button', { name: 'Start Playing' }).click();
+  await dialog.getByRole('button', { name: 'Save name' }).click();
 
-  await expect(dialog).toBeHidden();
-  await expect(page.getByText('Playing as fresh-player')).toBeVisible();
+  // Saving resumes the intercepted navigation into the round.
+  await expect(page).toHaveURL(/\/game\?region=VN/);
   expect(
     await page.evaluate((key) => window.localStorage.getItem(key), USERNAME_STORAGE_KEY)
   ).toBe('fresh-player');
+});
 
-  // A reload must not ask again.
-  await page.reload();
-  await expect(page.getByRole('dialog', { name: /Welcome to VNGeoGuessr/ })).toBeHidden();
+test('skip generates a random name and still starts the game', async ({ page }) => {
+  await page.getByRole('link', { name: /Play anywhere in Vietnam/ }).click();
+  const dialog = page.getByRole('dialog', { name: /Welcome to VNGeoGuessr/ });
+  await dialog.getByRole('button', { name: /Skip — random name/ }).click();
+
+  await expect(page).toHaveURL(/\/game\?region=VN/);
+  expect(
+    await page.evaluate((key) => window.localStorage.getItem(key), USERNAME_STORAGE_KEY)
+  ).toMatch(/^Player-[0-9a-z]{6}$/);
 });
 
 test('rejects an invalid username with a visible error', async ({ page }) => {
+  await page.getByRole('button', { name: 'Set name' }).click();
   const dialog = page.getByRole('dialog', { name: /Welcome to VNGeoGuessr/ });
   await dialog.getByLabel(/Username/).fill('bad name!');
-  await dialog.getByRole('button', { name: 'Start Playing' }).click();
+  await dialog.getByRole('button', { name: 'Save name' }).click();
   await expect(dialog.getByRole('alert')).toContainText(/letters, numbers/);
   await expect(dialog).toBeVisible();
 });
+
+test('the header chip reopens the prompt to change an existing name', async ({ page }) => {
+  await seedUsername(page, 'first-name');
+  await page.reload();
+
+  const chip = page.getByRole('button', { name: /Playing as first-name/ });
+  await expect(chip).toBeVisible();
+  await chip.click();
+
+  const dialog = page.getByRole('dialog', { name: /Change your name/ });
+  await expect(dialog.getByLabel(/Username/)).toHaveValue('first-name');
+  await dialog.getByLabel(/Username/).fill('second-name');
+  await dialog.getByRole('button', { name: 'Save name' }).click();
+
+  await expect(dialog).toBeHidden();
+  await expect(page.getByRole('button', { name: /Playing as second-name/ })).toBeVisible();
+  expect(
+    await page.evaluate((key) => window.localStorage.getItem(key), USERNAME_STORAGE_KEY)
+  ).toBe('second-name');
+});
diff --git a/tests/username.test.js b/tests/username.test.js
new file mode 100644
index 0000000..b10d3c0
--- /dev/null
+++ b/tests/username.test.js
@@ -0,0 +1,19 @@
+import { describe, expect, it } from 'vitest';
+import { generateRandomUsername } from '../src/lib/username.js';
+
+// The generated name must satisfy the same rules UsernameModal enforces,
+// because it is stored and later edited exactly like a typed name.
+describe('generateRandomUsername', () => {
+  it('produces a name the modal validation accepts', () => {
+    for (let i = 0; i < 50; i += 1) {
+      const name = generateRandomUsername();
+      expect(name.length).toBeGreaterThanOrEqual(2);
+      expect(name.length).toBeLessThanOrEqual(20);
+      expect(name).toMatch(/^[a-zA-Z0-9_-]+$/);
+    }
+  });
+
+  it('uses the Player- prefix so generated names are recognisable', () => {
+    expect(generateRandomUsername()).toMatch(/^Player-[0-9a-z]{6}$/);
+  });
+});