From 5a1f69937919986f64bff87daec1860648bf8fa2 Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 19 May 2026 01:37:19 +0200 Subject: [PATCH] fix(settings): Remote nav now scrolls to the right section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Remote Control nav item carried id: '@iptvnator/ui/remote-control' — the NX library name pasted in place of the section's HTML id. The settings-section-scroll directive resolves the target with document.getElementById(), which returned null for that string, so clicking "Remote" silently no-op'd. The active-state binding ([class.settings-group--active]="activeSection() === 'remote-control'") also never lit up because the nav reported a different id than the section template uses. Change the nav id to 'remote-control' so it matches the section's id="remote-control" attribute. Added a focused regression spec (settings-options.spec.ts) that: - asserts the desktop-only items light up only when isDesktop is true, - reads each section component's root
at test time and cross-checks the nav id set against it, so any future drift between the two (renamed section, new section without nav entry, copy-paste of a library path into the id) fails the build. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/app/settings/settings-options.spec.ts | 78 +++++++++++++++++++ apps/web/src/app/settings/settings-options.ts | 7 +- 2 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 apps/web/src/app/settings/settings-options.spec.ts diff --git a/apps/web/src/app/settings/settings-options.spec.ts b/apps/web/src/app/settings/settings-options.spec.ts new file mode 100644 index 000000000..7ac76cf97 --- /dev/null +++ b/apps/web/src/app/settings/settings-options.spec.ts @@ -0,0 +1,78 @@ +import { readFileSync, readdirSync } from 'fs'; +import { resolve } from 'path'; + +import { buildSettingsSectionNavItems } from './settings-options'; + +/** + * Every nav-item id must match the `id="..."` attribute on its + * corresponding section component template, otherwise clicking the nav + * link silently no-ops (the scroll directive runs `document.getElementById` + * and falls through when no element is found). + * + * This bug bit the Remote section once already — its nav id was set to + * the Nx library name (`@iptvnator/ui/remote-control`) instead of the + * template's `id="remote-control"`. The guard below catches future + * rename/copy-paste regressions before they ship. + */ +describe('buildSettingsSectionNavItems', () => { + // Anchor on the Nx workspace root (Jest runs from there); avoids + // depending on __dirname which isn't defined under the project's + // ESM Jest preset. + const settingsDir = resolve( + process.cwd(), + 'apps/web/src/app/settings' + ); + + function collectSectionTemplateIds(): Set { + const ids = new Set(); + for (const fileName of readdirSync(settingsDir)) { + if (!/^settings-.+-section\.component\.html$/.test(fileName)) { + continue; + } + const html = readFileSync( + resolve(settingsDir, fileName), + 'utf-8' + ); + // Match the FIRST `id="…"` on the
root only — + // descendant elements (form controls, anchors) also use id= + // attributes and would pollute the set. + const rootMatch = /]*\sid="([^"]+)"/i.exec(html); + if (rootMatch) { + ids.add(rootMatch[1]); + } + } + return ids; + } + + it('exposes desktop-only items only when isDesktop is true', () => { + const desktopItems = buildSettingsSectionNavItems(true); + const pwaItems = buildSettingsSectionNavItems(false); + + expect(desktopItems.map((item) => item.id)).toEqual( + expect.arrayContaining(['epg', 'remote-control']) + ); + expect(pwaItems.find((item) => item.id === 'epg')?.visible).toBe( + false + ); + expect( + pwaItems.find((item) => item.id === 'remote-control')?.visible + ).toBe(false); + }); + + it('every nav id matches an existing section template id (regression: remote-control nav no longer maps to the Nx lib name)', () => { + const navIds = new Set( + buildSettingsSectionNavItems(true).map((item) => item.id) + ); + const templateIds = collectSectionTemplateIds(); + + // Every nav id must exist as a section root id. + const orphans = [...navIds].filter((id) => !templateIds.has(id)); + expect(orphans).toEqual([]); + + // And every section template id must be reachable from the nav + // (catches the opposite drift — a new section added without a nav + // entry would never get scrolled to). + const unreachable = [...templateIds].filter((id) => !navIds.has(id)); + expect(unreachable).toEqual([]); + }); +}); diff --git a/apps/web/src/app/settings/settings-options.ts b/apps/web/src/app/settings/settings-options.ts index 8c509f186..93e8bb84d 100644 --- a/apps/web/src/app/settings/settings-options.ts +++ b/apps/web/src/app/settings/settings-options.ts @@ -108,7 +108,12 @@ export function buildSettingsSectionNavItems( visible: isDesktop, }, { - id: '@iptvnator/ui/remote-control', + // Must match the section's HTML id (`remote-control`) so the + // settings-section-scroll directive can resolve the anchor. + // Was previously '@iptvnator/ui/remote-control' (the NX lib + // name), which meant clicking the nav item silently no-op'd + // because document.getElementById of that string returned null. + id: 'remote-control', label: 'SETTINGS.NAV_REMOTE', icon: 'smartphone', visible: isDesktop,