Skip to content

feat(docs): samenhangende navigatie voor landing en documentatie#669

Open
anneschuth wants to merge 2 commits into
mainfrom
feat/landing-docs-nav
Open

feat(docs): samenhangende navigatie voor landing en documentatie#669
anneschuth wants to merge 2 commits into
mainfrom
feat/landing-docs-nav

Conversation

@anneschuth

Copy link
Copy Markdown
Member

Waarom

De navigatie liep tussen de landingspagina en de documentatie door elkaar. De hele balk verschoot bij de overgang (landing-anchors versus doc-secties), waardoor het als twee losse sites voelde. En wie vanaf de Nederlandse landing naar de Engelstalige docs ging, kwam bij terugkeer op de Engelse landing terecht: de taalkeuze werd niet onthouden en de terugknop wees hard naar /en/. Tot slot stonden de taal- en dark-modeschakelaar nogal prominent los in de menubar.

Wat

  • Vaste topbar overal: Home, Documentatie en Aanmelden links; Zoeken, GitHub en een kebab (⋮) rechts. Identiek op landing en docs.
  • Tweede navigatielaag direct onder de bar, alleen waar relevant: in-page anchors op de landing, de doc-secties in de docs. Beide links uitgelijnd, dezelfde NLDD-tokens, actieve sectie onderstreept.
  • Kebab (nldd-icon-button + nldd-menu) voor dark mode en taalkeuze, zodat de menubar rustiger wordt.
  • Taalgeheugen (rr-lang, met in-memory fallback): de weg terug uit de Engelstalige docs leidt naar de gekozen landingstaal in plaats van altijd naar /en/. Logo, Home-link en de no-JS fallback-brand worden client-side gepatcht; zonder JS valt alles terug op echte links.

Docs blijven Engels-only — dit is een navigatie-fix, geen vertaalslag.

Getest

  • Topbar identiek op /, /en/, /guide/..., /aanmelden, /en/signup.
  • Tweede laag: jump-nav op de landing, sectie-nav in de docs met correcte actieve sectie (aria-current).
  • Kebab: dark mode togglet en blijft sticky over navigaties; taalkeuze navigeert en wordt onthouden.
  • Taalgeheugen-pad: NL-landing → docs → logo/Home leidt terug naar / (NL); vanaf EN naar /en/.
  • npm run a11y (pa11y-ci): 61/61 pagina's, 0 errors. Geen console-errors.

De topbar is nu overal identiek (Home, Documentatie, Aanmelden links;
Zoeken, GitHub, kebab rechts). De wisselende navigatie verhuist naar een
tweede laag direct onder de bar: in-page anchors op de landing, de
doc-secties in de docs, beide links uitgelijnd en met dezelfde
NLDD-tokens. Eerder verschoot de hele bar tussen landing en docs, wat als
twee losse sites voelde.

De taal- en dark-modeschakelaar staan niet langer los in de menubar maar
onder een kebab (nldd-icon-button + nldd-menu). De taalkeuze wordt
onthouden (rr-lang), zodat de weg terug uit de Engelstalige docs naar de
gekozen landingstaal leidt in plaats van altijd naar /en/. Logo, Home en
de fallback-brand worden client-side naar die taal gepatcht; zonder JS
valt alles terug op echte links.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review: feat(docs) samenhangende navigatie voor landing en documentatie

This is a docs-site navigation refactor. No law YAML, engine Rust, or pipeline changes — so legal-faithfulness, engine-correctness, and job-queue dimensions are out of scope.


🟡 Minor — Dead ternary on homeLabel

Base.astro (newly added line):

const homeLabel = lang === 'en' ? 'Home' : 'Home';

Both branches produce the same string. This could just be const homeLabel = 'Home'; (or inline the literal).


🟡 Minor — No-JS language switch regresses on signup pages

In the no-JS fallback nav (newly added block):

{isLanding && (
  <li>
    <a href={lang === 'en' ? '/' : '/en/'}>
      {lang === 'en' ? 'Nederlands' : 'English'}
    </a>
  </li>
)}

isLanding is true on signup pages too (showSignup = isLanding && /aanmelden|signup/.test(pathname)). So on /aanmelden without JS the "English" link now goes to /en/ (EN landing) instead of /en/signup. The old server-side langToggle preserved the signup destination for the no-JS path. The JS-powered path (langTargetFor) handles this correctly — only the no-JS fallback is affected. Fix: guard the //en/ links with !showSignup, and render /aanmelden/en/signup links when showSignup is true.


🟡 Minor — if (target) guard is always truthy

const target = langTargetFor(lang, location.pathname);
if (target) location.assign(target);

langTargetFor always returns a non-empty string (never null/undefined/''), so the guard is dead. Not a bug, but slightly misleading — remove the if or document why it might be falsy.


No critical or significant issues found.

@github-actions

github-actions Bot commented May 21, 2026

Copy link
Copy Markdown

Preview Deployment — docs — docs

Your changes have been deployed to a preview environment:

URL: https://docs-pr669-regel-k4c.rig.prd1.gn2.quattro.rijksapps.nl

This deployment will be automatically cleaned up when the PR is closed.

De jump-nav en sectie-nav stonden op 1.5rem terwijl de NLDD-bar zijn
items inset met de page-section margin-token. Gebruik diezelfde token
(--semantics-page-sections-lg-margin-inline) zodat beide lagen exact
onder de menu-items uitlijnen, ook over NLDD's responsive breakpoints.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review

