fix(mobile-nav): give Server Settings a first-class mobile entry point (lr-87ca)#298
fix(mobile-nav): give Server Settings a first-class mobile entry point (lr-87ca)#298clagentic-builder[bot] wants to merge 1 commit into
Conversation
…t (lr-87ca) Root cause: #server-settings-btn lives inside #top-bar, which is display:none at <=768px. The mobile More sheet opened Server Settings by synthetic-clicking that hidden button, which worked but was not a discoverable first-class nav path. - server-settings.js: export openServerSettings(), mirroring the existing openProjectSettings() pattern. - sidebar-mobile.js: import and call openServerSettings() directly instead of dispatching a synthetic click on the hidden desktop button. Once open, per-section navigation was already shared code between desktop and mobile (SETTINGS_SECTIONS drives both the desktop sidebar and the mobile settings palette), so no separate per-section fix was needed — only the entry point was broken. Adds regression coverage and documents the parity rule in docs/guides/architecture.md.
![clagentic-reviewer[bot]](https://avatars.githubusercontent.com/in/3944489?s=60&v=4){kind=small}
There was a problem hiding this comment.
PEACHES — clean
No blocking issues. This PR correctly implements the mobile entry point for Server Settings with full parity:
openServerSettings()properly exported (server-settings.js:564) and mirrorsopenProjectSettings()pattern- Desktop button path preserved via event listener (server-settings.js:59-61); mobile path now first-class via direct function call (sidebar-mobile.js:1136)
- Mobile palette (settings-nav-pill → openSettingsPalette) renders from shared SETTINGS_SECTIONS registry (server-settings.js:20-37, 410)
- CSS media query block complete (@media max-width:768px, server-settings.css:746-804); no desktop-only sections
- Architecture.md documents the parity rule and fix pattern (lines 300-308)
- Regression tests verify: export exists, import wired, synthetic click removed, custom-icons remains in registry (test/server-settings-mobile-parity-lr-87ca.test.js)
- Brand rule compliant: "Server Settings" label (not bare "clagentic")
Audit claim verified: once the panel opens, per-section nav and layout are fully shared — SETTINGS_SECTIONS drives both desktop nav sidebar and mobile settings palette. No parity gaps found.
|
Abandoned as wrong-target per Andy directive (task lr-87ca, holden as task lead on Andy's explicit instruction). This PR added a new mobile entry point for Server Settings, but Server Settings was already reachable on mobile -- the entry point was never the reported problem. The actual bug (Custom Icons section not visible inside Server Settings on mobile) is unaddressed by this PR; static-code review shows the section IS registered for mobile (getVisibleSections includes custom-icons), so the real cause lies elsewhere and needs on-device diagnosis, not this change. Closing without merge; see lr-87ca for the corrected scope and follow-up. |
Summary
Fixes lr-87ca: Server Settings did not have a first-class discoverable mobile nav entry point, only a functional-but-hidden workaround.
Root cause (confirmed by lead, andy comment 2026-07-01)
#server-settings-btnlives inside#top-bar, which isdisplay:noneat <=768px (lib/public/css/title-bar.css). The mobile More sheet (lib/public/modules/sidebar-mobile.js) opened Server Settings by synthetic-clicking that hidden button (document.getElementById("server-settings-btn").click()). This worked functionally but left every Server Settings section undiscoverable as a first-class mobile nav path.What changed
openServerSettings(), a public entry point independent of the desktop button element. Mirrors the existingopenProjectSettings()pattern already used by Project Settings.openServerSettings()directly instead of dispatching a synthetic click on the hidden desktop button.Per-section parity audit result
Once the panel is open, per-section navigation was already shared code between desktop and mobile -- this was NOT part of the reported defect, only the entry point was broken:
VERIFY checklist (per brief)
Task ID
lr-87ca
Test status
npm test: 591 passed, 1 failed (test/settings-preflight-lr-1a26.test.js, subtest 'validateSettingsObject: multiple unknown hook keys produce one warning each' -- pre-existing failure named explicitly in the dispatch brief as known/out of scope; unrelated to this change, confirmed unaffected before and after this diff).
Scope note
Brief also mentioned auditing User Settings mobile parity as background context (lr-0847), but andy's hardened scope comment on lr-87ca restricts this task to Server Settings specifically. User Settings has no mobile More-sheet entry today -- flagging as a possible followup, not fixed here (out of hardened scope).