Skip to content

fix: preserve scroll when clicked on a sidebar link #537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kaulith wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

fix: preserve scroll when clicked on a sidebar link #537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
febe04a
fix: preserve scroll when clicked on a sidebar link
kaulith Feb 17, 2026
bb125ea
fix: enable sidebar scroll-to-selected with data-wiki-item attribute …
kaulith Feb 17, 2026
d8449a1
fix: resolve sidebar scroll positioning and navigation issue
kaulith Feb 18, 2026
0317701
fix: preserve browser state, optimize scroll viewport check, and rest…
kaulith Feb 18, 2026
676c13b
fix: clear scroll timers on component destroy
kaulith Feb 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 72 additions & 1 deletion frontend/src/components/NestedDraggable.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
class="flex items-center justify-between pr-2 py-1.5 hover:bg-surface-gray-2 group border-b border-outline-gray-1"
:class="getRowClasses(element)"
:style="{ paddingLeft: `${level * 12 + 8}px` }"
:data-wiki-item="element.document_name || element.doc_key"
@click="handleRowClick(element)"
>
<div class="flex items-center gap-1.5 flex-1 min-w-0">
Expand Down Expand Up @@ -120,7 +121,7 @@
</template>

<script setup>
import { ref, watch, computed } from 'vue';
import { ref, watch, computed, nextTick } from 'vue';
import { useRouter } from 'vue-router';
import { useStorage } from '@vueuse/core';
import { Dropdown, Badge, Button, toast } from 'frappe-ui';
Expand Down Expand Up @@ -187,6 +188,76 @@ function toggleExpanded(name) {
expandedNodes.value[name] = !expandedNodes.value[name];
}

function isElementInViewport(element, container) {
if (!element || !container) return true;
const rect = element.getBoundingClientRect();
const containerRect = container.getBoundingClientRect();

return (
rect.top >= containerRect.top &&
rect.bottom <= containerRect.bottom
);
}

function findParentGroups(items, targetKey, parents = []) {
for (const item of items) {
if (item.doc_key === targetKey || item.document_name === targetKey) {
return parents;
}
if (item.is_group && item.children) {
const found = findParentGroups(item.children, targetKey, [...parents, item.doc_key]);
if (found) return found;
}
}
return null;
}

function expandParentsAndScroll(selectedId) {
if (!selectedId) return;

const parentKeys = findParentGroups(localItems.value, selectedId);

if (parentKeys && parentKeys.length > 0) {
parentKeys.forEach(key => {
expandedNodes.value[key] = true;
});
}

nextTick(() => {
nextTick(() => {
try {
const escapedId = CSS.escape(selectedId);
const selectedElement = document.querySelector(`[data-wiki-item="${escapedId}"]`);

if (!selectedElement) {
return;
}

const scrollContainer = selectedElement.closest('[data-wiki-scroll-container]');

if (scrollContainer && !isElementInViewport(selectedElement, scrollContainer)) {
selectedElement.scrollIntoView({ behavior: 'smooth', block: 'nearest' });
}
} catch (error) {
console.error('Error scrolling to selected item:', error);
}
});
});
}

watch(
() => [props.selectedPageId, props.selectedDraftKey],
() => {
if (props.level !== 0) return;

const selectedId = props.selectedPageId || props.selectedDraftKey;
if (!selectedId) return;

expandParentsAndScroll(selectedId);
},
{ flush: 'post', immediate: true }
);

