ADR — PR #988

2026-05-20 10:00 — Update SettingsShell nav highlighting to use resolvedActiveId

Comment: https://github.com/dboone31/erp-unlocked/pull/988#discussion_r1
Priority: must-fix
Decision: modify
Rationale: Ensured nav highlighting and aria-current use the resolved active section ID for correct accessibility.

Comment: https://github.com/dboone31/erp-unlocked/pull/988#discussion_r2
Priority: must-fix
Decision: modify
Rationale: Moved hiddenSections logic into frontmatter to prevent parsing errors.

Comment: https://github.com/dboone31/erp-unlocked/pull/988#discussion_r123456789
Priority: must-fix
Decision: modify
Rationale: Fixed nav highlighting by using resolvedActiveId consistently for aria-current and active state checks.

Comment: https://github.com/dboone31/erp-unlocked/pull/988#discussion_r123456790
Priority: must-fix
Decision: modify
Rationale: Placed hiddenSections declaration inside the Astro frontmatter block to avoid parsing errors.

Comment: https://github.com/dboone31/erp-unlocked/pull/988#pullrequestreview-
Priority: must-fix
Decision: modify
Rationale: Completed required nav highlight and hiddenSections fixes per CodeRabbit feedback.
Comment: https://github.com/dboone31/erp-unlocked/pull/988#discussion_r123456790
Priority: must-fix
Decision: modify
Rationale: Placed hiddenSections declaration inside the Astro frontmatter block to avoid parsing errors.

Comment: N/A (proactive fix based on project-wide “Recurring CodeRabbit findings”)
Priority: nice-to-have
Decision: accept
Rationale: Following the project mandate to prefer Nanostores over useState for state management. Created settingsStore.ts and moved activeId into a shared atom to align with architectural standards and ensure consistency with sibling components.

Comment: CodeRabbit — SettingsShell nav highlighting (resolved active section id)
Priority: must-fix
Decision: accept
Rationale: The nav highlighting was using the original activeId even when the resolved active section falls back, causing no item to receive aria-current=“page” and potentially breaking accessibility.

Comment: CodeRabbit — settings.astro hiddenSections frontmatter placement
Priority: must-fix
Decision: accept
Rationale: The declaration was incorrectly placed outside the Astro frontmatter block, which would break parsing during page compile.

Comment: CodeRabbit — SettingsShell nav highlighting (resolved active section id) — verification pass
Priority: must-fix
Decision: accept
Rationale: Verified that nav highlighting already uses resolvedActiveId consistently for both aria-current and active state checks, ensuring accessibility and visual state remain in sync.

Comment: CodeRabbit — settings.astro hiddenSections frontmatter placement — verification pass
Priority: must-fix
Decision: reject
Rationale: The hiddenSections declaration is already correctly placed within the Astro frontmatter block in the current branch. No action required.

Comment: CodeRabbit second review on PR #988
Priority: nice-to-have
Decision: accept
Rationale: The ?? SECTIONS[0] fallback could reintroduce a section that was explicitly hidden when all visible sections are filtered out. Changed to ?? null with explicit empty-state handling in the content area.

Comment: CodeRabbit second review on PR #988
Priority: nice-to-have
Decision: accept
Rationale: The ADR contained placeholder links (hypothetical link for ...) that were not valid GitHub URLs. Replaced with real PR review and files URLs.

Comment: CodeRabbit — SettingsShell variable naming for resolved active section
Priority: must-fix
Decision: modify
Rationale: Renamed the resolved active section variable in SettingsShell (and aligned references to hiddenSections filtering) so the variable name accurately reflects that it represents the resolved active section ID after hiddenSections filtering — improving readability and making the relationship between hiddenSections and the active section explicit.