Skip to content

Commit f20cc42

Browse files
committed
fix(ui): scroll-to + flash the target Settings section on every redirect
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>
1 parent 5598451 commit f20cc42

4 files changed

Lines changed: 49 additions & 15 deletions

File tree

apps/desktop/src/App.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
useAnalyticsEnabled,
1818
useActiveNav,
1919
useNavigateTo,
20+
useSetPendingSettingsSection,
2021
} from '@/stores';
2122
import { NAV_ZONES, NAV_SETTINGS } from '@/lib/navigation';
2223
import { spaceAccentColor } from '@/lib/spaceAccent';
@@ -97,6 +98,7 @@ function AppContent() {
9798

9899
const activeNav = useActiveNav();
99100
const navigateTo = useNavigateTo();
101+
const setPendingSettingsSection = useSetPendingSettingsSection();
100102
const [availableUpdate, setAvailableUpdate] = useState<{ version: string } | null>(null);
101103

102104
// Auto-check for updates on startup (silent check after 5 seconds).
@@ -348,6 +350,8 @@ function AppContent() {
348350
</span>
349351
<button
350352
onClick={() => {
353+
// Land on (and flash) the Updates section, not the top of Settings.
354+
setPendingSettingsSection('updates');
351355
navigateTo('settings');
352356
setAvailableUpdate(null);
353357
}}

apps/desktop/src/components/ConnectionCard.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
Sliders,
1111
} from 'lucide-react';
1212
import { Card, Button } from '@mcpmux/ui';
13-
import { useViewSpace, useNavigateTo } from '@/stores';
13+
import { useViewSpace, useNavigateTo, useSetPendingSettingsSection } from '@/stores';
1414
import { useGatewayControl } from '@/features/gateway/useGatewayControl';
1515
import { useGatewayEvents } from '@/hooks/useDomainEvents';
1616
import {
@@ -40,6 +40,7 @@ function extractPort(url: string | null): string {
4040
export function ConnectionCard() {
4141
const viewSpace = useViewSpace();
4242
const navigateTo = useNavigateTo();
43+
const setPendingSettingsSection = useSetPendingSettingsSection();
4344
const gatewayControl = useGatewayControl();
4445

4546
const [status, setStatus] = useState<{ running: boolean; url: string | null }>({
@@ -183,7 +184,11 @@ export function ConnectionCard() {
183184
</label>
184185
<button
185186
type="button"
186-
onClick={() => navigateTo('settings')}
187+
onClick={() => {
188+
// Land on (and flash) the Gateway section where the port lives.
189+
setPendingSettingsSection('gateway');
190+
navigateTo('settings');
191+
}}
187192
className="group inline-flex items-center gap-1 text-xs text-[rgb(var(--muted))] hover:text-[rgb(var(--foreground))] transition-colors"
188193
data-testid="connection-port-settings-link"
189194
>

apps/desktop/src/features/settings/SettingsPage.tsx

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -68,17 +68,36 @@ export function SettingsPage() {
6868

6969
// Deep-link: when another surface routes here for a specific section, scroll
7070
// it into view and briefly flash it so the user lands on the right control.
71+
// Generic over section keys — any surface can target a section by calling
72+
// `setPendingSettingsSection('<key>')` before `navigateTo('settings')`. A
73+
// section becomes targetable by wrapping its card with `registerSection` +
74+
// `sectionFlashClass` (see `<SECTION_KEYS>` below).
7175
const pendingSection = usePendingSettingsSection();
7276
const clearPendingSection = useSetPendingSettingsSection();
73-
const securityRef = useRef<HTMLDivElement>(null);
74-
const [flashSecurity, setFlashSecurity] = useState(false);
77+
const sectionEls = useRef<Record<string, HTMLDivElement | null>>({});
78+
const [flashedSection, setFlashedSection] = useState<string | null>(null);
79+
80+
const registerSection = (key: string) => (el: HTMLDivElement | null) => {
81+
sectionEls.current[key] = el;
82+
};
83+
const sectionFlashClass = (key: string) =>
84+
flashedSection === key
85+
? 'rounded-xl ring-2 ring-primary-500 ring-offset-2 ring-offset-[rgb(var(--background))] transition-shadow duration-500'
86+
: 'rounded-xl ring-0 transition-shadow duration-500';
7587

7688
useEffect(() => {
77-
if (pendingSection !== 'security' || !securityRef.current) return;
78-
securityRef.current.scrollIntoView({ behavior: 'smooth', block: 'center' });
79-
setFlashSecurity(true);
89+
if (!pendingSection) return;
90+
const el = sectionEls.current[pendingSection];
91+
// Unknown or not-yet-mounted section: drop the request so a stale value
92+
// doesn't fire the flash on a later, unrelated render.
93+
if (!el) {
94+
clearPendingSection(null);
95+
return;
96+
}
97+
el.scrollIntoView({ behavior: 'smooth', block: 'center' });
98+
setFlashedSection(pendingSection);
8099
clearPendingSection(null);
81-
const t = setTimeout(() => setFlashSecurity(false), 2200);
100+
const t = setTimeout(() => setFlashedSection(null), 2200);
82101
return () => clearTimeout(t);
83102
}, [pendingSection, clearPendingSection]);
84103

@@ -378,7 +397,9 @@ export function SettingsPage() {
378397
</div>
379398

380399
{/* Updates Section */}
381-
<UpdateChecker />
400+
<div ref={registerSection('updates')} className={sectionFlashClass('updates')}>
401+
<UpdateChecker />
402+
</div>
382403

383404
{/* Startup & System Tray Section - always show toggles so e2e and slow backends see the section */}
384405
<Card data-testid="settings-startup-section">
@@ -474,6 +495,7 @@ export function SettingsPage() {
474495
</Card>
475496

476497
{/* Gateway Section — port override + reset to default */}
498+
<div ref={registerSection('gateway')} className={sectionFlashClass('gateway')}>
477499
<Card data-testid="settings-gateway-section">
478500
<CardHeader>
479501
<CardTitle className="flex items-center gap-2">
@@ -612,8 +634,10 @@ export function SettingsPage() {
612634
)}
613635
</CardContent>
614636
</Card>
637+
</div>
615638

616639
{/* Workspaces Section */}
640+
<div ref={registerSection('workspaces')} className={sectionFlashClass('workspaces')}>
617641
<Card data-testid="settings-workspaces-section">
618642
<CardHeader>
619643
<CardTitle className="flex items-center gap-2">
@@ -646,16 +670,13 @@ export function SettingsPage() {
646670
</div>
647671
</CardContent>
648672
</Card>
673+
</div>
649674

650675
{/* Security Section */}
651676
<div
652-
ref={securityRef}
677+
ref={registerSection('security')}
653678
id="settings-security"
654-
className={
655-
flashSecurity
656-
? 'rounded-xl ring-2 ring-primary-500 ring-offset-2 ring-offset-[rgb(var(--background))] transition-shadow duration-500'
657-
: 'rounded-xl ring-0 transition-shadow duration-500'
658-
}
679+
className={sectionFlashClass('security')}
659680
>
660681
<Card data-testid="settings-security-section">
661682
<CardHeader>

tests/ts/components/App.test.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
22
import { render, screen, waitFor, act } from '@testing-library/react';
33
import userEvent from '@testing-library/user-event';
4+
import { useAppStore } from '@/stores';
45

56
// ---------- Hoisted mock functions (available before vi.mock factories run) ----------
67

@@ -400,5 +401,8 @@ describe('App – update banner', () => {
400401
// Settings page should be rendered
401402
expect(screen.getByTestId('settings-page')).toBeInTheDocument();
402403
});
404+
// ...and it targets the Updates section so SettingsPage scrolls/flashes
405+
// there rather than dumping the user at the top of the page.
406+
expect(useAppStore.getState().pendingSettingsSection).toBe('updates');
403407
});
404408
});

0 commit comments

Comments
 (0)