function handleRowClick(element) {
if (element._changeType === 'deleted') {
return;
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/pages/SpaceDetails.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
<p class="text-sm text-ink-gray-5 mt-0.5">{{ space.doc?.route }}</p>
</div>

<div v-if="space.doc && mergedTreeData" class="flex-1 overflow-auto p-2">
<div v-if="space.doc && mergedTreeData" class="flex-1 overflow-auto p-2" data-wiki-scroll-container>
<WikiDocumentList
:tree-data="mergedTreeData"
:space-id="spaceId"
Expand Down
4 changes: 2 additions & 2 deletions wiki/templates/wiki/document.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ <h1 id="wiki-page-title" class="text-3xl font-semibold text-[var(--ink-gray-9)]"
<div class="flex flex-col sm:flex-row sm:items-stretch gap-4 {{ 'sm:justify-between' if prev_doc and next_doc else ('sm:justify-start' if prev_doc else 'sm:justify-end') }}">
{% if prev_doc %}
<a href="/{{ prev_doc.route }}"
@click.prevent="$store.navigation.navigateTo('{{ prev_doc.route }}')"
@click="$store.navigation.navigateTo('{{ prev_doc.route }}', true, $event)"
@mouseenter="$store.navigation.prefetch('{{ prev_doc.route }}')"
class="group sm:flex-1 flex items-center gap-3 p-3 rounded-lg border border-[var(--outline-gray-1)] hover:border-[var(--outline-gray-2)] hover:bg-[var(--surface-gray-1)] transition-colors duration-200 no-underline sm:max-w-[50%] overflow-hidden">
{{ render_icon("chevron-left", "w-5 h-5 text-[var(--ink-gray-4)] group-hover:text-[var(--ink-gray-6)] transition-colors duration-200 shrink-0") }}
Expand All @@ -81,7 +81,7 @@ <h1 id="wiki-page-title" class="text-3xl font-semibold text-[var(--ink-gray-9)]"

{% if next_doc %}
<a href="/{{ next_doc.route }}"
@click.prevent="$store.navigation.navigateTo('{{ next_doc.route }}')"
@click="$store.navigation.navigateTo('{{ next_doc.route }}', true, $event)"
@mouseenter="$store.navigation.prefetch('{{ next_doc.route }}')"
class="group sm:flex-1 flex items-center gap-3 p-3 rounded-lg border border-[var(--outline-gray-1)] hover:border-[var(--outline-gray-2)] hover:bg-[var(--surface-gray-1)] transition-colors duration-200 no-underline sm:max-w-[50%] overflow-hidden {{ 'sm:ml-auto' if not prev_doc else '' }}">
<div class="flex flex-col gap-1 items-end min-w-0 ml-auto">
Expand Down
167 changes: 143 additions & 24 deletions wiki/templates/wiki/includes/sidebar.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,17 @@
}
},

async navigateTo(route, pushState = true) {
async navigateTo(route, pushState = true, event = null) {
if (event && (event.ctrlKey || event.metaKey || event.shiftKey || event.altKey)) {
return;
}

if (event && event.preventDefault) {
event.preventDefault();
}

if (route === this.currentRoute) return;

// Check prefetch cache first
if (this.prefetchCache[route]) {
this.updateContent(this.prefetchCache[route]);
this.currentRoute = route;
Expand Down Expand Up @@ -145,7 +152,6 @@
throw new Error('Invalid response');
}
} catch (error) {
// Fallback to full page load
window.location.href = '/' + route;
} finally {
this.loading = false;
Expand Down Expand Up @@ -203,8 +209,11 @@
hljs.highlightAll();
}

// Scroll to top
window.scrollTo(0, 0);

window.dispatchEvent(new CustomEvent('wiki-route-changed', {
detail: { route: data.route || this.currentRoute }
}));
},

