From f20cc4223661b2a06875747e47456b713c80584e Mon Sep 17 00:00:00 2001
From: Mohammod Al Amin Ashik <maa.ashik00@gmail.com>
Date: Thu, 25 Jun 2026 16:45:44 +0800
Subject: [PATCH] fix(ui): scroll-to + flash the target Settings section on
 every redirect
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Only the Security section highlighted on deep-link; other "open Settings"
links dumped the user at the top of the page. Generalize the mechanism so any
section can be targeted, and wire the existing redirects to the right one.

- SettingsPage: replace the security-only `securityRef`/`flashSecurity` with a
  section-keyed registry (`registerSection`/`sectionFlashClass`) driven by
  `pendingSettingsSection`. The matching section scrolls into view + flashes
  the ring for 2.2s, then the pending value is cleared. Unknown/unmounted
  sections are dropped (no stale flash). Wrapped the redirect-target sections:
  Updates, Gateway, Workspaces, Security.
- ConnectionCard "Port … change in Settings" → targets the Gateway section.
- App update banner "Update now" → targets the Updates section (was top of page).
- WorkspaceInstallPanel auth nudge already targets Security — unchanged.

Test: App update-banner test now asserts the redirect sets
`pendingSettingsSection = 'updates'`. Full TS suite green (224).

Claude-Session: https://claude.ai/code/session_01Baan9JmzR43uxxRUh7CAMF
Signed-off-by: Mohammod Al Amin Ashik <maa.ashik00@gmail.com>
---
 apps/desktop/src/App.tsx                      |  4 ++
 .../desktop/src/components/ConnectionCard.tsx |  9 +++-
 .../src/features/settings/SettingsPage.tsx    | 47 ++++++++++++++-----
 tests/ts/components/App.test.tsx              |  4 ++
 4 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/apps/desktop/src/App.tsx b/apps/desktop/src/App.tsx
index b0d64656..94ca61b2 100644
--- a/apps/desktop/src/App.tsx
+++ b/apps/desktop/src/App.tsx
@@ -17,6 +17,7 @@ import {
   useAnalyticsEnabled,
   useActiveNav,
   useNavigateTo,
+  useSetPendingSettingsSection,
 } from '@/stores';
 import { NAV_ZONES, NAV_SETTINGS } from '@/lib/navigation';
 import { spaceAccentColor } from '@/lib/spaceAccent';
