When opening ToC on mobile, ensure active ToC item is scrolled into v…

…iew (#54) * When opening mobile view, scroll to the active item * Return early if nav element isn't available * update dependencies * DemoNav remove falsely copy-pasted console.error * 'subheadings are indented' simplify test DOM * reset windowWidth in between unit tests prevents side-effect leakage * emit 'open' events whenever open prop changes add test for custom open event * test toggling open state when clicking ToC menu button * change condition class:active={activeTocLi === tocItems[idx]} was class:active={activeHeading === heading} also rename handler->li_click_key_handler * ensure active ToC is in view when ToC opens on mobile * test `active heading is in into view and highlighted when opening ToC on mobile` --------- Co-authored-by: Janosh Riebesell <[email protected]>
janosh · Mar 1, 2024 · 34b92a0 · 34b92a0
1 parent ccd6407
commit 34b92a0
Show file tree

Hide file tree

Showing 6 changed files with 126 additions and 57 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -35,7 +35,7 @@ repos:
         args: [--ignore-words-list, falsy, --check-filenames]
 
   - repo: https://github.com/pre-commit/mirrors-eslint
-    rev: v9.0.0-alpha.1
+    rev: v9.0.0-beta.1
     hooks:
       - id: eslint
         types: [file]

diff --git a/package.json b/package.json
@@ -23,34 +23,34 @@
     "update-coverage": "vitest tests/unit --run --coverage && npx istanbul-badges-readme"
   },
   "dependencies": {
-    "svelte": "^4.2.9"
+    "svelte": "^4.2.12"
   },
   "devDependencies": {
-    "@playwright/test": "1.41.1",
+    "@playwright/test": "1.42.0",
     "@sveltejs/adapter-static": "^3.0.1",
-    "@sveltejs/kit": "^2.4.1",
-    "@sveltejs/package": "^2.2.6",
-    "@sveltejs/vite-plugin-svelte": "^3.0.1",
-    "@typescript-eslint/eslint-plugin": "^6.19.0",
-    "@typescript-eslint/parser": "^6.19.0",
-    "@vitest/coverage-v8": "^1.2.1",
-    "eslint": "^8.56.0",
+    "@sveltejs/kit": "^2.5.2",
+    "@sveltejs/package": "^2.2.7",
+    "@sveltejs/vite-plugin-svelte": "^3.0.2",
+    "@typescript-eslint/eslint-plugin": "^7.1.0",
+    "@typescript-eslint/parser": "^7.1.0",
+    "@vitest/coverage-v8": "^1.3.1",
+    "eslint": "^8.57.0",
     "eslint-plugin-svelte": "^2.35.1",
     "hastscript": "^9.0.0",
     "jsdom": "^24.0.0",
     "mdsvex": "^0.11.0",
     "mdsvexamples": "^0.4.1",
-    "prettier": "^3.2.4",
-    "prettier-plugin-svelte": "^3.1.2",
+    "prettier": "^3.2.5",
+    "prettier-plugin-svelte": "^3.2.2",
     "rehype-autolink-headings": "^7.1.0",
     "rehype-slug": "^6.0.0",
-    "svelte-check": "^3.6.3",
+    "svelte-check": "^3.6.6",
     "svelte-preprocess": "^5.1.3",
-    "svelte-zoo": "^0.4.9",
-    "svelte2tsx": "^0.7.0",
+    "svelte-zoo": "^0.4.10",
+    "svelte2tsx": "^0.7.3",
     "typescript": "5.3.3",
-    "vite": "^5.0.12",
-    "vitest": "^1.2.1"
+    "vite": "^5.1.4",
+    "vitest": "^1.3.1"
   },
   "keywords": [
     "svelte",

diff --git a/src/lib/Toc.svelte b/src/lib/Toc.svelte
@@ -1,13 +1,13 @@
 <script lang="ts">
-  import { onMount } from 'svelte'
+  import { createEventDispatcher, onMount } from 'svelte'
   import { blur, type BlurParams } from 'svelte/transition'
   import { MenuIcon } from '.'
 
   export let activeHeading: HTMLHeadingElement | null = null
   export let activeHeadingScrollOffset: number = 100
   export let activeTocLi: HTMLLIElement | null = null
   export let aside: HTMLElement | undefined = undefined
-  export let breakpoint: number = 1000
+  export let breakpoint: number = 1000 // in pixels (smaller window width is considered mobile, larger is desktop)
   export let desktop: boolean = true
   export let flashClickedHeadingsFor: number = 1500
   export let getHeadingIds = (node: HTMLHeadingElement): string => node.id
@@ -34,13 +34,16 @@
   export let blurParams: BlurParams | undefined = { duration: 200 }
 
   let window_width: number
+  // dispatch open event when open changes
+  const dispatch = createEventDispatcher()
+  $: dispatch(`open`, { open })
 
   $: levels = headings.map(getHeadingLevels)
   $: minLevel = Math.min(...levels)
   $: desktop = window_width > breakpoint
 
   function close(event: MouseEvent) {
-    if (!aside.contains(event.target as Node)) open = false
+    if (!aside?.contains(event.target as Node)) open = false
   }
 
   // (re-)query headings on mount and on route changes
@@ -91,19 +94,34 @@
     }
   }
 