updateNavButtons(prevDoc, nextDoc) {
Expand All @@ -220,7 +229,7 @@
if (prevDoc) {
html += `
<a href="/${prevDoc.route}"
@click.prevent="$store.navigation.navigateTo('${prevDoc.route}')"
@click="$store.navigation.navigateTo('${prevDoc.route}', true, $event)"
@mouseenter="$store.navigation.prefetch('${prevDoc.route}')"
class="group sm:flex-1 flex items-center gap-3 p-3 rounded-lg border border-[var(--outline-gray-1)] hover:border-[var(--outline-gray-2)] hover:bg-[var(--surface-gray-1)] transition-colors duration-200 no-underline sm:max-w-[50%] overflow-hidden">
<svg class="w-5 h-5 text-[var(--ink-gray-4)] group-hover:text-[var(--ink-gray-6)] transition-colors duration-200 shrink-0" fill="none" stroke="currentColor" viewBox="0 0 24 24">
Expand All @@ -237,7 +246,7 @@
const mlClass = !prevDoc ? 'sm:ml-auto' : '';
html += `
<a href="/${nextDoc.route}"
@click.prevent="$store.navigation.navigateTo('${nextDoc.route}')"
@click="$store.navigation.navigateTo('${nextDoc.route}', true, $event)"
@mouseenter="$store.navigation.prefetch('${nextDoc.route}')"
class="group sm:flex-1 flex items-center gap-3 p-3 rounded-lg border border-[var(--outline-gray-1)] hover:border-[var(--outline-gray-2)] hover:bg-[var(--surface-gray-1)] transition-colors duration-200 no-underline sm:max-w-[50%] overflow-hidden ${mlClass}">
<div class="flex flex-col gap-1 items-end min-w-0 ml-auto">
Expand All @@ -263,18 +272,46 @@
// Store expanded states with persistence
expandedStates: Alpine.$persist({}),
currentRoute: '{{ doc.route if doc else "" }}',

previousScrollRestoration: null,
scrollTimeoutHandle: null,
scrollRafHandle: null,

get isCollapsed() {
return this.$store.sidebar.isCollapsed;
},

init() {
// Auto-expand parents of current page
this.expandCurrentPageParents();

// Debug: log all route elements
this.$nextTick(() => {
const allRouteElements = document.querySelectorAll('[data-route]');
if ('scrollRestoration' in history) {
this.previousScrollRestoration = history.scrollRestoration;
history.scrollRestoration = 'manual';
}

const initialScrollNeeded = this.currentRoute;
const handleInitialScroll = () => {
if (initialScrollNeeded) {
this.expandCurrentPageParents();
}
};

if (document.readyState === 'complete') {
handleInitialScroll();
} else {
window.addEventListener('load', handleInitialScroll, { once: true });
}

window.addEventListener('wiki-route-changed', (event) => {
const newRoute = event.detail?.route;
if (newRoute && newRoute !== this.currentRoute) {
this.currentRoute = newRoute;
this.expandCurrentPageParents();
}
});

this.$watch('$store.navigation.currentRoute', (newRoute) => {
if (newRoute && newRoute !== this.currentRoute) {
this.currentRoute = newRoute;
this.expandCurrentPageParents();
}
});
},

Expand All @@ -293,35 +330,117 @@
},

expandCurrentPageParents() {
// Find and expand all parent groups of the current page
if (!this.currentRoute) return;

// Wait for DOM to be fully rendered

this.$nextTick(() => {
// Find the current page element
const currentPageElement = document.querySelector(`[data-route="${this.currentRoute}"]`);
const sidebarContainer = document.querySelector('.wiki-sidebar');
if (!sidebarContainer) return;

const currentPageElement = sidebarContainer.querySelector(`[data-route="${this.currentRoute}"]`);
if (!currentPageElement) return;

// Find all parent group containers and expand them

let parentElement = currentPageElement.parentElement;
const parentsToExpand = [];

while (parentElement) {
// Look for parent wiki-children containers
if (parentElement.classList.contains('wiki-children')) {
// Find the sibling wiki-item-content that has a data-node-id
const parentItem = parentElement.parentElement;
if (parentItem) {
const parentContent = parentItem.querySelector('.wiki-item-content[data-node-id]');
if (parentContent) {
const nodeId = parentContent.getAttribute('data-node-id');
if (nodeId) {
this.expandedStates[nodeId] = true;
parentsToExpand.push(nodeId);
}
}
}
}
parentElement = parentElement.parentElement;
}

parentsToExpand.forEach(nodeId => {
this.expandedStates[nodeId] = true;
});

this.$nextTick(() => {
this.scrollToSelectedItem();
});
});
},

isElementInViewport(element, container) {
if (!element || !container) return true;
const rect = element.getBoundingClientRect();
const containerRect = container.getBoundingClientRect();
return (
rect.top >= containerRect.top &&
rect.bottom <= containerRect.bottom
);
},

scrollToSelectedItem() {
const attemptScroll = (retries = 0) => {
const sidebarContainer = document.querySelector('.wiki-sidebar');
if (!sidebarContainer) return;

const currentPageElement = sidebarContainer.querySelector(`[data-route="${this.currentRoute}"]`);
if (!currentPageElement) return;

const sidebarNav = sidebarContainer.querySelector('nav');
if (!sidebarNav) return;

const computedStyle = window.getComputedStyle(sidebarContainer);
const isHidden = computedStyle.display === 'none';
const isCollapsed = sidebarContainer.offsetWidth === 0;

if (isHidden || isCollapsed) return;

const containerHeight = sidebarNav.clientHeight;
const containerWidth = sidebarNav.clientWidth;

if ((containerHeight === 0 || containerWidth === 0) && retries < 8) {
const delay = Math.min(100 * Math.pow(1.5, retries), 1000);
this.scrollTimeoutHandle = setTimeout(() => {
this.scrollRafHandle = requestAnimationFrame(() => attemptScroll(retries + 1));
}, delay);
return;
Comment on lines +401 to +406
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Retry timers are not cancelled on destroy().

scrollToSelectedItem may schedule up to 8 setTimeout/requestAnimationFrame callbacks (Lines 399–404) with a maximum combined window of several seconds. destroy() (Lines 430–434) only restores scroll restoration but never cancels these pending handles.

While each callback guards against a missing DOM and exits early, the handles still fire in the destroyed component's closure. Track the active timer and cancel it on teardown:

🛠️ Proposed fix 
+            _scrollRetryTimer: null,
+
             scrollToSelectedItem() {
                 const attemptScroll = (retries = 0) => {
                     ...
                     if ((containerHeight === 0 || containerWidth === 0) && retries < 8) {
                         const delay = Math.min(100 * Math.pow(1.5, retries), 1000);
-                        setTimeout(() => {
+                        this._scrollRetryTimer = setTimeout(() => {
                             requestAnimationFrame(() => attemptScroll(retries + 1));
                         }, delay);
                         return;
                     }
             destroy() {
+                if (this._scrollRetryTimer !== null) {
+                    clearTimeout(this._scrollRetryTimer);
+                    this._scrollRetryTimer = null;
+                }
                 if ('scrollRestoration' in history && this.previousScrollRestoration !== null) {
                     history.scrollRestoration = this.previousScrollRestoration;
                 }
             }

Also applies to: 430-434

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@wiki/templates/wiki/includes/sidebar.html` around lines 399 - 404, The retry
timers started in scrollToSelectedItem can leave pending
setTimeout/requestAnimationFrame callbacks after component teardown; modify
scrollToSelectedItem to store the timeoutId and rafId in outer-scope variables
(e.g., timeoutHandle and rafHandle) whenever calling
setTimeout/requestAnimationFrame, and update any retry recursion to overwrite
those handles; then update destroy() to clear the scheduled tasks by calling
clearTimeout(timeoutHandle) and cancelAnimationFrame(rafHandle) (and reset the
handles) before restoring scroll restoration so no timers fire after destroy;
ensure attempts still early-return if DOM is gone.

}

if (containerHeight === 0) return;

if (this.isElementInViewport(currentPageElement, sidebarNav)) {
return;
}

const elementRect = currentPageElement.getBoundingClientRect();
const containerRect = sidebarNav.getBoundingClientRect();
const elementRelativeTop = elementRect.top - containerRect.top + sidebarNav.scrollTop;
const elementHeight = currentPageElement.offsetHeight;
const scrollPosition = elementRelativeTop - (containerHeight / 2) + (elementHeight / 2);

sidebarNav.scrollTo({
top: Math.max(0, scrollPosition),
behavior: 'instant'
});
};

this.$nextTick(() => {
attemptScroll();
});
},

destroy() {
if (this.scrollTimeoutHandle !== null) {
clearTimeout(this.scrollTimeoutHandle);
this.scrollTimeoutHandle = null;
}
if (this.scrollRafHandle !== null) {
cancelAnimationFrame(this.scrollRafHandle);
this.scrollRafHandle = null;
}
if ('scrollRestoration' in history && this.previousScrollRestoration !== null) {
history.scrollRestoration = this.previousScrollRestoration;
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion wiki/templates/wiki/macros/sidebar_tree.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
{% else %}
{# Page/leaf item #}
<a href="/{{ node.route }}"
@click.prevent.exact="$store.navigation.navigateTo('{{ node.route }}')"
@click="$store.navigation.navigateTo('{{ node.route }}', true, $event)"
@mouseenter="$store.navigation.prefetch('{{ node.route }}')"
class="wiki-item-content wiki-link group w-full flex items-center h-7 rounded px-2 text-[var(--ink-gray-7)] no-underline transition-all duration-200"
:class="$store.navigation.currentRoute === '{{ node.route }}' ? 'bg-[var(--surface-selected)] shadow-sm text-[var(--ink-gray-9)]' : 'hover:bg-[var(--surface-gray-2)]'"
Expand Down