@@ -97,6 +98,7 @@ function AppContent() {
 
   const activeNav = useActiveNav();
   const navigateTo = useNavigateTo();
+  const setPendingSettingsSection = useSetPendingSettingsSection();
   const [availableUpdate, setAvailableUpdate] = useState<{ version: string } | null>(null);
 
   // Auto-check for updates on startup (silent check after 5 seconds).
@@ -348,6 +350,8 @@ function AppContent() {
               </span>
               <button
                 onClick={() => {
+                  // Land on (and flash) the Updates section, not the top of Settings.
+                  setPendingSettingsSection('updates');
                   navigateTo('settings');
                   setAvailableUpdate(null);
                 }}
diff --git a/apps/desktop/src/components/ConnectionCard.tsx b/apps/desktop/src/components/ConnectionCard.tsx
index 9762526b..a8050d7a 100644
--- a/apps/desktop/src/components/ConnectionCard.tsx
+++ b/apps/desktop/src/components/ConnectionCard.tsx
@@ -10,7 +10,7 @@ import {
   Sliders,
 } from 'lucide-react';
 import { Card, Button } from '@mcpmux/ui';
-import { useViewSpace, useNavigateTo } from '@/stores';
+import { useViewSpace, useNavigateTo, useSetPendingSettingsSection } from '@/stores';
 import { useGatewayControl } from '@/features/gateway/useGatewayControl';
 import { useGatewayEvents } from '@/hooks/useDomainEvents';
 import {
@@ -40,6 +40,7 @@ function extractPort(url: string | null): string {
 export function ConnectionCard() {
   const viewSpace = useViewSpace();
   const navigateTo = useNavigateTo();
+  const setPendingSettingsSection = useSetPendingSettingsSection();
   const gatewayControl = useGatewayControl();
 
   const [status, setStatus] = useState<{ running: boolean; url: string | null }>({
@@ -183,7 +184,11 @@ export function ConnectionCard() {
             </label>
             <button
               type="button"
-              onClick={() => navigateTo('settings')}
+              onClick={() => {
+                // Land on (and flash) the Gateway section where the port lives.
+                setPendingSettingsSection('gateway');
+                navigateTo('settings');
+              }}
               className="group inline-flex items-center gap-1 text-xs text-[rgb(var(--muted))] hover:text-[rgb(var(--foreground))] transition-colors"
               data-testid="connection-port-settings-link"
             >
diff --git a/apps/desktop/src/features/settings/SettingsPage.tsx b/apps/desktop/src/features/settings/SettingsPage.tsx
index 21434808..8f1822d9 100644
--- a/apps/desktop/src/features/settings/SettingsPage.tsx
+++ b/apps/desktop/src/features/settings/SettingsPage.tsx
@@ -68,17 +68,36 @@ export function SettingsPage() {
 
   // Deep-link: when another surface routes here for a specific section, scroll
   // it into view and briefly flash it so the user lands on the right control.
+  // Generic over section keys — any surface can target a section by calling
+  // `setPendingSettingsSection('<key>')` before `navigateTo('settings')`. A
+  // section becomes targetable by wrapping its card with `registerSection` +
+  // `sectionFlashClass` (see `<SECTION_KEYS>` below).
   const pendingSection = usePendingSettingsSection();
   const clearPendingSection = useSetPendingSettingsSection();
-  const securityRef = useRef<HTMLDivElement>(null);
-  const [flashSecurity, setFlashSecurity] = useState(false);
+  const sectionEls = useRef<Record<string, HTMLDivElement | null>>({});
+  const [flashedSection, setFlashedSection] = useState<string | null>(null);
+
+  const registerSection = (key: string) => (el: HTMLDivElement | null) => {
+    sectionEls.current[key] = el;
+  };
+  const sectionFlashClass = (key: string) =>
+    flashedSection === key
+      ? 'rounded-xl ring-2 ring-primary-500 ring-offset-2 ring-offset-[rgb(var(--background))] transition-shadow duration-500'
+      : 'rounded-xl ring-0 transition-shadow duration-500';
 
   useEffect(() => {
-    if (pendingSection !== 'security' || !securityRef.current) return;
-    securityRef.current.scrollIntoView({ behavior: 'smooth', block: 'center' });
-    setFlashSecurity(true);
+    if (!pendingSection) return;
+    const el = sectionEls.current[pendingSection];
+    // Unknown or not-yet-mounted section: drop the request so a stale value
+    // doesn't fire the flash on a later, unrelated render.
+    if (!el) {
+      clearPendingSection(null);
+      return;
+    }
+    el.scrollIntoView({ behavior: 'smooth', block: 'center' });
+    setFlashedSection(pendingSection);
     clearPendingSection(null);
-    const t = setTimeout(() => setFlashSecurity(false), 2200);
+    const t = setTimeout(() => setFlashedSection(null), 2200);
     return () => clearTimeout(t);
   }, [pendingSection, clearPendingSection]);
 
@@ -378,7 +397,9 @@ export function SettingsPage() {
         </div>
 
         {/* Updates Section */}
-        <UpdateChecker />
+        <div ref={registerSection('updates')} className={sectionFlashClass('updates')}>
+          <UpdateChecker />
+        </div>
 
         {/* Startup & System Tray Section - always show toggles so e2e and slow backends see the section */}
         <Card data-testid="settings-startup-section">
@@ -474,6 +495,7 @@ export function SettingsPage() {
         </Card>
 
         {/* Gateway Section — port override + reset to default */}
+        <div ref={registerSection('gateway')} className={sectionFlashClass('gateway')}>
         <Card data-testid="settings-gateway-section">
           <CardHeader>
             <CardTitle className="flex items-center gap-2">
@@ -612,8 +634,10 @@ export function SettingsPage() {
             )}
           </CardContent>
         </Card>
+        </div>
 
         {/* Workspaces Section */}
+        <div ref={registerSection('workspaces')} className={sectionFlashClass('workspaces')}>
         <Card data-testid="settings-workspaces-section">
           <CardHeader>
             <CardTitle className="flex items-center gap-2">
@@ -646,16 +670,13 @@ export function SettingsPage() {
             </div>
           </CardContent>
         </Card>
+        </div>
 
         {/* Security Section */}
         <div
-          ref={securityRef}
+          ref={registerSection('security')}
           id="settings-security"
-          className={
-            flashSecurity
-              ? 'rounded-xl ring-2 ring-primary-500 ring-offset-2 ring-offset-[rgb(var(--background))] transition-shadow duration-500'
-              : 'rounded-xl ring-0 transition-shadow duration-500'
-          }
+          className={sectionFlashClass('security')}
         >
         <Card data-testid="settings-security-section">
           <CardHeader>
diff --git a/tests/ts/components/App.test.tsx b/tests/ts/components/App.test.tsx
index 0fd35105..17825dea 100644
--- a/tests/ts/components/App.test.tsx
+++ b/tests/ts/components/App.test.tsx
@@ -1,6 +1,7 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import { render, screen, waitFor, act } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
+import { useAppStore } from '@/stores';
 
 // ---------- Hoisted mock functions (available before vi.mock factories run) ----------
 
@@ -400,5 +401,8 @@ describe('App – update banner', () => {
       // Settings page should be rendered
       expect(screen.getByTestId('settings-page')).toBeInTheDocument();
     });
+    // ...and it targets the Updates section so SettingsPage scrolls/flashes
+    // there rather than dumping the user at the top of the page.
+    expect(useAppStore.getState().pendingSettingsSection).toBe('updates');
   });
 });