Skip to content

Follow-up to #248: Affinity Debug toggle broken + reversed carets; group-row aria-expanded on mobile #254

Description

@dborup

Follow-up from the review of #248 (#189 group caret, merged as c12aa60b). Both items were outside that PR's scope. Line numbers refer to master c12aa60b.

1. Node page "Affinity Debug" toggle cannot open, and its carets are reversed

public/nodes.js:920: the <h4> heading of the #node-affinity-debug card has an inline onclick. That onclick is a double-quoted attribute, but it contains the Phosphor sprite markup with its own double quotes (class="ph-icon", aria-hidden="true"). The browser therefore cuts the attribute off at the first inner ". The handler that remains is not valid JavaScript (reported as "Invalid or unexpected token"), so clicking the heading does nothing, and the debug body stays hidden.

The break came with the emoji-to-Phosphor migration (30627454, upstream #1648 M2), the same change that caused #189.

The carets also contradict the convention #248 chose (collapsed = caret-right, expanded = caret-down): the toggle shows caret-down while collapsed and caret-up while open.

The card is only shown when debugAffinity is enabled (client config or the local-storage flag), which is probably why nobody noticed.

Expected:

  • the heading toggles the debug body;
  • collapsed shows caret-right and expanded shows caret-down;
  • the toggle is a delegated handler instead of an inline onclick with nested markup;
  • ideally the heading is a <button> with aria-expanded, or follows the same pattern as the packets group rows.

Tests:

  • a unit test that renders the card and asserts that the toggle opens and closes and the caret follows;
  • a mutant check: put the old inline markup back and the test must fail.

2. Packets group rows announce aria-expanded on mobile, but activation selects instead

public/packets.js:2444 (from #248) sets aria-expanded on multi-observation group rows. On mobile, public/mobile-page-actions.js (upstream #1461 #7) rewrites data-action="toggle-select" to select-hash on click, and the expand column is hidden. A screen-reader user therefore hears "collapsed" on a row that opens the detail pane instead of expanding.

This is a minor a11y issue and not a blocker. Options:

  • drop aria-expanded (or render the row with select-hash) under the mobile breakpoint, so the announced state matches what activation does;
  • or keep the attribute and document why.

Tests: a narrow-viewport E2E (or a unit test with isMobile() stubbed) asserting that the row has no aria-expanded when activation selects.

Not in scope

Evidence

Activity

  1. added
    bugSomething isn't working
    type:bugSomething broken
    on Oct 5, 2026
  2. dborup-agent commented on Oct 5, 2026

    @dborup-agent
    Collaborator

    Plan — CS-pve-agent3 #254

    Branch codex/issue-254-affinity-toggle-mobile-aria from origin/master c12aa60b. Tests first (red), then the fix. One draft PR, "Relates to #254".

    1. Affinity Debug toggle (public/nodes.js:920)

    • Move the card markup out of the loadFullNode template into a small pure renderer, renderAffinityDebugCard(). The heading holds a real <button type="button" class="affinity-debug-toggle" aria-expanded="false" aria-controls="affinityDebugBody">. The body gets id="affinityDebugBody" and the hidden attribute instead of style="display:none".
    • Caret per the fix(packets): collapsed groups show a right caret, expanded a down caret (#189) #248 convention: collapsed ph-caret-right, expanded ph-caret-down, from one helper (affinityDebugCaretHtml(expanded)), so the initial render and the toggle cannot disagree.
    • toggleAffinityDebug(btn, doc) flips aria-expanded, the body's hidden and the caret. One delegated click listener on #nodeFullBody is registered once in init(). It is not registered in loadFullNode, which re-runs on theme-refresh and would stack listeners. The <button> gives Enter and Space for free.
    • No inline handler, no nested markup in an attribute. The button style resets to the heading look with inherit (font, color), plus a cursor rule in style.css. No new colours.
    • The fetch to /api/debug/affinity and its rendering are unchanged.

    2. Mobile group rows: render the row as select-hash under the mobile breakpoint

    Choice: option 2, render the row as select-hash on mobile, rather than only dropping the attribute.

    Why not just drop aria-expanded: the Kpa-clawbot#1461 #7 redirect only catches click. On mobile with a keyboard, Enter/Space still go through toggle-select and expand the group. Dropping only the attribute would leave a row that expands with no announced state. Rendering select-hash makes every activation path (tap, screen-reader activation, Enter, Space) select, which matches what a mobile tap already does today.

    3. Tests and mutants

    • Unit, new test-issue-254-affinity-debug-toggle.js (vm sandbox on the real nodes.js, registered in test-all.sh):
      • the card renders a button with aria-expanded="false", aria-controls pointing at the hidden body, caret-right and no onclick;
      • the toggle opens (true, body shown, caret-down) and closes again.
    • Unit, test-packets.js:
      • at innerWidth 390 with MobilePageActions stubbed, a multi-observation row is select-hash without aria-expanded;
      • at 1400 it stays toggle-select with aria-expanded;
      • single-observation rows are unchanged.
    • E2E, new test-issue-254-affinity-toggle-mobile-aria-e2e.js (registered in deploy.yml next to the test: 13 red orphan unit tests + 4 orphan E2E files left after #187; collapsed packet groups show an up-caret #189 line):
      • Node page with meshcore-affinity-debug=true, and /api/debug/affinity mocked through page.route (no request reaches the server and no API key is used). The card is visible, the body is hidden, then click → open/caret-down/true, click → closed/caret-right/false, then keyboard Enter.
      • The seeded Packets page collapse button in the left column of the table opens the dialog. Kpa-clawbot/CoreScope#1486 group at 1400 px: toggle-select + aria-expanded.
      • The same group at 390 px (touch): select-hash, no aria-expanded, and a tap opens the detail instead of expanding.
      • A 1400 → 390 → 1400 resize updates the row.
    • Mutants (each against the unit tests and the E2E, then restored):
      • M1, the old inline onclick markup;
      • M2, reversed carets;
      • M3, aria-expanded back on mobile;
      • M4, aria-expanded dropped on desktop;
      • plus the resize re-render removed.
    • Suites: sh test-all.sh, node test-frontend-helpers.js, node test-packets.js; test-issue-189-group-caret-e2e.js, #1486, #1461 and the other packets and nodes E2Es against a local Go server on e2e-fixture.db, prepared as in CI.
    • Gates: scripts/check-xss-sinks.sh --diff, no hardcoded colours, fork guards stay 9 + 1.

    Browser validation

    Screenshots of the Affinity Debug card collapsed and expanded in light and dark, and of the packets list at 390 px.

    Customizer

    No new configurable values.

  3. added a commit that references this issue on Oct 5, 2026
  4. dborup commented on Oct 5, 2026

    @dborup
    OwnerAuthor

    Fixed by #255 (merged as 3bb2cb89).

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingtype:bugSomething broken

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions