✨ Preserve sidebar scroll positions across tab switches (#11694)

* 🐛 Preserve layers panel scroll position across tab switches

Fixes #7440. Switching between the Layers, Assets and Tokens tabs in
the workspace left sidebar unmounts the active panel component, causing
its scroll position to reset to the top on re-entry.

Add a module-level `scroll-positions` atom keyed by page-id. The
layers scroll handler now also saves the current scrollTop value into
the atom; a `mf/with-effect` on the page-id dep restores it whenever
the `layers-toolbox*` component mounts or the page changes.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* 🐛 Preserve sidebar scroll positions across tab switches

Replace the Layers-only global atom with a scroll store held in a
use-var in left-sidebar*, shared by the Layers, Assets and Tokens
panels through a new sidebar.scroll helper. Positions are keyed per
panel and page (or token set) and restore waits for list content to
settle, so deep positions in lazily rendered lists survive tab
switches.

Closes #7440.

AI-assisted-by: muse-spark-1.3-contributor

* 🐛 Add e2e coverage for sidebar scroll preservation

Port the regression tests from closed PR #7544 for issue #7440,
adapted to the current ref-based implementation and fixtures:
async restore needs polled assertions, and setup uses the shared
tokens helpers. Also add data-scroll-container hooks to the Assets
and Tokens scroll containers so the specs can locate them.

AI-assisted-by: muse-spark-1.3-contributor

* 📎 Fix rebase issue

---------

Co-authored-by: Sumit Ridhal <sridhal@redhat.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>

frontend/playwright/ui/specs/sidebar-scroll.spec.js

@@ -0,0 +1,129 @@
+import { test, expect } from "@playwright/test";
+import { BaseWebSocketPage } from "../pages/BaseWebSocketPage";
+import { WasmWorkspacePage } from "../pages/WasmWorkspacePage";
+import { setupTokensFileRender, unfoldTokenType } from "./tokens/helpers";
+
+// Regression tests for issue #7440: scroll position in the left sidebar
+// tabs (Layers / Assets / Tokens) must survive tab switches.
+// Ported from closed PR #7544 and adapted to the current implementation
+// (scroll store held in the sidebar + async restore on mount) and to the
+// current fixtures and page objects.
+
+// We allow a small tolerance because content can shift slightly on remount.
+const POSITION_TOLERANCE = 20; // pixels
+
+test.beforeEach(async ({ page }) => {
+  await WasmWorkspacePage.init(page);
+  await BaseWebSocketPage.mockRPC(page, "get-teams", "get-teams-tokens.json");
+});
+
+async function visibleScrollContainers(page) {
+  const containers = page.locator(
+    '[data-testid="left-sidebar"] [data-scroll-container="true"]',
+  );
+  const count = await containers.count();
+  const visible = [];
+  for (let i = 0; i < count; i++) {
+    const el = containers.nth(i);
+    // eslint-disable-next-line no-await-in-loop
+    if (await el.evaluate((node) => !!node && node.offsetParent !== null)) {
+      visible.push(el);
+    }
+  }
+  return visible;
+}
+
+async function scrollableContainers(page) {
+  const visible = await visibleScrollContainers(page);
+  const scrollable = [];
+  for (const el of visible) {
+    // eslint-disable-next-line no-await-in-loop
+    if (
+      await el.evaluate((node) => node.scrollHeight > node.clientHeight + 100)
+    ) {
+      scrollable.push(el);
+    }
+  }
+  return scrollable;
+}
+
+async function scrollToBottom(locator) {
+  await locator.evaluate((el) => {
+    el.scrollTop = el.scrollHeight;
+  });
+  // Programmatic scrolls dispatch scroll events asynchronously; wait for
+  // the save handler before switching tabs.
+  await locator.page().waitForTimeout(150);
+}
+
+async function expectScrollRestored(locator, expected) {
+  // Restore runs in a post-mount effect with frame retries, so poll
+  // instead of asserting immediately after the tab click.
+  await expect
+    .poll(async () => locator.evaluate((el) => el.scrollTop), {
+      timeout: 10000,
+    })
+    .toBeGreaterThanOrEqual(expected - POSITION_TOLERANCE);
+}
+
+test("Sidebar scroll position preserved when switching tabs", async ({
+  page,
+}) => {
+  const { tokensSidebar } = await setupTokensFileRender(page);
+  await unfoldTokenType(tokensSidebar, "color");
+
+  const containers = await scrollableContainers(page);
+  expect(
+    containers.length,
+    "Tokens tab should have a scrollable container",
+  ).toBeGreaterThan(0);
+  const scrollEl = containers[0];
+
+  await scrollToBottom(scrollEl);
+  const initialPos = await scrollEl.evaluate((el) => el.scrollTop);
+  expect(initialPos).toBeGreaterThan(0);
+
+  await page.getByRole("tab", { name: "Assets" }).click();
+  await page.getByRole("tab", { name: "Tokens" }).click();
+
+  const restored = (await scrollableContainers(page))[0];
+  await expectScrollRestored(restored, initialPos);
+});
+
+test("Sidebar maintains independent scroll positions per tab", async ({
+  page,
+}) => {
+  const { tokensSidebar } = await setupTokensFileRender(page);
+  await unfoldTokenType(tokensSidebar, "color");
+
+  // Scroll Tokens to the bottom.
+  const tokensContainers = await scrollableContainers(page);
+  expect(
+    tokensContainers.length,
+    "Tokens tab should have a scrollable container",
+  ).toBeGreaterThan(0);
+  await scrollToBottom(tokensContainers[0]);
+  const tokensPos = await tokensContainers[0].evaluate((el) => el.scrollTop);
+  expect(tokensPos).toBeGreaterThan(0);
+
+  // Switch to Layers: fresh panel, must start at top (not contaminated
+  // by the Tokens position saved under a different key).
+  await page.getByRole("tab", { name: "Layers" }).click();
+  const layersVisible = await visibleScrollContainers(page);
+  expect(layersVisible.length).toBeGreaterThan(0);
+  const layersPos = await layersVisible[0].evaluate((el) => el.scrollTop);
+  expect(layersPos).toBe(0);
+
+  // Back to Tokens: its own position must be restored, not Layers'.
+
+  // Back to Tokens: its own position must be restored, not Layers'.
+  await page.getByRole("tab", { name: "Tokens" }).click();
+  const tokensRestored = (await scrollableContainers(page))[0];
+  await expectScrollRestored(tokensRestored, tokensPos);
+
+  // And back to Layers: still untouched at top.
+  await page.getByRole("tab", { name: "Layers" }).click();
+  const layersAgain = await visibleScrollContainers(page);
+  const layersAgainPos = await layersAgain[0].evaluate((el) => el.scrollTop);
+  expect(layersAgainPos).toBe(0);
+});

frontend/src/app/main/ui/workspace/sidebar.cljs

@@ -79,7 +79,7 @@
 (mf/defc layers-content*
   {::mf/private true
    ::mf/memo true}
-  [{:keys [width layout]}]
+  [{:keys [width layout scroll-store]}]
   (let [{on-pointer-down :on-pointer-down
          on-lost-pointer-capture :on-lost-pointer-capture
          on-pointer-move :on-pointer-move
@@ -112,7 +112,8 @@
 
         [:div {:class (stl/css :layers-tab-resize-handle)}]])
 
-     [:> layers-toolbox* {:size-parent width}]]))
+     [:> layers-toolbox* {:size-parent width
+                          :scroll-store scroll-store}]]))
 
 
 (mf/defc left-sidebar*
@@ -121,6 +122,7 @@
   (let [options-mode   (mf/deref refs/options-mode-global)
         project        (mf/deref refs/project)
         file-id        (get file :id)
+        scroll-store*  (mf/use-var {})
 
         design-tokens? (features/use-feature "design-tokens/v1")
         mode-inspect?  (= options-mode :inspect)
@@ -213,18 +215,21 @@
           (case section
             :assets
             [:> assets-toolbox* {:size (- width  58)
-                                 :file-id file-id}]
+                                 :file-id file-id
+                                 :scroll-store scroll-store*}]
 
             :tokens
             [:> tokens-sidebar-tab*
              {:tokens-lib tokens-lib
               :tokens-status tokens-status
               :active-tokens active-tokens
-              :resolved-active-tokens resolved-active-tokens}]
+              :resolved-active-tokens resolved-active-tokens
+              :scroll-store scroll-store*}]
 
             :layers
             [:> layers-content* {:layout layout
-                                 :width width}])]])]]))
+                                 :width width
+                                 :scroll-store scroll-store*}])]])]]))
 
 ;; --- Right Sidebar (Component)
 

