Conversation
Review — PR #5253 "add searchbar for settings" (round 1)Reviewer: Claude Opus 5 (1M context), Scope: Verified locally on a detached worktree of
No schema, migration, Hasura, auth or installer surface is touched; the Findings
F1 — medium — F2 — medium — filter state goes stale when the navigation re-renders.
F3 — medium — the JS/DOM approach duplicates an idiom the repo already has in C#. <input id="searchOwner" type="text" class="form-control me-1" @bind="OwnerSearchTerm" @bind:event="oninput"
placeholder="@userConfig.GetText("search")" aria-label="@userConfig.GetText("search")" />A F4 — medium — the filtering algorithm has zero test coverage. F5 — medium — the JS depends on a markup contract nothing enforces, and
No test or assertion protects the contract, and the contract is not documented in the razor file. F6 — medium — background-image: linear-gradient(var(--bg-color, #054B8C), var(--bg-color-2, #03335E));The sidebar container itself ( F7 — medium — every keystroke costs a server round-trip, undebounced. F8 — low — external users now get a rendered-but-empty <li class="nav-item px-2">
@if (userConfig.User.Dn.EndsWith(GlobalConst.kLdapInternalPostfix))
{
<NavLink class="nav-link" href="settings/password"> ... </NavLink>
}
</li>For an external (non-internal-LDAP) user this renders an empty list item — announced by screen readers as a blank list entry. Moving the F9 — low — F10 — low — separators are dropped wholesale while searching. F11 — low — no help content for a UI feature change. F12 — low — F13 — low — copy-pasted, unused setup and a magic number in the new test. layout.WaitForAssertion(() => Assert.That(GetNavbarHeightSubscriberCount(eventService), Is.EqualTo(1)));
await layout.InvokeAsync(() => eventService.InvokeNavbarHeightChanged(50));Nothing the test asserts depends on either line, and F14 — low — no feedback when nothing matches. F15 — low — no diacritic folding. Security passNo security findings. Concretely, for the code reachable from this diff:
Not raised as a finding: RecommendationsShould fix before merge
Nice to have
Process disclosures
|
|



refs #5146
Adds a search field above the settings sidebar navigation that filters the settings pages by their localized labels. Matching is case and diacritic insensitive; a matching chapter heading keeps all of its pages. Role visibility is unchanged and still owned by
ExecutionModeAuthorizeView.Note: issue #5146 asks for full-text search across settings page content with navigable results. This PR only filters the navigation labels, so the issue stays open.