Skip to content

Address late review feedback on sidebar restructure (#962) #983

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
IEvangelist wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Address late review feedback on sidebar restructure (#962) #983

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f53ce1
Address late review feedback on sidebar restructure (#962)
Copilot May 15, 2026
8d45f6e
Fix TDZ ReferenceError in sidebar toggle teleport script
Copilot May 15, 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
8 changes: 4 additions & 4 deletions src/frontend/src/components/SiteTour.astro
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ const siteTourStrings = {
'Use this control to collapse the sidebar for more reading space, or bring it back when you need the page and section navigation.'
),
},
topicsDropdown: {
title: tt('siteTour.steps.topicsDropdown.title', 'Jump to another doc area'),
topicsList: {
title: tt('siteTour.steps.topicsList.title', 'Jump to another doc area'),
body: tt(
'siteTour.steps.topicsDropdown.body',
'Open this menu to move between top-level doc areas without leaving the docs layout.'
'siteTour.steps.topicsList.body',
'Switch between top-level doc areas from this list without leaving the docs layout.'
),
},
search: {
Expand Down
84 changes: 10 additions & 74 deletions src/frontend/src/components/site-tour.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ declare global {

export type SiteTourStepId =
| 'sidebar-toggle'
| 'topics-dropdown'
| 'topics-list'
| 'search'
| 'install-cli'
| 'cookie-preferences'
Expand Down Expand Up @@ -53,7 +53,7 @@ export interface SiteTourStrings {
};
steps: {
sidebarToggle: SiteTourCopy;
topicsDropdown: SiteTourCopy;
topicsList: SiteTourCopy;
search: SiteTourCopy;
installCli: SiteTourCopy;
cookiePreferences: SiteTourCopy;
Expand Down Expand Up @@ -124,7 +124,7 @@ export const SITE_TOUR_STORAGE_KEY = 'aspire-site-tour-v1';
export const SITE_TOUR_MOBILE_MEDIA_QUERY = '(max-width: 49.999rem)';
export const SITE_TOUR_STEP_ORDER = [
'sidebar-toggle',
'topics-dropdown',
'topics-list',
'search',
'install-cli',
'cookie-preferences',
Expand Down Expand Up @@ -176,14 +176,12 @@ export function createSiteTourStepDefinitions(strings: SiteTourStrings): SiteTou
selectors: ['[data-tour-step="sidebar-toggle"]', '[data-tour-target="sidebar-toggle"]'],
},
{
id: 'topics-dropdown',
title: strings.steps.topicsDropdown.title,
body: strings.steps.topicsDropdown.body,
id: 'topics-list',
title: strings.steps.topicsList.title,
body: strings.steps.topicsList.body,
selectors: [
'[data-tour-step="topics-dropdown"] [data-dropdown-trigger]',
'[data-tour-target="topics-dropdown"] [data-dropdown-trigger]',
'[data-tour-step="topics-dropdown"]',
'[data-tour-target="topics-dropdown"]',
'[data-tour-step="topics-list"]',
'[data-tour-target="topics-list"]',
],
},
{
Expand Down Expand Up @@ -1008,71 +1006,9 @@ class AspireSiteTour {
return null;
}

private getTopicsDropdown(): Element | null {
return document.querySelector('[data-tour-target="topics-dropdown"]');
}

private isTopicsDropdownOpen(): boolean {
const dropdown = this.getTopicsDropdown();
if (!(dropdown instanceof HTMLElement)) {
return false;
}

return (
dropdown.dataset.open === 'true' ||
dropdown.querySelector('[data-dropdown-trigger]')?.getAttribute('aria-expanded') === 'true'
);
}

private getHighlightRect(step: SiteTourStepDefinition, target: HTMLElement): HighlightRect {
private getHighlightRect(target: HTMLElement): HighlightRect {
const rect = target.getBoundingClientRect();

if (step.id === 'topics-dropdown') {
const dropdown = this.getTopicsDropdown();
const trigger =
target.matches('[data-dropdown-trigger]')
? target
: dropdown?.querySelector('[data-dropdown-trigger]');
const triggerRect = trigger instanceof HTMLElement ? trigger.getBoundingClientRect() : rect;

if (!this.isTopicsDropdownOpen()) {
return {
top: triggerRect.top,
left: triggerRect.left,
right: triggerRect.right,
bottom: triggerRect.bottom,
width: triggerRect.width,
height: triggerRect.height,
};
}

const panel = dropdown?.querySelector('[data-dropdown-panel]:not([hidden])');
if (panel instanceof HTMLElement) {
const panelRect = panel.getBoundingClientRect();
return {
top: Math.min(triggerRect.top, panelRect.top),
left: Math.min(triggerRect.left, panelRect.left),
right: Math.max(triggerRect.right, panelRect.right),
bottom: Math.max(triggerRect.bottom, panelRect.bottom),
width:
Math.max(triggerRect.right, panelRect.right) -
Math.min(triggerRect.left, panelRect.left),
height:
Math.max(triggerRect.bottom, panelRect.bottom) -
Math.min(triggerRect.top, panelRect.top),
};
}

return {
top: triggerRect.top,
left: triggerRect.left,
right: triggerRect.right,
bottom: triggerRect.bottom,
width: triggerRect.width,
height: triggerRect.height,
};
}

return {
top: rect.top,
left: rect.left,
Expand Down Expand Up @@ -1339,7 +1275,7 @@ class AspireSiteTour {
this.hint.textContent = '';
}

const rawRect = this.getHighlightRect(step, target);
const rawRect = this.getHighlightRect(target);
const padding = getStepHighlightPadding(step.id);
const hole = {
top: clamp(rawRect.top - padding, 0, window.innerHeight),
Expand Down
101 changes: 77 additions & 24 deletions src/frontend/src/components/starlight/Sidebar.astro
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,14 @@ const hasTopicsList = Boolean(currentTopic && effectiveTopics.length > 0);
{hasTopicsList && (
<ul
class="starlight-sidebar-topics topics-sidebar-list"
data-tour-target="topics-dropdown"
data-tour-step="topics-dropdown"
data-tour-target="topics-list"
data-tour-step="topics-list"
>
{effectiveTopics.map((topic) => (
<li>
<a
href={topic.link}
title={topic.label}
aria-label={topic.label}
aria-current={topic.isCurrent ? 'page' : undefined}
class:list={{ 'starlight-sidebar-topics-current': topic.isCurrent }}
>
Expand Down Expand Up @@ -243,15 +242,14 @@ const hasTopicsList = Boolean(currentTopic && effectiveTopics.length > 0);
<div class="topic-sidebar-sticky not-content">
<ul
class="starlight-sidebar-topics topics-sidebar-list"
data-tour-target="topics-dropdown"
data-tour-step="topics-dropdown"
data-tour-target="topics-list"
data-tour-step="topics-list"
>
{effectiveTopics.map((topic) => (
<li>
<a
href={topic.link}
title={topic.label}
aria-label={topic.label}
aria-current={topic.isCurrent ? 'page' : undefined}
class:list={{ 'starlight-sidebar-topics-current': topic.isCurrent }}
>
Expand Down Expand Up @@ -402,8 +400,7 @@ const hasTopicsList = Boolean(currentTopic && effectiveTopics.length > 0);
data-tour-target="sidebar-toggle"
aria-label="Expand sidebar"
title="Expand sidebar"
aria-expanded="false"
aria-controls="starlight__sidebar"
aria-hidden="true"
>
<svg
viewBox="0 0 24 24"
Expand Down Expand Up @@ -459,8 +456,7 @@ const hasTopicsList = Boolean(currentTopic && effectiveTopics.length > 0);
data-tour-target="sidebar-toggle"
aria-label="Expand sidebar"
title="Expand sidebar"
aria-expanded="false"
aria-controls="starlight__sidebar"
aria-hidden="true"
>
<svg
viewBox="0 0 24 24"
Expand Down Expand Up @@ -1649,14 +1645,29 @@ const hasTopicsList = Boolean(currentTopic && effectiveTopics.length > 0);
}
}

function syncAriaExpanded(
/**
* Keep the disclosure-button pair semantically correct: only one button
* in each pair is visually present (the other is `visibility: hidden`),
* so only the visible one should advertise `aria-controls` /
* `aria-expanded`. The hidden one gets `aria-hidden="true"` and drops
* those attrs entirely, so assistive tech sees a single live control
* for the sidebar disclosure at any time.
*/
function syncToggleA11y(
collapseBtn: HTMLElement,
expandBtn: HTMLElement,
isCollapsed: boolean
): void {
const value = isCollapsed ? 'false' : 'true';
collapseBtn.setAttribute('aria-expanded', value);
expandBtn.setAttribute('aria-expanded', value);
const active = isCollapsed ? expandBtn : collapseBtn;
const inactive = isCollapsed ? collapseBtn : expandBtn;

active.setAttribute('aria-expanded', isCollapsed ? 'false' : 'true');
active.setAttribute('aria-controls', 'starlight__sidebar');
active.removeAttribute('aria-hidden');

inactive.setAttribute('aria-hidden', 'true');
inactive.removeAttribute('aria-expanded');
inactive.removeAttribute('aria-controls');
}

function initSidebarCollapse() {
Expand All @@ -1679,7 +1690,7 @@ const hasTopicsList = Boolean(currentTopic && effectiveTopics.length > 0);

// Sync aria-expanded with the data-sidebar-collapsed attribute that
// Head.astro set from localStorage before this script ran.
syncAriaExpanded(
syncToggleA11y(
collapseBtn,
expandBtn,
document.documentElement.hasAttribute('data-sidebar-collapsed')
Expand Down Expand Up @@ -1721,7 +1732,7 @@ const hasTopicsList = Boolean(currentTopic && effectiveTopics.length > 0);
return;
}

syncAriaExpanded(
syncToggleA11y(
collapseBtn,
expandBtn,
document.documentElement.hasAttribute('data-topic-sidebar-collapsed')
Expand Down Expand Up @@ -1758,8 +1769,7 @@ const hasTopicsList = Boolean(currentTopic && effectiveTopics.length > 0);
window.clearTimeout(topicSidebarExpandTimer);
root.removeAttribute('data-topic-sidebar-expanding');
root.setAttribute('data-topic-sidebar-collapsed', '');
collapseBtn.setAttribute('aria-expanded', 'false');
expandBtn.setAttribute('aria-expanded', 'false');
syncToggleA11y(collapseBtn, expandBtn, true);
// Focus the now-visible button so keyboard users stay anchored.
try {
expandBtn.focus({ preventScroll: true });
Expand All @@ -1777,8 +1787,7 @@ const hasTopicsList = Boolean(currentTopic && effectiveTopics.length > 0);
}, SIDEBAR_EXPAND_ANIMATION_MS);
}
root.removeAttribute('data-topic-sidebar-collapsed');
collapseBtn.setAttribute('aria-expanded', 'true');
expandBtn.setAttribute('aria-expanded', 'true');
syncToggleA11y(collapseBtn, expandBtn, false);
if (wasCollapsed) {
try {
collapseBtn.focus({ preventScroll: true });
Expand Down Expand Up @@ -1806,8 +1815,7 @@ const hasTopicsList = Boolean(currentTopic && effectiveTopics.length > 0);
window.clearTimeout(sidebarExpandTimer);
root.removeAttribute('data-sidebar-expanding');
root.setAttribute('data-sidebar-collapsed', '');
collapseBtn.setAttribute('aria-expanded', 'false');
expandBtn.setAttribute('aria-expanded', 'false');
syncToggleA11y(collapseBtn, expandBtn, true);
try {
expandBtn.focus({ preventScroll: true });
} catch {
Expand All @@ -1822,8 +1830,7 @@ const hasTopicsList = Boolean(currentTopic && effectiveTopics.length > 0);
}, SIDEBAR_EXPAND_ANIMATION_MS);
}
root.removeAttribute('data-sidebar-collapsed');
collapseBtn.setAttribute('aria-expanded', 'true');
expandBtn.setAttribute('aria-expanded', 'true');
syncToggleA11y(collapseBtn, expandBtn, false);
if (wasCollapsed) {
try {
collapseBtn.focus({ preventScroll: true });
Expand All @@ -1846,6 +1853,11 @@ const hasTopicsList = Boolean(currentTopic && effectiveTopics.length > 0);
document.addEventListener('astro:after-swap', initTopicSidebarUi);
document.addEventListener('DOMContentLoaded', initBannerHeightTracking);
document.addEventListener('astro:after-swap', initBannerHeightTracking);
// Remove teleported toggle buttons from <body> *before* the swap so
// they can't conflict with the fresh buttons rendered into the next
// page's sidebar (and so any stale handlers bound to them are dropped
// along with the nodes themselves).
document.addEventListener('astro:before-swap', cleanupTeleportedToggles);
document.addEventListener('DOMContentLoaded', teleportSidebarToggles);
document.addEventListener('astro:after-swap', teleportSidebarToggles);
}
Expand All @@ -1858,6 +1870,34 @@ const hasTopicsList = Boolean(currentTopic && effectiveTopics.length > 0);

/* ── Sidebar toggle teleport ──────────────────────────────────── */

/**
* Remove any toggle buttons that were previously teleported to
* `<body>`. Called as an `astro:before-swap` listener so stale
* buttons (and the click handlers bound to them) are discarded
* before the next page's HTML morph runs. After the swap, the
* new page's freshly-rendered buttons get found, bound, and
* re-teleported by `initSidebarCollapse` / `teleportSidebarToggles`.
*/
function cleanupTeleportedToggles() {
const ids = [
'sidebar-collapse-btn',
'sidebar-expand-btn',
'topic-sidebar-collapse-btn',
'topic-sidebar-expand-btn',
];
for (const id of ids) {
// querySelectorAll guards against the (invalid but possible) case
// where a stale teleported button and a fresh in-sidebar button
// briefly share the same id during a swap.
const matches = document.querySelectorAll<HTMLElement>(`#${id}`);
matches.forEach((btn) => {
if (btn.parentElement === document.body) {
btn.remove();
}
});
}
}

/**
* Move the floating sidebar collapse/expand toggle buttons to
* `<body>` so they escape the sidebar's stacking context. The
Expand All @@ -1875,6 +1915,19 @@ const hasTopicsList = Boolean(currentTopic && effectiveTopics.length > 0);
'topic-sidebar-collapse-btn',
'topic-sidebar-expand-btn',
];
// Defensive sweep: if any stale teleported buttons survived the
// last swap (e.g. an aborted view transition), drop them so they
// can't shadow the fresh in-sidebar buttons via getElementById.
for (const id of ids) {
const matches = document.querySelectorAll<HTMLElement>(`#${id}`);
if (matches.length <= 1) continue;
matches.forEach((btn) => {
if (btn.parentElement === document.body) {
btn.remove();
}
});
}

for (const id of ids) {
const btn = document.getElementById(id);
if (btn && btn.parentElement !== document.body) {
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/src/content/i18n/da.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
"title": "Vis eller skjul sidepanelet",
"body": "Brug denne kontrol til at skjule sidepanelet for at få mere læseplads, eller vis det igen, når du har brug for side- og sektionsnavigation."
},
"topicsDropdown": {
"topicsList": {
"title": "Gå til et andet dokumentområde",
"body": "Åbn denne menu for at skifte mellem overordnede dokumentområder uden at forlade dokumentlayoutet."
},
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/src/content/i18n/de.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
"title": "Seitenleiste ein- oder ausblenden",
"body": "Mit diesem Steuerelement kannst du die Seitenleiste ausblenden, um mehr Platz zum Lesen zu haben, oder sie wieder einblenden, wenn du die Seiten- und Abschnittsnavigation brauchst."
},
"topicsDropdown": {
"topicsList": {
"title": "Zu einem anderen Dokumentbereich wechseln",
"body": "Öffne dieses Menü, um zwischen Dokumentbereichen der obersten Ebene zu wechseln, ohne das Dokumentlayout zu verlassen."
},
Expand Down
4 changes: 2 additions & 2 deletions src/frontend/src/content/i18n/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@
"title": "Show or hide the sidebar",
"body": "Use this control to collapse the sidebar for more reading space, or bring it back when you need the page and section navigation."
},
"topicsDropdown": {
"topicsList": {
"title": "Jump to another doc area",
"body": "Open this menu to move between top-level doc areas without leaving the docs layout."
"body": "Switch between top-level doc areas from this list without leaving the docs layout."
},
"search": {
"title": "Search the docs",
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/src/content/i18n/es.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
"title": "Mostrar u ocultar la barra lateral",
"body": "Usa este control para contraer la barra lateral y ganar espacio de lectura, o volver a mostrarla cuando necesites la navegación por páginas y secciones."
},
"topicsDropdown": {
"topicsList": {
"title": "Ir a otra sección de la documentación",
"body": "Abre este menú para moverte entre áreas principales de la documentación sin salir del diseño de docs."
},
Expand Down
Loading
Loading