frontend/src/app/main/ui/workspace/sidebar/assets.cljs

@@ -22,6 +22,7 @@
    [app.main.ui.icons :as deprecated-icon]
    [app.main.ui.workspace.sidebar.assets.common :as cmm]
    [app.main.ui.workspace.sidebar.assets.file-library :refer [file-library*]]
+   [app.main.ui.workspace.sidebar.scroll :as sc]
    [app.util.dom :as dom]
    [app.util.i18n :as i18n :refer [tr]]
    [cuerdas.core :as str]
@@ -89,8 +90,9 @@
 
 (mf/defc assets-toolbox*
   {::mf/wrap [mf/memo]}
-  [{:keys [size file-id]}]
+  [{:keys [size file-id scroll-store]}]
   (let [read-only?     (mf/use-ctx ctx/workspace-read-only?)
+        assets-ref     (mf/use-ref nil)
         filters*       (mf/use-state
                         (fn []
                           (-> (or (get @session-filters* file-id)
@@ -150,6 +152,12 @@
          (fn []
            (modal/show! :libraries-dialog {:file-id file-id})))
 
+        on-scroll-save
+        (mf/use-fn
+         (mf/deps file-id)
+         (fn [event]
+           (sc/save-scroll! scroll-store [:assets file-id] event)))
+
         on-open-menu
         (mf/use-fn  #(swap! filters* update :open-menu not))
 
@@ -184,7 +192,12 @@
     (mf/with-effect [file-id term section]
       (swap! session-filters* assoc file-id {:term term :section section}))
 
-    [:article  {:class (stl/css :assets-bar)}
+    (sc/use-restore-scroll scroll-store :assets file-id assets-ref)
+
+    [:article  {:class (stl/css :assets-bar)
+                :data-scroll-container true
+                :on-scroll on-scroll-save
+                :ref assets-ref}
      [:div {:class (stl/css :assets-header)}
       (when-not ^boolean read-only?
         (if (and (= num-libs 1) (empty? components) (not shared?))

frontend/src/app/main/ui/workspace/sidebar/layers.cljs

@@ -26,6 +26,7 @@
    [app.main.ui.hooks :as hooks]
    [app.main.ui.notifications.badge :refer [badge-notification]]
    [app.main.ui.workspace.sidebar.layer-item :refer [layer-item*]]
+   [app.main.ui.workspace.sidebar.scroll :as sc]
    [app.util.dom :as dom]
    [app.util.globals :as globals]
    [app.util.i18n :as i18n :refer [tr]]
@@ -799,7 +800,7 @@
 
 (mf/defc layers-toolbox*
   {::mf/wrap [mf/memo]}
-  [{:keys [size-parent]}]
+  [{:keys [size-parent scroll-store]}]
   (let [page           (mf/deref refs/workspace-page)
         page-id        (get page :id)
 
@@ -811,6 +812,8 @@
 
         observer-var   (mf/use-var nil)
         lazy-load-ref  (mf/use-ref nil)
+        tree-ref       (mf/use-ref nil)
+        search-ref     (mf/use-ref nil)
 
         [filtered-objects show-more filter-component]
         (use-search page objects)
@@ -834,10 +837,33 @@
               (do (.disconnect ^js @observer-var)
                   (reset! observer-var nil)))))
 
+        ;; The search-results container reuses the lazy-load observer root
+        ;; and additionally tracks its node for scroll restore.
+        on-render-search-container
+        (fn [element]
+          (mf/set-ref-val! search-ref element)
+          (on-render-container element))
+
+        on-scroll-with-save
+        (mf/use-fn
+         (mf/deps page-id)
+         (fn [event]
+           (sc/save-scroll! scroll-store [:layers page-id] event)
+           (on-scroll event)))
+
+        on-search-scroll
+        (mf/use-fn
+         (mf/deps page-id)
+         (fn [event]
+           (sc/save-scroll! scroll-store [:layers-search page-id] event)))
+
         toogle-focus-mode
         (mf/use-fn
          #(st/emit! (dw/toggle-focus-mode)))]
 
+    (sc/use-restore-scroll scroll-store :layers page-id tree-ref)
+    (sc/use-restore-scroll scroll-store :layers-search page-id search-ref)
+
     [:div {:id "layers"
            :class (stl/css :layers)
            :data-testid "layer-tree"}
@@ -863,13 +889,15 @@
        [:*
         [:div {:class (stl/css :tool-window-content)
                :data-scroll-container true
-               :ref on-render-container}
+               :on-scroll on-search-scroll
+               :ref on-render-search-container}
          [:> filters-tree* {:objects filtered-objects
                             :key (dm/str page-id)
                             :parent-size size-parent}]
          [:div {:ref lazy-load-ref}]]
 
-        [:div {:on-scroll on-scroll
+        [:div {:on-scroll on-scroll-with-save
+               :ref tree-ref
                :class (stl/css :tool-window-content)
                :data-scroll-container true
                :style {:display (when (some? filtered-objects) "none")}}
@@ -878,7 +906,8 @@
                                    :is-filtered true
                                    :parent-size size-parent}]]]
 
-       [:div {:on-scroll on-scroll
+       [:div {:on-scroll on-scroll-with-save
+              :ref tree-ref
               :class (stl/css :tool-window-content)
               :data-scroll-container true
               :style {:display (when (some? filtered-objects) "none")}}

frontend/src/app/main/ui/workspace/sidebar/scroll.cljs

@@ -0,0 +1,61 @@
+;; This Source Code Form is subject to the terms of the Mozilla Public
+;; License, v. 2.0. If a copy of the MPL was not distributed with this
+;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;;
+;; Copyright (c) KALEIDOS SUBSIDIARY SL
+
+(ns app.main.ui.workspace.sidebar.scroll
+  "Scroll position save/restore for sidebar panels.
+
+  Panels unmount on every tab switch, so positions that must survive it
+  cannot live in component state. The store is created once in
+  `left-sidebar*` (which stays mounted while tabs swap, unlike
+  `layers-content*` and the toolboxes) with `mf/use-var` and threaded to
+  the panels as a prop. Shape: {[panel id] scroll-top}."
+  (:require
+   [app.util.dom :as dom]
+   [app.util.timers :as tm]
+   [rumext.v2 :as mf]))
+
+(def ^:private max-restore-attempts
+  10)
+
+(defn save-scroll!
+  "Saves the scrollTop of the scroll event target into the store under k.
+  Plain swap!, never triggers renders. Missing target is a no-op."
+  [store* k event]
+  (when-let [target (dom/get-target event)]
+    (swap! store* assoc k (.-scrollTop target))))
+
+(defn needs-restore-retry?
+  "Pure decision for the restore loop: keep retrying while the content
+  height is still changing (lazily rendered lists) and attempts remain."
+  [prev-height cur-height attempts]
+  (and (not= prev-height cur-height)
+       (< attempts max-restore-attempts)))
+
+(defn restore-scroll!
+  [node saved]
+  (set! (.-scrollTop node) saved))
+
+(defn- restore-loop!
+  [node saved prev-height attempts raf*]
+  (restore-scroll! node saved)
+  (let [height (.-scrollHeight node)]
+    (when (needs-restore-retry? prev-height height attempts)
+      (vreset! raf* (tm/raf #(restore-loop! node saved height (inc attempts) raf*))))))
+
+(defn use-restore-scroll
+  "Restores the scrollTop saved under [panel id] on mount and whenever
+  panel or id change. Retries while the content height keeps changing so
+  deep positions in lazily rendered lists are not clamped to the first
+  chunk. Missing key or node is a silent no-op."
+  [store* panel id node-ref]
+  (mf/with-effect [panel id]
+    (let [raf* (volatile! nil)]
+      (when-let [node (mf/ref-val node-ref)]
+        (when-let [saved (get @store* [panel id])]
+          (restore-loop! node saved -1 0 raf*)))
+      (fn []
+        (when-some [raf @raf*]
+          (tm/cancel-af! raf))))))

frontend/src/app/main/ui/workspace/tokens/sidebar.cljs

@@ -21,6 +21,7 @@
    [app.main.ui.ds.foundations.assets.icon :as i]
    [app.main.ui.hooks :as h]
    [app.main.ui.hooks.resize :refer [use-resize-hook]]
+   [app.main.ui.workspace.sidebar.scroll :as sc]
    [app.main.ui.workspace.tokens.management :refer [tokens-section*]]
    [app.main.ui.workspace.tokens.sets :as tsets]
    [app.main.ui.workspace.tokens.sets.context-menu :refer [token-set-context-menu*]]
@@ -150,13 +151,12 @@
                          :on-click open-settings-modal}])]))
 
 (mf/defc tokens-sidebar-tab*
-  [{:keys [] :as props}]
+  [{:keys [scroll-store] :as props}]
   (let [{on-pointer-down-pages :on-pointer-down
          on-lost-pointer-capture-pages :on-lost-pointer-capture
          on-pointer-move-pages :on-pointer-move
          size-pages-opened :size}
         (use-resize-hook :tokens 200 38 "0.6" :y false nil)
-
         current-file-data
         (mf/deref refs/workspace-data)
 
@@ -166,7 +166,21 @@
         can-edit-tokens?
         (mf/with-memo [can-edit-file? current-file-data]
           (and can-edit-file?
-               (cfo/editable-tokens? current-file-data)))]
+               (cfo/editable-tokens? current-file-data)))
+
+        set-id
+        (mf/deref refs/selected-token-set-id)
+
+        tokens-ref
+        (mf/use-ref nil)
+
+        on-scroll-save
+        (mf/use-fn
+         (mf/deps set-id)
+         (fn [event]
+           (sc/save-scroll! scroll-store [:tokens set-id] event)))]
+
+    (sc/use-restore-scroll scroll-store :tokens set-id tokens-ref)
 
     [:> (mf/provider ctx/can-edit-tokens?) {:value can-edit-tokens?}
      [:div {:class (stl/css :sidebar-wrapper)}
@@ -174,7 +188,10 @@
        {:resize-height size-pages-opened
         :current-file-data current-file-data}]
       [:article {:class (stl/css :tokens-section-wrapper)
-                 :data-testid "tokens-sidebar"}
+                 :data-testid "tokens-sidebar"
+                 :data-scroll-container true
+                 :on-scroll on-scroll-save
+                 :ref tokens-ref}
        [:div {:class (stl/css :resize-area-horiz)
               :on-pointer-down on-pointer-down-pages
               :on-lost-pointer-capture on-lost-pointer-capture-pages

frontend/test/frontend_tests/runner.cljs

@@ -104,6 +104,7 @@
    [frontend-tests.ui.routes-test]
    [frontend-tests.ui.settings-password-schema-test]
    [frontend-tests.ui.settings-shortcuts-test]
+   [frontend-tests.ui.sidebar-scroll-test]
    [frontend-tests.util-clipboard-test]
    [frontend-tests.util-object-test]
    [frontend-tests.util-queue-test]
@@ -228,6 +229,7 @@
    'frontend-tests.text-editor-paste-guard-test
    'frontend-tests.ui.settings-password-schema-test
    'frontend-tests.ui.settings-shortcuts-test
+   'frontend-tests.ui.sidebar-scroll-test
    'frontend-tests.util-clipboard-test
    'frontend-tests.util-object-test
    'frontend-tests.util-queue-test

frontend/test/frontend_tests/ui/sidebar_scroll_test.cljs

@@ -0,0 +1,66 @@
+;; This Source Code Form is subject to the terms of the Mozilla Public
+;; License, v. 2.0. If a copy of the MPL was not distributed with this
+;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;;
+;; Copyright (c) KALEIDOS SUBSIDIARY SL
+
+(ns frontend-tests.ui.sidebar-scroll-test
+  (:require
+   [app.main.ui.workspace.sidebar.scroll :as sc]
+   [app.util.timers :as tm]
+   [cljs.test :as t :include-macros true]))
+
+(def ^:private restore-loop! @#'sc/restore-loop!)
+
+(defn- scroll-event
+  [scroll-top]
+  #js {:target #js {:scrollTop scroll-top}})
+
+(t/deftest save-scroll-stores-position-per-key
+  (let [store (atom {})]
+    (sc/save-scroll! store [:layers :page-1] (scroll-event 250))
+    (sc/save-scroll! store [:assets :file-1] (scroll-event 80))
+    (t/is (= {[:layers :page-1] 250
+              [:assets :file-1] 80}
+             @store))))
+
+(t/deftest save-scroll-ignores-events-without-target
+  (let [store (atom {})]
+    (sc/save-scroll! store [:layers :page-1] nil)
+    (sc/save-scroll! store [:layers :page-1] #js {})
+    (t/is (= {} @store))))
+
+(t/deftest restore-scroll-writes-saved-position
+  (let [node #js {:scrollTop 0}]
+    (sc/restore-scroll! node 320)
+    (t/is (= 320 (.-scrollTop node)))))
+
+(t/deftest needs-restore-retry
+  (t/is (true? (sc/needs-restore-retry? -1 300 0)))
+  (t/is (true? (sc/needs-restore-retry? 100 300 9)))
+  (t/is (false? (sc/needs-restore-retry? 300 300 0)))
+  (t/is (false? (sc/needs-restore-retry? 100 300 10))))
+
+(t/deftest restore-loop-writes-once-when-content-settled
+  (let [node      #js {:scrollTop 0 :scrollHeight 300}
+        scheduled (atom [])
+        raf*      (volatile! nil)]
+    (with-redefs [tm/raf (fn [f] (swap! scheduled conj f) 1)]
+      (restore-loop! node 250 300 0 raf*)
+      (t/is (= 250 (.-scrollTop node)))
+      (t/is (empty? @scheduled) "no follow-up when height is stable")
+      (t/is (nil? @raf*)))))
+
+(t/deftest restore-loop-retries-while-content-grows
+  (let [node      #js {:scrollTop 0 :scrollHeight 300}
+        scheduled (atom [])
+        raf*      (volatile! nil)]
+    (with-redefs [tm/raf (fn [f] (swap! scheduled conj f) 1)]
+      (restore-loop! node 250 -1 0 raf*)
+      (t/is (= 250 (.-scrollTop node)))
+      (t/is (= 1 (count @scheduled)) "follow-up scheduled while growing")
+      (t/is (= 1 @raf*) "frame id tracked for cancellation")
+      ;; Next frame: content settled at the same height, retry stops.
+      ((first @scheduled))
+      (t/is (= 250 (.-scrollTop node)))
+      (t/is (= 1 (count @scheduled)) "no further frames once settled"))))