Skip to content

Product-bar follow-up: primary-button leak, brand link, CSS leftovers (post #146) #149

Description

@khagele

Two code-review passes over the merge of #146 (product bar) turned up a handful of things worth fixing in one follow-up PR.

User-visible

  • Primary button rule leaks. .step-panel button sets min-height: 38px and padding: 0 16px on every button in a step panel. The Step 1 entry tabs (Prefixes / Spam incident) override background and padding but not min-height, so they render 38px tall instead of 28px. A primary label that wraps to two lines in the 300px column sits flush against the top and bottom edge because the vertical padding is 0. Same construction in .intro-foot button. Expected: give the primary its own class (.button-primary, alongside .button-secondary / .button-muted) so tabs, step heads, candidate cards and the import row no longer have to out-specify it; restore vertical padding for wrapped labels.
  • Brand title navigates away in the same tab. The logo + "MeshCore Triangulator" link to dutchmeshcore.nl has no target="_blank" rel="noopener", unlike every other outbound link in the bar. Clicking it mid-resolve discards the locked cluster and edited weights.
  • Menu roles promise keyboard behaviour that isn't there. The language and theme menus keep role="menu" + menuitemradio + aria-haspopup="true" without arrow-key handling; the apps menu dropped role="menu" for exactly that reason. Make all three consistent disclosures.

AGENTS.md rules

  • Hardcoded color: #ffffff on the primary and .intro-foot button (§7 colours via tokens). --white is #f4f4f5, which is 4.27:1 on the fill and under AA, so a dedicated --on-accent token is needed.
  • Dropdown menus show/hide via .dd.open .dd-menu { display: block } instead of the hidden attribute (§7).

Dead code / stale comments

  • .topbar .dd-trig, .topbar-meta .is-icon and its :hover are declared twice at equal specificity; the later quiet-role block wins, so half the first block is dead.
  • .controls-card button.is-icon matches nothing (the PR's own comment says so) yet was edited 32px → 28px.
  • Unused tokens: --bar-h, --bar-bg (both themes), --heading-shadow (lost its only consumer in One top bar, one button system, and the review fixes #146).
  • body::before / body::after grid + scanline layers are at opacity 0 in both themes but still composited with mask + mix-blend-mode under the map canvas.
  • Stale comments: the removed .topbar { position: relative; z-index: 2 } comment, the "uppercased, with a hard drop shadow" heading comment contradicted by the rule below it, the phone bar "brand with GitHub" comment.
  • syncNavHeight is wired to both a ResizeObserver and a window resize listener; the latter is redundant when ResizeObserver exists.

Activity

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions