Keep a collapsed sidebar section collapsed on docs and components

954c9a4f7c7f · Devin AI · · parent d36e2dadd3e6

Keep a collapsed sidebar section collapsed on docs and components

The sidebar section hook forced open any section holding the row marked `aria-current` and recorded that as the reader's own choice. Every row on `/docs` and `/components` marks itself current, so reading a page re-opened the section that held it on every load and overwrote the cookie with `true`. The application shell has no current row inside a section, which is why a collapse survived there and nowhere else.

The hook now applies only what the reader chose and never writes a choice for them. A section holding the current page still opens for a reader who has not collapsed it, through the server-rendered seed, which the server already resolves against the reader's cookie on a cold GET and on a LiveView navigation.

`Plugs.SidebarSections` now states the scoping rule it always had: one namespace across every surface, keyed by the section title, with an explicit `id` for a section that must stand alone.

Fixes #28

Co-Authored-By: Christopher David <chris@openagents.com>
Co-Authored-By
Christopher David <chris@openagents.com>
Fixes
#28

Deploy story

What this commit did to the running system — joined from the forge receipt chain, the part a commit page elsewhere cannot show.

Not deployed through the forge lane

No push, promotion, build, or deploy receipt references this commit (receipts are scanned over a bounded recent window). Changes shipped by full node replacement carry their proof in the release gate receipt instead.

Changed files

  • added assets/test/sidebar_section_test.mjs
  • modified lib/openagents_web/components/layouts.ex
  • modified lib/openagents_web/plugs/sidebar_sections.ex
  • modified test/openagents_web/sidebar_state_test.exs

Diff

4 files changed, +278 -23

assets/test/sidebar_section_test.mjs added +135

@@ -0,0 +1,135 @@

1
// The rule this pins is a client rule, and it is the rule the docs and
2
// component-library sidebars used to break: what the reader collapsed wins,
3
// and the hook never records a choice on the reader's behalf. The server sends
4
// the same answer -- see `OpenAgentsWeb.SidebarStateTest` -- but only the
5
// browser decides what happens after a LiveView update repaints the seed.
6
import assert from "node:assert/strict"
7
import {execFileSync} from "node:child_process"
8
import {readdirSync, readFileSync} from "node:fs"
9
import {resolve} from "node:path"
10
import test from "node:test"
11
import {fileURLToPath} from "node:url"
12
13
const assetsDir = resolve(fileURLToPath(new URL("..", import.meta.url)))
14
const projectDir = resolve(assetsDir, "..")
15
const mixEnv = process.env.MIX_ENV ?? "test"
16
17
// The hook is colocated in `layouts.ex`, so the file to load is the one the
18
// compiler extracts. Compile first if this clone has not been built yet.
19
const hookPath = () => {
20
  const directory = resolve(
21
    projectDir,
22
    `_build/${mixEnv}/phoenix-colocated/openagents/OpenAgentsWeb.Layouts`,
23
  )
24
25
  const found = readdirSync(directory)
26
    .filter(entry => entry.endsWith(".js"))
27
    .map(entry => resolve(directory, entry))
28
    .find(path => readFileSync(path, "utf8").includes("sidebar_sections"))
29
30
  assert.ok(found, "no colocated hook in layouts.ex maintains sidebar_sections")
31
  return found
32
}
33
34
const loadHook = async () => {
35
  try {
36
    return (await import(hookPath())).default
37
  } catch (_missingBuild) {
38
    execFileSync("mix", ["compile"], {
39
      cwd: projectDir,
40
      encoding: "utf8",
41
      env: {...process.env, MIX_ENV: mixEnv},
42
      stdio: "pipe",
43
    })
44
    return (await import(hookPath())).default
45
  }
46
}
47
48
// Enough of a document for a hook that reads and writes one cookie.
49
const stubDocument = jar => {
50
  globalThis.document = {
51
    get cookie() {
52
      return Object.entries(jar)
53
        .map(([key, value]) => `${key}=${value}`)
54
        .join("; ")
55
    },
56
    set cookie(assignment) {
57
      const [pair] = assignment.split("; ")
58
      const separator = pair.indexOf("=")
59
      jar[pair.slice(0, separator)] = pair.slice(separator + 1)
60
    },
61
  }
62
}
63
64
const cookieValue = sections => encodeURIComponent(JSON.stringify(sections))
65
66
// A `<details>` with a summary and, optionally, the row marking the page the
67
// reader is on.
68
const stubSection = ({id, open, active}) => {
69
  const listeners = {}
70
71
  return {
72
    id,
73
    open,
74
    dataset: {},
75
    addEventListener: (name, handler) => (listeners[name] = handler),
76
    removeEventListener: name => delete listeners[name],
77
    querySelector: selector => {
78
      if (selector === "summary") return {addEventListener() {}, removeEventListener() {}}
79
      if (selector === "[aria-current]") return active ? {} : null
80
      return null
81
    },
82
    toggle(next) {
83
      this.open = next
84
      listeners.toggle?.()
85
    },
86
  }
87
}
88
89
const mount = async element => {
90
  const hook = await loadHook()
91
  const instance = Object.create(hook)
92
  instance.el = element
93
  instance.mounted()
94
  return instance
95
}
96
97
test("a collapsed section the reader is reading stays collapsed", async () => {
98
  const jar = {sidebar_sections: cookieValue({"sidebar-section-getting-started": false})}
99
  stubDocument(jar)
100
101
  // Painted open, as a LiveView update repainting the seed would leave it.
102
  const element = stubSection({id: "sidebar-section-getting-started", open: true, active: true})
103
  await mount(element)
104
105
  assert.equal(element.open, false)
106
  assert.deepEqual(JSON.parse(decodeURIComponent(jar.sidebar_sections)), {
107
    "sidebar-section-getting-started": false,
108
  })
109
})
110
111
test("the seed governs a section the reader has said nothing about", async () => {
112
  const jar = {}
113
  stubDocument(jar)
114
115
  const element = stubSection({id: "sidebar-section-issues", open: true, active: true})
116
  await mount(element)
117
118
  assert.equal(element.open, true)
119
  // Silence stays silence: an unwritten cookie is what lets the seed keep
120
  // deciding, including the seed that opens the section holding the page.
121
  assert.deepEqual(jar, {})
122
})
123
124
test("turning a caret records the reader's choice", async () => {
125
  const jar = {}
126
  stubDocument(jar)
127
128
  const element = stubSection({id: "sidebar-section-reference", open: true, active: false})
129
  await mount(element)
130
  element.toggle(false)
131
132
  assert.deepEqual(JSON.parse(decodeURIComponent(jar.sidebar_sections)), {
133
    "sidebar-section-reference": false,
134
  })
135
})
lib/openagents_web/components/layouts.ex modified +29 -23

@@ -396,8 +396,10 @@ defmodule OpenAgentsWeb.Layouts do

396 396
  patches rather than remounts, the element survives navigation and so does
397 397
  whatever the reader collapsed.
398 398
399
  `open` should be true for the section holding the current page: collapsing
400
  the section you are reading would hide your own location.
399
  `open` should be true for the section holding the current page, so a reader
400
  who has never touched this section lands with their own location visible.
401
  It is a seed: once the reader collapses a section, `state` carries that
402
  choice and it wins, even for the section they are reading.
401 403
  """
402 404
  attr :title, :string, required: true
403 405

@@ -414,7 +416,16 @@ defmodule OpenAgentsWeb.Layouts do

414 416
    painting the seed and having a hook correct it a frame later.
415 417
    """
416 418
417
  attr :id, :string, default: nil, doc: "defaults to a slug of the title"
419
  attr :id, :string,
420
    default: nil,
421
    doc: """
422
    Defaults to a slug of the title, which is also the key the reader's choice
423
    is stored under. Section ids are one namespace across every surface, so an
424
    identically titled section shares its collapsed state; pass an explicit id
425
    for a section that must stand alone. See
426
    `OpenAgentsWeb.Plugs.SidebarSections`.
427
    """
428
418 429
  slot :inner_block, required: true
419 430
420 431
  def sidebar_section(assigns) do

@@ -443,14 +454,12 @@ defmodule OpenAgentsWeb.Layouts do

443 454
        {render_slot(@inner_block)}
444 455
      </div>
445 456
    </details>
446
    <%!-- `open` above is a seed, not the truth. Re-applying it on every
447
    navigation collapses every section the reader opened but is not currently
