Skip to content

Commit 23cecbf

Browse files
authored
fix(admin): reset scroll position on tab navigation (#3921)
Closes #3748 Automatically scroll to the top of the main content area when switching between admin UI tabs, preventing users from landing mid-page on previously scrolled tabs. - Add requestAnimationFrame-based scroll reset in showTab() - Add data-scroll-container attribute to <main> element - Add Playwright E2E test for scroll reset behavior - Add vitest unit tests for scroll reset and fallback selector - Update fake timers config to include requestAnimationFrame Signed-off-by: Rakhi Dutta <rakhibiswas@yahoo.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
1 parent b9863b3 commit 23cecbf

6 files changed

Lines changed: 185 additions & 20 deletions

File tree

.secrets.baseline

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9647,15 +9647,15 @@
96479647
{
96489648
"hashed_secret": "d3ecb0d890368d7659ee54010045b835dacb8efe",
96499649
"is_verified": false,
9650-
"line_number": 20821,
9650+
"line_number": 20844,
96519651
"type": "Secret Keyword",
96529652
"verified_result": null,
96539653
"is_secret": false
96549654
},
96559655
{
96569656
"hashed_secret": "b4e44716dbbf57be3dae2f819d96795a85d06652",
96579657
"is_verified": false,
9658-
"line_number": 33885,
9658+
"line_number": 33908,
96599659
"type": "Secret Keyword",
96609660
"verified_result": null,
96619661
"is_secret": false

mcpgateway/static/admin.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8803,6 +8803,29 @@ function showTab(tabName) {
88038803
const panel = safeGetElement(`${tabName}-panel`);
88048804
if (panel) {
88058805
panel.classList.remove("hidden");
8806+
8807+
// Reset scroll position on the main content area
8808+
// Use requestAnimationFrame to ensure DOM updates have completed
8809+
try {
8810+
requestAnimationFrame(() => {
8811+
try {
8812+
// Try data attribute first (most specific), fall back to class selector
8813+
const mainContent =
8814+
document.querySelector("[data-scroll-container]") ||
8815+
document.querySelector("main.overflow-y-auto");
8816+
if (mainContent) {
8817+
mainContent.scrollTop = 0;
8818+
}
8819+
} catch (error) {
8820+
console.debug(
8821+
"Error resetting scroll position:",
8822+
error,
8823+
);
8824+
}
8825+
});
8826+
} catch (error) {
8827+
console.debug("requestAnimationFrame not available:", error);
8828+
}
88068829
} else {
88078830
console.error(`Panel ${tabName}-panel not found`);
88088831
const fallbackTab = getDefaultTabName();

mcpgateway/templates/admin.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1003,7 +1003,7 @@ <h1 class="text-lg font-semibold text-gray-800 dark:text-gray-200 hidden sm:bloc
10031003
</div>
10041004

10051005
<!-- Tab Panels Container -->
1006-
<main class="flex-1 overflow-y-auto bg-gray-100 dark:bg-gray-900 p-4 lg:p-6">
1006+
<main data-scroll-container class="flex-1 overflow-y-auto bg-gray-100 dark:bg-gray-900 p-4 lg:p-6">
10071007

10081008
{% if 'logs' not in hidden_sections %}
10091009
<!-- Logs Panel -->

tests/js/admin-tabs.test.js

Lines changed: 90 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,16 @@
22
* Unit tests for tab visibility behavior in admin.js.
33
*/
44

5-
import { afterAll, beforeAll, beforeEach, describe, expect, test, vi } from "vitest";
5+
import {
6+
afterAll,
7+
afterEach,
8+
beforeAll,
9+
beforeEach,
10+
describe,
11+
expect,
12+
test,
13+
vi,
14+
} from "vitest";
615
import { cleanupAdminJs, loadAdminJs } from "./helpers/admin-env.js";
716

817
let win;
@@ -96,7 +105,8 @@ describe("showTab hidden tab fallback", () => {
96105

97106
describe("showTab idempotency", () => {
98107
test("does not re-process a tab that is already visible", () => {
99-
const { panel: overviewPanel, link: overviewLink } = createTab("overview");
108+
const { panel: overviewPanel, link: overviewLink } =
109+
createTab("overview");
100110
createTab("gateways");
101111

102112
win.showTab("overview");
@@ -274,8 +284,14 @@ describe("renderGlobalSearchResults hidden section filtering", () => {
274284
win.renderGlobalSearchResults({
275285
groups: [
276286
{ entity_type: "tools", items: [{ id: "t1", name: "Tool 1" }] },
277-
{ entity_type: "gateways", items: [{ id: "g1", name: "GW 1" }] },
278-
{ entity_type: "prompts", items: [{ id: "p1", name: "Prompt 1" }] },
287+
{
288+
entity_type: "gateways",
289+
items: [{ id: "g1", name: "GW 1" }],
290+
},
291+
{
292+
entity_type: "prompts",
293+
items: [{ id: "p1", name: "Prompt 1" }],
294+
},
279295
],
280296
});
281297

@@ -311,17 +327,18 @@ describe("runGlobalSearch visible entity filtering", () => {
311327
win.ROOT_PATH = "";
312328
win.IS_ADMIN = true;
313329
win.UI_HIDDEN_SECTIONS = ["tools", "prompts", "teams"];
314-
const fetchSpy = vi
315-
.spyOn(win, "fetchWithAuth")
316-
.mockResolvedValue({
317-
ok: true,
318-
json: async () => ({ groups: [] }),
319-
});
330+
const fetchSpy = vi.spyOn(win, "fetchWithAuth").mockResolvedValue({
331+
ok: true,
332+
json: async () => ({ groups: [] }),
333+
});
320334

321335
await win.runGlobalSearch("gateway");
322336

323337
expect(fetchSpy).toHaveBeenCalledTimes(1);
324-
const requestUrl = new URL(fetchSpy.mock.calls[0][0], "http://localhost");
338+
const requestUrl = new URL(
339+
fetchSpy.mock.calls[0][0],
340+
"http://localhost",
341+
);
325342
expect(requestUrl.searchParams.get("entity_types")).toBe(
326343
"servers,gateways,resources,agents,users",
327344
);
@@ -349,7 +366,9 @@ describe("runGlobalSearch visible entity filtering", () => {
349366
await win.runGlobalSearch("anything");
350367

351368
expect(fetchSpy).not.toHaveBeenCalled();
352-
expect(container.innerHTML).toContain("No searchable sections are visible");
369+
expect(container.innerHTML).toContain(
370+
"No searchable sections are visible",
371+
);
353372
});
354373
});
355374

@@ -421,8 +440,10 @@ describe("resolveTabForNavigation", () => {
421440

422441
describe("showTab normal navigation", () => {
423442
test("shows the requested visible tab and hides others", () => {
424-
const { panel: overviewPanel, link: overviewLink } = createTab("overview");
425-
const { panel: gatewaysPanel, link: gatewaysLink } = createTab("gateways");
443+
const { panel: overviewPanel, link: overviewLink } =
444+
createTab("overview");
445+
const { panel: gatewaysPanel, link: gatewaysLink } =
446+
createTab("gateways");
426447

427448
win.showTab("gateways");
428449

@@ -433,6 +454,61 @@ describe("showTab normal navigation", () => {
433454
});
434455
});
435456

457+
describe("showTab scroll reset (#3748)", () => {
458+
let originalRAF;
459+
460+
beforeEach(() => {
461+
// JSDOM does not provide requestAnimationFrame; install a synchronous
462+
// shim on the JSDOM window so the production code path executes.
463+
originalRAF = win.requestAnimationFrame;
464+
win.requestAnimationFrame = (cb) => {
465+
cb();
466+
return 0;
467+
};
468+
});
469+
afterEach(() => {
470+
win.requestAnimationFrame = originalRAF;
471+
});
472+
473+
test("resets scrollTop on the data-scroll-container element when switching tabs", () => {
474+
createTab("overview");
475+
createTab("gateways");
476+
477+
const scrollContainer = doc.createElement("main");
478+
scrollContainer.setAttribute("data-scroll-container", "");
479+
scrollContainer.className = "overflow-y-auto";
480+
scrollContainer.scrollTop = 500;
481+
doc.body.appendChild(scrollContainer);
482+
483+
win.showTab("gateways");
484+
485+
expect(scrollContainer.scrollTop).toBe(0);
486+
});
487+
488+
test("falls back to main.overflow-y-auto when data-scroll-container is absent", () => {
489+
createTab("overview");
490+
createTab("gateways");
491+
492+
const mainEl = doc.createElement("main");
493+
mainEl.className = "overflow-y-auto";
494+
mainEl.scrollTop = 300;
495+
doc.body.appendChild(mainEl);
496+
497+
win.showTab("gateways");
498+
499+
expect(mainEl.scrollTop).toBe(0);
500+
});
501+
502+
test("does not throw when no scroll container exists", () => {
503+
createTab("overview");
504+
createTab("gateways");
505+
506+
expect(() => {
507+
win.showTab("gateways");
508+
}).not.toThrow();
509+
});
510+
});
511+
436512
describe("getDefaultTabName edge cases", () => {
437513
test("returns gateways fallback when all tabs are hidden and no panels exist", () => {
438514
// No tabs created, nothing available

tests/js/admin-tag-extraction.test.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -369,10 +369,10 @@ function buildLoadedServersTable() {
369369
describe("showTab catalog — restore filters from URL on return", () => {
370370
const showTab = () => win.showTab;
371371

372-
// showTab wraps content-loading logic in setTimeout; use fake timers
373-
// so vi.runAllTimers() flushes the callback synchronously.
372+
// showTab wraps content-loading logic in setTimeout and requestAnimationFrame;
373+
// use fake timers so vi.runAllTimers() flushes the callbacks synchronously.
374374
beforeEach(() => {
375-
vi.useFakeTimers();
375+
vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'setInterval', 'clearInterval', 'requestAnimationFrame'] });
376376
});
377377
afterEach(() => {
378378
vi.useRealTimers();

tests/playwright/test_admin_ui.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,3 +147,69 @@ def test_sidebar_visible_after_mobile_collapse_and_resize(self, admin_page: Admi
147147
admin_page.page.set_viewport_size({"width": 1280, "height": 800})
148148
admin_page.page.wait_for_timeout(300)
149149
expect(sidebar).to_be_visible()
150+
151+
def test_scroll_reset_on_tab_navigation(self, admin_page: AdminPage):
152+
"""Test that tab navigation resets scroll position to top (#3748)."""
153+
admin_page.navigate()
154+
155+
admin_page.click_servers_tab()
156+
admin_page.page.wait_for_selector("#catalog-panel", state="visible", timeout=10000)
157+
158+
main_container = admin_page.page.locator("[data-scroll-container]")
159+
expect(main_container).to_be_attached()
160+
161+
# Inject a tall spacer so the container is guaranteed to be scrollable
162+
# regardless of how much real content is loaded.
163+
admin_page.page.evaluate(
164+
"""() => {
165+
const container = document.querySelector('[data-scroll-container]');
166+
if (container) {
167+
const spacer = document.createElement('div');
168+
spacer.id = '_scroll_test_spacer';
169+
spacer.style.height = '5000px';
170+
container.appendChild(spacer);
171+
}
172+
}"""
173+
)
174+
175+
# Scroll down and wait for the browser to apply it
176+
admin_page.page.evaluate(
177+
"""() => {
178+
const container = document.querySelector('[data-scroll-container]');
179+
if (container) { container.scrollTop = 500; }
180+
}"""
181+
)
182+
admin_page.page.wait_for_function(
183+
"document.querySelector('[data-scroll-container]').scrollTop > 0",
184+
timeout=5000,
185+
)
186+
187+
# Switch tab — showTab() should reset scroll via requestAnimationFrame
188+
admin_page.click_tools_tab()
189+
admin_page.page.wait_for_selector("#tools-panel", state="visible", timeout=10000)
190+
191+
# Wait for the RAF-driven scroll reset rather than a fixed sleep
192+
admin_page.page.wait_for_function(
193+
"document.querySelector('[data-scroll-container]').scrollTop === 0",
194+
timeout=5000,
195+
)
196+
197+
# Second round: verify consistency across another tab pair
198+
admin_page.page.evaluate(
199+
"""() => {
200+
const container = document.querySelector('[data-scroll-container]');
201+
if (container) { container.scrollTop = 300; }
202+
}"""
203+
)
204+
admin_page.page.wait_for_function(
205+
"document.querySelector('[data-scroll-container]').scrollTop > 0",
206+
timeout=5000,
207+
)
208+
209+
admin_page.click_gateways_tab()
210+
admin_page.page.wait_for_selector("#gateways-panel", state="visible", timeout=10000)
211+
212+
admin_page.page.wait_for_function(
213+
"document.querySelector('[data-scroll-container]').scrollTop === 0",
214+
timeout=5000,
215+
)

0 commit comments

Comments
 (0)