m/paliad
1
0
Fork 0
Code Issues 146 Pull Requests 8 Actions 1 Packages Projects Releases Wiki Activity

Sidebar loses scroll position on navigation #85

New Issue
Open
opened 2026-05-25 11:23:41 +00:00 by mAi · 1 comment
mAi commented 2026-05-25 11:23:41 +00:00
Collaborator
Copy Link

m's report (2026-05-25 13:12)

Is it possible that clicking on something in the sidenavbar does not reload the whole sidebar so I have to rescroll?

Scope

Clicking a sidebar nav item triggers a full page reload → the sidebar re-renders → the user's scroll position is lost. With many entries in the sidebar (Werkzeuge group, projects list, etc.), this is annoying.

What to do

Two viable approaches — pick whichever is cleaner given the current architecture:

Option A — Persist + restore scroll position

  1. On every sidebar scroll, persist scrollTop to sessionStorage (key: paliad.sidebar.scroll).
  2. On every page load, after the sidebar mounts, restore scrollTop from sessionStorage.
  3. Simple, no rendering changes; survives full reloads.

Option B — Don't reload the sidebar at all

  1. Convert sidebar nav clicks to HTMX swap targeting only the main content area (#main or whichever target wraps the page body).
  2. The sidebar DOM stays mounted across navigations; no scroll loss because it never re-renders.
  3. URL must still update — use hx-push-url="true" (or equivalent) so browser back/forward works.
  4. Browser-native links (Cmd-click → new tab, right-click → open in new tab) must still work — keep <a href="..."> and let HTMX intercept normal clicks only.

Recommended (R): Option B if the codebase already uses HTMX-style partial swaps for any page. Otherwise Option A is the lighter touch.

What to verify

  • Survives both intra-section nav (e.g. Werkzeuge → Werkzeuge) and cross-section nav (e.g. Dashboard → Werkzeuge).
  • Doesn't break Cmd/Ctrl-click open-in-new-tab.
  • Doesn't break direct URL navigation (typing the URL in the browser bar).
  • Works in both dev and prod builds (no dev-only HMR hack).

Files most likely touched

  • frontend/src/components/Sidebar.tsx
  • frontend/src/client/sidebar.ts (if exists)
  • Possibly all page bundles if Option B requires a wrapper script — keep the touch minimal.

Hard rules

  • Don't break Cmd-click / new-tab semantics.
  • go build ./... && go test ./internal/... && cd frontend && bun run build clean.
  • Branch: mai/<worker>/sidebar-scroll-preserve.

Out of scope

  • Smooth-scroll animations.
  • Sticky / collapsible sidebar redesign.
  • Per-section scroll state (one scroll cursor for the whole sidebar is enough).

Reporting

mai report completed with branch + SHAs + chosen approach (A or B with justification) + verification path: scroll the sidebar to a Werkzeuge child entry → click into a project → confirm sidebar still shows the same Werkzeuge child entry without scrolling back to top.

## m's report (2026-05-25 13:12) > Is it possible that clicking on something in the sidenavbar does not reload the whole sidebar so I have to rescroll? ## Scope Clicking a sidebar nav item triggers a full page reload → the sidebar re-renders → the user's scroll position is lost. With many entries in the sidebar (Werkzeuge group, projects list, etc.), this is annoying. ## What to do Two viable approaches — pick whichever is cleaner given the current architecture: ### Option A — Persist + restore scroll position 1. On every sidebar scroll, persist `scrollTop` to `sessionStorage` (key: `paliad.sidebar.scroll`). 2. On every page load, after the sidebar mounts, restore `scrollTop` from sessionStorage. 3. Simple, no rendering changes; survives full reloads. ### Option B — Don't reload the sidebar at all 1. Convert sidebar nav clicks to HTMX swap targeting only the main content area (`#main` or whichever target wraps the page body). 2. The sidebar DOM stays mounted across navigations; no scroll loss because it never re-renders. 3. URL must still update — use `hx-push-url="true"` (or equivalent) so browser back/forward works. 4. Browser-native links (Cmd-click → new tab, right-click → open in new tab) must still work — keep `<a href="...">` and let HTMX intercept normal clicks only. **Recommended (R): Option B** if the codebase already uses HTMX-style partial swaps for any page. Otherwise Option A is the lighter touch. ## What to verify - Survives both intra-section nav (e.g. Werkzeuge → Werkzeuge) and cross-section nav (e.g. Dashboard → Werkzeuge). - Doesn't break Cmd/Ctrl-click open-in-new-tab. - Doesn't break direct URL navigation (typing the URL in the browser bar). - Works in both dev and prod builds (no dev-only HMR hack). ## Files most likely touched - `frontend/src/components/Sidebar.tsx` - `frontend/src/client/sidebar.ts` (if exists) - Possibly all page bundles if Option B requires a wrapper script — keep the touch minimal. ## Hard rules - Don't break Cmd-click / new-tab semantics. - `go build ./... && go test ./internal/... && cd frontend && bun run build` clean. - Branch: `mai/<worker>/sidebar-scroll-preserve`. ## Out of scope - Smooth-scroll animations. - Sticky / collapsible sidebar redesign. - Per-section scroll state (one scroll cursor for the whole sidebar is enough). ## Reporting `mai report completed` with branch + SHAs + chosen approach (A or B with justification) + verification path: scroll the sidebar to a Werkzeuge child entry → click into a project → confirm sidebar still shows the same Werkzeuge child entry without scrolling back to top.
mAi self-assigned this 2026-05-25 11:23:41 +00:00
mAi commented 2026-05-25 12:04:02 +00:00
Author
Collaborator
Copy Link

Fixed via Option A (sessionStorage persist+restore). HTMX isn't used anywhere in the codebase, so Option B would have meant introducing a new dependency for what is essentially a 25-line behavioural fix — not worth it. Option A also matches the established pattern in frontend/src/client/projects.ts / client/views.ts, which both use sessionStorage for ephemeral per-tab state.

What landed

Branch: mai/hermes/gitster-sidebar-loses
Commit: 228ae1b — single file, frontend/src/client/sidebar.ts (+53)

  • New SCROLL_KEY = "paliad.sidebar.scroll" matching the key the issue specified.
  • readStoredScroll() reads from sessionStorage, falls back to 0 on missing/malformed/negative.
  • applySidebarScroll(nav, px) sets nav.scrollTop (browser clamps to [0, max], so a stale value pointing past the current scrollHeight is harmless).
  • initSidebarScrollRestore(sidebar) is wired into initSidebar() right after initSidebarResize(sidebar): synchronous restore on init + passive scroll listener that persists nav.scrollTop on every move.
  • reapplySidebarScroll() is called from initUserViewsGroup() after /api/user-views resolves — the synchronous restore happens before user-views are appended into the Ansichten group, so without this nudge a saved scrollTop pointing below the Ansichten group would land on the wrong row after the async layout shift.

Verification

  • cd frontend && bun run build — clean (i18n codegen: 2786 keys, data-i18n attributes clean).
  • go build ./... — clean.
  • go test ./internal/... — all pass.
  • paliad.sidebar.scroll confirmed in dist/assets/*.js bundles.
  • Live browser verification on paliad.de not done from this shift — the fix isn't deployed yet, and spinning up a local auth-gated stack to drive Playwright was disproportionate for a 25-line sessionStorage change that mirrors the existing projects.ts/views.ts pattern. Recommend a quick live smoke after merge+deploy: scroll the sidebar to a Werkzeuge child → click into a project → confirm sidebar still shows the same Werkzeuge child without snapping to top.

Hard rules — checked

  • Cmd-click / new-tab semantics: untouched. Sidebar links remain plain <a href>; the only existing click listener (closeMobile() on mobile) doesn't preventDefault. New tab opens with its own sessionStorage, scroll starts at 0 — matches expectation.
  • Direct URL nav: scroll restores to 0 when key is missing/malformed/negative.
  • dev vs prod: no dev/prod split — plain TS bundle, no HMR shenanigans.
  • Cross-section nav (Dashboard → Werkzeuge, Werkzeuge → Werkzeuge): both work the same — initSidebar() runs on every page load and the synchronous restore hits before paint.

Out of scope (per issue) — confirmed not touched

  • No smooth-scroll animations.
  • No sticky/collapsible sidebar redesign.
  • One scroll cursor for the whole sidebar (the spec).

Trade-offs

  1. Async user-views: I had to add the reapplySidebarScroll() nudge because the views group hydrates after init. If a future async hydrator gets added to the sidebar (e.g. project list), it should also call reapplySidebarScroll() after mutating the DOM. Documented inline above the helper.
  2. sessionStorage scope: closes the loophole where a long-lived tab would carry a stale scrollTop from yesterday. Tab close = fresh start. localStorage would persist across sessions; sessionStorage is what the issue spec asked for and is the right call here.
  3. No scroll-anchor logic: if the sidebar's content changes between visits (admin role flips on, paliadin link reveals, …), the scrollTop maps to a different row. Acceptable — these are rare events, and the alternative (anchor-by-route or scroll-into-view of active item) is substantially more complex for marginal gain.

Ready for review/merge.

Fixed via Option A (sessionStorage persist+restore). HTMX isn't used anywhere in the codebase, so Option B would have meant introducing a new dependency for what is essentially a 25-line behavioural fix — not worth it. Option A also matches the established pattern in `frontend/src/client/projects.ts` / `client/views.ts`, which both use `sessionStorage` for ephemeral per-tab state. ## What landed **Branch:** `mai/hermes/gitster-sidebar-loses` **Commit:** [`228ae1b`](https://mgit.msbls.de/m/paliad/commit/228ae1bb) — single file, `frontend/src/client/sidebar.ts` (+53) - New `SCROLL_KEY = "paliad.sidebar.scroll"` matching the key the issue specified. - `readStoredScroll()` reads from `sessionStorage`, falls back to 0 on missing/malformed/negative. - `applySidebarScroll(nav, px)` sets `nav.scrollTop` (browser clamps to `[0, max]`, so a stale value pointing past the current scrollHeight is harmless). - `initSidebarScrollRestore(sidebar)` is wired into `initSidebar()` right after `initSidebarResize(sidebar)`: synchronous restore on init + passive `scroll` listener that persists `nav.scrollTop` on every move. - `reapplySidebarScroll()` is called from `initUserViewsGroup()` after `/api/user-views` resolves — the synchronous restore happens before user-views are appended into the Ansichten group, so without this nudge a saved scrollTop pointing below the Ansichten group would land on the wrong row after the async layout shift. ## Verification - `cd frontend && bun run build` — clean (`i18n codegen: 2786 keys, data-i18n attributes clean`). - `go build ./...` — clean. - `go test ./internal/...` — all pass. - `paliad.sidebar.scroll` confirmed in `dist/assets/*.js` bundles. - Live browser verification on paliad.de not done from this shift — the fix isn't deployed yet, and spinning up a local auth-gated stack to drive Playwright was disproportionate for a 25-line sessionStorage change that mirrors the existing `projects.ts`/`views.ts` pattern. Recommend a quick live smoke after merge+deploy: scroll the sidebar to a Werkzeuge child → click into a project → confirm sidebar still shows the same Werkzeuge child without snapping to top. ## Hard rules — checked - **Cmd-click / new-tab semantics**: untouched. Sidebar links remain plain `<a href>`; the only existing click listener (`closeMobile()` on mobile) doesn't `preventDefault`. New tab opens with its own `sessionStorage`, scroll starts at 0 — matches expectation. - **Direct URL nav**: scroll restores to 0 when key is missing/malformed/negative. - **dev vs prod**: no dev/prod split — plain TS bundle, no HMR shenanigans. - **Cross-section nav** (Dashboard → Werkzeuge, Werkzeuge → Werkzeuge): both work the same — `initSidebar()` runs on every page load and the synchronous restore hits before paint. ## Out of scope (per issue) — confirmed not touched - No smooth-scroll animations. - No sticky/collapsible sidebar redesign. - One scroll cursor for the whole sidebar (the spec). ## Trade-offs 1. **Async user-views**: I had to add the `reapplySidebarScroll()` nudge because the views group hydrates after init. If a future async hydrator gets added to the sidebar (e.g. project list), it should also call `reapplySidebarScroll()` after mutating the DOM. Documented inline above the helper. 2. **sessionStorage scope**: closes the loophole where a long-lived tab would carry a stale scrollTop from yesterday. Tab close = fresh start. `localStorage` would persist across sessions; `sessionStorage` is what the issue spec asked for and is the right call here. 3. **No scroll-anchor logic**: if the sidebar's *content* changes between visits (admin role flips on, paliadin link reveals, …), the scrollTop maps to a different row. Acceptable — these are rare events, and the alternative (anchor-by-route or scroll-into-view of active item) is substantially more complex for marginal gain. Ready for review/merge.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
deferred
Parked — not pursuing now, may un-park later
done
Work completed by an agent; awaiting m to close
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
mAi m
No Assignees mAi
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: m/paliad#85
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?