448
    inside, and re-opens every section they closed that happens to be open by
449
    default. The hook makes the reader's own choices the state and keeps them
450
    in sessionStorage; the seed decides first paint, and a section containing
451
    the active row still opens regardless, so a page can never be hidden inside
452
    a section the reader had closed. With no JavaScript the seed governs and
453
    the sidebar still works. --%>
457
    <%!-- `open` above is a seed, not the truth. The server already resolves
458
    the reader's choice against it, so the seed decides the first paint only
459
    for a section the reader has never spoken about. The hook records each
460
    turn of a caret in the cookie and re-applies the reader's choice after a
461
    LiveView update, whose diff would otherwise repaint the seed. With no
462
    JavaScript the seed governs and the sidebar still works. --%>
454 463
    <script :type={Phoenix.LiveView.ColocatedHook} name=".SidebarSection">
455 464
      // A cookie rather than sessionStorage, because the server has to know.
456 465
      // Several sidebar destinations live in different live sessions, so

@@ -508,18 +517,15 @@ defmodule OpenAgentsWeb.Layouts do

508 517
          if (this.summary) this.summary.removeEventListener("click", this.onClick)
509 518
        },
510 519
        restore() {
511
          // A section holding the active page opens regardless of what the
512
          // reader last did, so navigation can never land on a hidden row.
513
          // Asked of the DOM rather than of the server: the active row already
514
          // marks itself `aria-current`, and a server-side answer has to be
515
          // derived from the section's `open` attribute, which is true for a
516
          // section that is merely open by default -- so every navigation
517
          // re-forced such a section open and undid the reader's collapse.
518
          if (this.el.querySelector("[aria-current]")) {
519
            this.el.open = true
520
            this.onToggle()
521
            return
522
          }
520
          // Only the reader's own choice is applied, and it is never written
521
          // back from here. Forcing the section that holds the active page
522
          // open instead -- and recording that as a choice -- is what made
523
          // /docs and /components forget a collapse the application shell
524
          // kept: every row on those surfaces marks itself `aria-current`, so
525
          // every reload re-opened the section being read and overwrote the
526
          // cookie with `true`. A section holding the current page still
527
          // opens for a reader who has not collapsed it, through the
528
          // server-rendered seed.
523 529
          const stored = read()[this.el.id]
524 530
          if (stored !== undefined) this.el.open = stored
525 531
        },
lib/openagents_web/plugs/sidebar_sections.ex modified +12

@@ -17,6 +17,18 @@ defmodule OpenAgentsWeb.Plugs.SidebarSections do

17 17
  The value is a reader's own UI state, so it is treated as untrusted input and
18 18
  bounded on the way in: unparseable JSON, a non-object, oversized content, or
19 19
  keys that are not section ids are dropped rather than passed inward.
20
21
  ## Scoping
22
23
  There is one namespace, shared by every surface, keyed by the section id that
24
  `OpenAgentsWeb.Layouts.sidebar_section/1` derives from the section title. A
25
  section named the same thing on two surfaces is therefore one preference:
26
  collapse it on `/docs` and it is collapsed on `/components` too. That is the
27
  rule on purpose -- a reader who hides a group of links is talking about the
28
  group, not about the page they happened to be on -- and
29
  `test/openagents_web/sidebar_state_test.exs` pins it. A surface that needs a
30
  section of its own passes an explicit `id` rather than relying on its title
31
  being unique.
20 32
  """
21 33
22 34
  @behaviour Plug
test/openagents_web/sidebar_state_test.exs modified +102

@@ -121,6 +121,89 @@ defmodule OpenAgentsWeb.SidebarStateTest do

121 121
    end
122 122
  end
123 123
124
  # The docs and component-library surfaces are where this used to fail. Their
125
  # rows mark themselves `aria-current`, and the hook forced any section
126
  # holding such a row open and wrote that back to the cookie, so the reader's
127
  # collapse survived on the application shell -- whose sections hold no
128
  # current row -- and was erased on `/docs` and `/components` on every reload.
129
  describe "collapsed sections survive on the docs and component surfaces" do
130
    test "a cold GET of a docs page paints the reader's collapsed sections collapsed", %{
131
      conn: conn
132
    } do
133
      html =
134
        conn
135
        |> put_req_cookie("sidebar_sections", ~s({"sidebar-section-transparency":false}))
136
        |> get(~p"/docs/welcome")
137
        |> html_response(200)
138
139
      refute html =~ ~r/id="sidebar-section-transparency"[^>]*\sopen/
140
    end
141
142
    test "the section holding the page being read stays collapsed if the reader closed it", %{
143
      conn: conn
144
    } do
145
      html =
146
        conn
147
        |> put_req_cookie("sidebar_sections", ~s({"sidebar-section-getting-started":false}))
148
        |> get(~p"/docs/welcome")
149
        |> html_response(200)
150
151
      refute html =~ ~r/id="sidebar-section-getting-started"[^>]*\sopen/
152
    end
153
154
    test "with no cookie the section holding the page being read is open", %{conn: conn} do
155
      html = conn |> get(~p"/docs/welcome") |> html_response(200)
156
157
      assert html =~ ~r/id="sidebar-section-getting-started"[^>]*\sopen/
158
    end
159
160
    test "a navigation within docs keeps the collapse", %{conn: conn} do
161
      conn = put_req_cookie(conn, "sidebar_sections", ~s({"sidebar-section-issues":false}))
162
163
      {:ok, view, _html} = live(conn, ~p"/docs/welcome")
164
      html = render_patch(view, ~p"/docs/issues")
165
166
      refute html =~ ~r/id="sidebar-section-issues"[^>]*\sopen/
167
    end
168
169
    test "the component library honours a collapse the same way", %{conn: conn} do
170
      html =
171
        conn
172
        |> put_req_cookie("sidebar_sections", ~s({"sidebar-section-reference":false}))
173
        |> get(~p"/components/icons")
174
        |> html_response(200)
175
176
      refute html =~ ~r/id="sidebar-section-reference"[^>]*\sopen/
177
    end
178
179
    # The behavioural half of the above is not renderable from here: the force
180
    # was in the colocated hook, which only runs in a browser. Pinned at the
181
    # source, the same way the viewport defaults are.
182
    test "the hook applies the reader's choice and never writes one for them" do
183
      source = File.read!("lib/openagents_web/components/layouts.ex")
184
185
      refute source =~ ~s([aria-current]),
186
             "the hook forces the section holding the active row open again"
187
    end
188
  end
189
190
  # Scoping is a decision, not an accident of naming: one namespace for every
191
  # surface, keyed by the section title. A section called the same thing in two
192
  # sidebars is one preference.
193
  describe "section ids are one namespace across surfaces" do
194
    test "the id comes from the title alone, not from the surface", %{conn: conn} do
195
      html = conn |> get(~p"/docs") |> html_response(200)
196
197
      assert html =~ ~s(id="sidebar-section-getting-started")
198
      assert standalone_section_id("Getting started") == "sidebar-section-getting-started"
199
    end
200
201
    test "an explicit id is how a section stands alone" do
202
      assert standalone_section_id("Getting started", id: "sidebar-section-gallery-demo") ==
203
               "sidebar-section-gallery-demo"
204
    end
205
  end
206
124 207
  # The Issues and Projects rows used to be repository-scoped: they took the
125 208
  # `:owner`/`:repo` of whatever page you were on, and fell back to the first
126 209
  # repository in your workspace alphabetically. The same row therefore led

@@ -274,6 +357,25 @@ defmodule OpenAgentsWeb.SidebarStateTest do

274 357
    end
275 358
  end
276 359
360
  # The section rendered on its own, so the id it derives can be read without a
361
  # surface around it.
362
  defp standalone_section_id(title, options \\ []) do
363
    html =
364
      render_component(&OpenAgentsWeb.Layouts.sidebar_section/1,
365
        title: title,
366
        id: options[:id],
367
        inner_block: [
368
          %{
369
            __slot__: :inner_block,
370
            inner_block: fn _changed, _arguments -> "row" end
371
          }
372
        ]
373
      )
374
375
    [_match, id] = Regex.run(~r/<details[^>]*\sid="([^"]+)"/, html)
376
    id
377
  end
378
277 379
  defp parse(value) do
278 380
    :get
279 381
    |> Plug.Test.conn("/")

This page updates live while a promote is in flight · changelog