-  const handler = (node: HTMLHeadingElement) => (event: MouseEvent | KeyboardEvent) => {
-    if (event instanceof KeyboardEvent && ![`Enter`, ` `].includes(event.key)) return
-    open = false
-    node.scrollIntoView({ behavior: scrollBehavior, block: `start` })
+  const li_click_key_handler =
+    (node: HTMLHeadingElement) => (event: MouseEvent | KeyboardEvent) => {
+      if (event instanceof KeyboardEvent && ![`Enter`, ` `].includes(event.key)) return
+      open = false
+      node.scrollIntoView({ behavior: scrollBehavior, block: `start` })
 
-    const id = getHeadingIds && getHeadingIds(node)
-    if (id) history.replaceState({}, ``, `#${id}`)
+      const id = getHeadingIds && getHeadingIds(node)
+      if (id) history.replaceState({}, ``, `#${id}`)
 
-    if (flashClickedHeadingsFor) {
-      node.classList.add(`toc-clicked`)
-      setTimeout(() => node.classList.remove(`toc-clicked`), flashClickedHeadingsFor)
+      if (flashClickedHeadingsFor) {
+        node.classList.add(`toc-clicked`)
+        setTimeout(() => node.classList.remove(`toc-clicked`), flashClickedHeadingsFor)
+      }
+    }
+
+  function scroll_to_active_toc_item(behavior: 'auto' | 'smooth' | 'instant' = `smooth`) {
+    if (keepActiveTocItemInView && activeTocLi && nav) {
+      // scroll the active ToC item into the middle of the ToC container
+      const top = activeTocLi?.offsetTop - nav.offsetHeight / 2
+      nav?.scrollTo?.({ top, behavior })
     }
   }
+
+  // ensure active ToC is in view when ToC opens on mobile
+  $: if (open && nav) {
+    set_active_heading()
+    scroll_to_active_toc_item(`instant`)
+  }
 </script>
 
 <svelte:window
@@ -113,13 +131,7 @@
   on:scrollend={() => {
     // wait for scroll end since Chrome doesn't support multiple simultaneous scrolls,
     // smooth or otherwise (https://stackoverflow.com/a/63563437)
-    if (keepActiveTocItemInView && activeTocLi) {
-      // scroll the active ToC item into the middle of the ToC container
-      nav.scrollTo?.({
-        top: activeTocLi?.offsetTop - nav.offsetHeight / 2,
-        behavior: `smooth`,
-      })
-    }
+    scroll_to_active_toc_item()
   }}
 />
 
@@ -156,9 +168,9 @@
           <li
             style:margin="0 0 0 {levels[idx] - minLevel}em"
             style:font-size="{2 - 0.2 * (levels[idx] - minLevel)}ex"
-            class:active={activeHeading === heading}
-            on:click={handler(heading)}
-            on:keyup={handler(heading)}
+            class:active={activeTocLi === tocItems[idx]}
+            on:click={li_click_key_handler(heading)}
+            on:keyup={li_click_key_handler(heading)}
             bind:this={tocItems[idx]}
             role="menuitem"
           >

diff --git a/src/site/DemoNav.svelte b/src/site/DemoNav.svelte
@@ -4,12 +4,9 @@
   export let style: string | null = null
 
   const routes = Object.keys(
-    import.meta.glob(`/src/routes/\\(demos\\)/*/+page*.{svx,md,svelte}`)
+    import.meta.glob(`/src/routes/\\(demos\\)/*/+page*.{svx,md,svelte}`),
   ).map((filename) => filename.split(`/`)[4])
 
-  if (routes.length < 3) {
-    console.error(`Too few demo routes found: ${routes.length}`)
-  }
   $: is_current = (path: string) => {
     if (`/${path}` == $page.url.pathname) return `page`
     return undefined

diff --git a/tests/unit/index.ts b/tests/unit/index.ts
@@ -2,6 +2,8 @@ import { beforeEach } from 'vitest'
 
 beforeEach(() => {
   document.body.innerHTML = ``
+  // reset window width
+  window.innerWidth = 1024
 })
 
 export function doc_query<T extends HTMLElement>(selector: string): T {

diff --git a/tests/unit/toc.test.ts b/tests/unit/toc.test.ts
@@ -52,9 +52,9 @@ describe(`Toc`, () => {
       expect(toc).toBeTruthy()
       await tick()
 
-      const toc_ul = doc_query(`aside.toc ol`)
-      expect(toc_ul.children.length).toBe(expected_lis)
-      expect(toc_ul.textContent?.trim()).toBe(expected_text?.join(` `))
+      const toc_list = doc_query(`aside.toc > nav > ol`)
+      expect(toc_list.children.length).toBe(expected_lis)
+      expect(toc_list.textContent?.trim()).toBe(expected_text?.join(` `))
     },
   )
 
@@ -112,21 +112,19 @@ describe(`Toc`, () => {
 
   test(`subheadings are indented`, async () => {
     document.body.innerHTML = `
-      <main>
-        <h1>Heading 1</h1>
-        <h2>Heading 2</h2>
-        <h3>Heading 3</h3>
-        <h4>Heading 4</h4>
-      </main>
+      <h1>Heading 1</h1>
+      <h2>Heading 2</h2>
+      <h3>Heading 3</h3>
+      <h4>Heading 4</h4>
     `
 
     new Toc({ target: document.body })
     await tick()
 
-    const toc_ul = doc_query(`aside.toc > nav > ol`)
-    expect(toc_ul.children.length).toBe(3)
+    const toc_list = doc_query(`aside.toc > nav > ol`)
+    expect(toc_list.children.length).toBe(3)
 
-    const lis = [...toc_ul.children] as HTMLLIElement[]
+    const lis = [...toc_list.children] as HTMLLIElement[]
     expect(lis[0].style.marginLeft).toBe(`0em`)
     expect(lis[1].style.marginLeft).toBe(`1em`)
     expect(lis[2].style.marginLeft).toBe(`2em`)
@@ -151,10 +149,10 @@ describe(`Toc`, () => {
           (lvl) => lvl >= 2 && lvl <= 4,
         ).length
         if (matches >= minItems) {
-          const toc_ul = doc_query(`aside.toc ol`)
+          const toc_list = doc_query(`aside.toc > nav > ol`)
 
           expect(
-            toc_ul.children.length,
+            toc_list.children.length,
             `heading_levels=${heading_levels}, minItems=${minItems}`,
           ).toBe(matches)
         } else {
@@ -219,4 +217,64 @@ describe(`Toc`, () => {
     expect(toc.nav).toBeInstanceOf(HTMLElement)
     expect(toc.nav.tagName).toBe(`NAV`)
   })
+
+  test(`open custom event fires whenever open changes`, async () => {
+    const toc = new Toc({ target: document.body })
+
+    const open_handler = vi.fn()
+    toc.$on(`open`, open_handler)
+
+    toc.open = true
+    await tick()
+    expect(open_handler).toHaveBeenCalledOnce()
+    // check event.detail.open == true
+    expect(open_handler.mock.calls[0][0].detail.open).toBe(true)
+
+    toc.open = false
+    await tick()
+    expect(open_handler).toHaveBeenCalledTimes(2)
+    // check event.detail.open == false
+    expect(open_handler.mock.calls[1][0].detail.open).toBe(false)
+  })
+
+  test(`should toggle open state when clicking the button`, async () => {
+    // simulate mobile
+    window.innerWidth = 600
+
+    const toc = new Toc({ target: document.body })
+
+    const button = doc_query(`aside.toc button`)
+    expect(button).toBeTruthy()
+
+    expect(toc.open).toBe(false)
+    button.click()
+    expect(toc.open).toBe(true)
+    // click anywhere else
+    document.body.click()
+    expect(toc.open).toBe(false)
+    button.click()
+    expect(toc.open).toBe(true)
+  })
+
+  test(`active heading is in into view and highlighted when opening ToC on mobile`, async () => {
+    document.body.innerHTML = [...Array(100)]
+      .map((_, idx) => `<h2>Heading ${idx + 1}</h2>`)
+      .join(`\n`)
+
+    // simulate mobile
+    window.innerWidth = 600
+
+    const toc = new Toc({ target: document.body })
+
+    expect(toc.desktop).toBe(false)
+    expect(document.querySelector(`aside.toc ol li.active`)).toBeNull()
+
+    // open the ToC
+    toc.open = true
+
+    // active heading should be one of the last ones and should be scrolled into view
+    const active_li = doc_query(`aside.toc ol li.active`)
+    expect(active_li).toBeTruthy()
+    expect(active_li.textContent?.trim()).toBe(`Heading 100`)
+  })
 })