Navigatie-refactor is goed opgezet: de topbar-unificatie, de tweede laag, en het taalgeheugen zijn logisch gestructureerd. XSS-risico is afwezig — de rr-lang waarde uit localStorage wordt nooit direct in de DOM gezet maar loopt altijd door een ternary (lang === 'nl' ? '/' : '/en/'). De a11y-tests (61/61, 0 errors) geven vertrouwen.

Twee bevindingen, beide minor.


🟡 Minor — Dode ternary op homeLabel

docs/src/layouts/Base.astro (nieuwe regel ~58)

const homeLabel = lang === 'en' ? 'Home' : 'Home';

Beide takken zijn identiek. Dat maakt de conditie zinloos. Kan direct worden:

const homeLabel = 'Home';

Of, omdat de variabele op precies één plek gebruikt wordt in de globalLinks-array, kan hij helemaal worden weggelaten en 'Home' inline worden gezet.


🟡 Minor — SSR-gebaseerde selected-staat voor taalitems reflecteert niet het onthouden voorkeur

docs/src/layouts/Base.astro (nieuwe regels voor de twee nldd-menu-item-taalitems)

selected={lang === 'nl' || undefined}   // docs: altijd undefined
selected={lang === 'en' || undefined}   // docs: altijd true

In de docs is lang altijd 'en', dus 'English' verschijnt altijd als geselecteerd — ook voor een gebruiker wiens opgeslagen voorkeur nl is. wireKebab() synct na load wél de theme-toggle, maar niet de taalitems. Het navigatiegedrag klopt (klikken stuurt je naar de juiste taal), maar de visuele feedback suggereert "je huidige voorkeur" terwijl het "de taal van de huidige pagina" betekent.

De eenvoudigste fix is om in patchHomeLinks() (of een aparte wireLangItems()) de selected-attributen client-side te patchen op basis van readLang():

function wireLangItems() {
  const remembered = readLang();
  if (!remembered) return;
  document.querySelectorAll('.rr-lang-item').forEach((item) => {
    item.toggleAttribute('selected', item.getAttribute('value') === remembered);
  });
}

Aanroepen naast wireKebab() en patchHomeLinks() in de astro:page-load-handler. Dit is een UX-verbetering, geen bug.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review

Navigatie-refactor is goed opgezet: de topbar-unificatie, de tweede laag, en het taalgeheugen zijn logisch gestructureerd. XSS-risico is afwezig — de rr-lang waarde uit localStorage wordt nooit direct in de DOM gezet maar loopt altijd door een ternary (lang === 'nl' ? '/' : '/en/'). De a11y-tests (61/61, 0 errors) geven vertrouwen.

Twee bevindingen, beide minor.


Minor 1 — Dode ternary op homeLabel

In docs/src/layouts/Base.astro, nieuwe regel:

const homeLabel = lang === 'en' ? 'Home' : 'Home';

Beide takken zijn identiek. Dit kan direct worden:

const homeLabel = 'Home';

Minor 2 — SSR selected-staat voor taalitems reflecteert niet het onthouden voorkeur

In de docs is lang altijd 'en', dus selected={lang === 'en' || undefined} is altijd true voor het Engelse item. wireKebab() synct na load wel de theme-toggle, maar niet de taalitems. Gevolg: een gebruiker met opgeslagen voorkeur NL ziet in de docs "English" als geselecteerd. Navigatiegedrag klopt wel — dit is puur visuele feedback.

Suggestie: voeg een klein wireLangItems() toe dat na page-load de selected-attributen patcht op basis van readLang():

function wireLangItems() {
  const remembered = readLang();
  if (!remembered) return;
  document.querySelectorAll('.rr-lang-item').forEach((item) => {
    item.toggleAttribute('selected', item.getAttribute('value') === remembered);
  });
}

Aanroepen in de astro:page-load-handler naast wireKebab() en patchHomeLinks().

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review

Navigatie-refactor is goed opgezet: topbar-unificatie, de tweede laag, en het taalgeheugen zijn logisch gestructureerd. XSS-risico is afwezig — de rr-lang waarde uit localStorage wordt nooit direct in de DOM gezet maar loopt altijd door een ternary (lang === 'nl' ? '/' : '/en/'). De a11y-tests (61/61, 0 errors) geven vertrouwen.

Twee minor bevindingen.


Minor 1 — Dode ternary op homeLabel

// docs/src/layouts/Base.astro (nieuw)
const homeLabel = lang === 'en' ? 'Home' : 'Home';

Beide takken zijn identiek. Vereenvoudigen naar const homeLabel = 'Home'; of de variabele helemaal weglaten.


Minor 2 — selected-staat taalitems reflecteert niet onthouden voorkeur

// docs/src/layouts/Base.astro (nieuw)
selected={lang === 'nl' || undefined}   // op docs-pagina's altijd undefined
selected={lang === 'en' || undefined}   // op docs-pagina's altijd true

Docs zijn altijd lang === 'en', dus "English" verschijnt altijd geselecteerd in het kebab-menu — ook voor bezoekers wiens opgeslagen voorkeur nl is. wireKebab() synct de theme-toggle na load maar patcht de taalitems niet. Navigatiegedrag is correct; het is puur visuele feedback die afwijkt van de herinnerde voorkeur.

Mogelijke fix: voeg een wireLangItems() toe die na astro:page-load de selected-attributen patcht op basis van readLang():

function wireLangItems() {
  const remembered = readLang();
  if (!remembered) return;
  document.querySelectorAll('.rr-lang-item').forEach((item) => {
    item.toggleAttribute('selected', item.getAttribute('value') === remembered);
  });
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant