Files
worldmonitor/todos/040-pending-p3-font-settings-toggleattribute-simplification.md
Elie Habib 110ab402c4 feat(intelligence): analytical framework selector for AI panels (#2380)
* feat(frameworks): add settings section and import modal

- Add Analysis Frameworks group to preferences-content.ts between Intelligence and Media sections
- Per-panel active framework display (read-only, 4 panels)
- Skill library list with built-in badge, Rename and Delete actions for imported frameworks
- Import modal with two tabs: From agentskills.io (fetch + preview) and Paste JSON
- All error cases handled inline: network, domain validation, missing instructions, invalid JSON, duplicate name, instructions too long, rate limit
- Add api/skills/fetch-agentskills.ts edge function (proxy to agentskills.io)
- Add analysis-framework-store.ts (loadFrameworkLibrary, saveImportedFramework, deleteImportedFramework, renameImportedFramework, getActiveFrameworkForPanel)
- Add fw-* CSS classes to main.css matching dark panel aesthetic

* feat(panels): wire analytical framework store into InsightsPanel, CountryDeepDive, DailyMarketBrief, DeductionPanel

- InsightsPanel: append active framework to geoContext in updateFromClient(); subscribe in constructor, unsubscribe in destroy()
- CountryIntelManager: pass framework as query param to fetchCountryIntelBrief(); subscribe to re-open brief on framework change; unsubscribe in destroy()
- DataLoaderManager: add dailyBriefGeneration counter for stale-result guard; pass frameworkAppend to buildDailyMarketBrief(); subscribe to framework changes to force refresh; unsubscribe in destroy()
- daily-market-brief service: add frameworkAppend? field to BuildDailyMarketBriefOptions; append to extendedContext before summarize call
- DeductionPanel: append active framework to geoContext in handleSubmit() before RPC call

* feat(frameworks): add FrameworkSelector UI component

- Create FrameworkSelector component with premium/locked states
- Premium: select dropdown with all framework options, change triggers setActiveFrameworkForPanel
- Locked: disabled select + PRO badge, click calls showGatedCta(FREE_TIER)
- InsightsPanel: adds asterisk note (client-generated analysis hint)
- Wire into InsightsPanel, DailyMarketBriefPanel, DeductionPanel (via this.header)
- Wire into CountryDeepDivePanel header right-side (no Panel base, panel=null)
- Add framework-selector CSS to main.css

* fix(frameworks): make new proto fields optional in generated types

* fix(frameworks): extract firstMsg to satisfy strict null checks in tsconfig.api.json

* fix(docs): add blank lines around lists/headings to pass markdownlint

* fix(frameworks): add required proto string fields to call sites after make generate

* chore(review): add code review todos 041-057 for PR #2380

7 review agents (TypeScript, Security, Architecture, Performance,
Simplicity, Agent-Native, Learnings) identified 17 findings across
5 P1, 8 P2, and 4 P3 categories.
2026-03-27 23:36:44 +04:00

46 lines
1.5 KiB
Markdown

---
status: pending
priority: p3
issue_id: "040"
tags: [code-review, quality, font, simplification]
dependencies: []
---
# `applyFont()` if/else can be replaced with `toggleAttribute`
## Problem Statement
The current `applyFont()` uses an if/else with asymmetric operations (`dataset.font = 'system'` vs `delete dataset.font`).
This introduces a string coupling between TS (`'system'`) and CSS (`[data-font="system"]`).
`toggleAttribute` is the idiomatic DOM API for boolean attribute toggling and collapses 5 lines to 1.
## Proposed Solution
### Option A: Use `toggleAttribute` + boolean CSS selector
```ts
export function applyFont(font?: FontFamily): void {
const resolved = font ?? getFontFamily();
document.documentElement.toggleAttribute('data-font-system', resolved === 'system');
}
```
CSS changes to:
```css
[data-font-system] {
--font-body-base: system-ui, -apple-system, 'Segoe UI', Roboto, 'Helvetica Neue', Arial, sans-serif;
}
```
**Pros:** No string value coupling, 1 line, idiomatic | **Cons:** Requires CSS selector rename | **Effort:** Small | **Risk:** Low
### Option B: Keep as-is
The if/else is not harmful, just not minimal. Accept current form.
## Technical Details
- File: `src/services/font-settings.ts`, `src/styles/main.css`
- PR: koala73/worldmonitor#2318
## Acceptance Criteria
- [ ] `applyFont()` is one expression
- [ ] No string value coupling between TS and CSS selector
## Work Log
- 2026-03-27: Identified during PR #2318 review via code-simplicity-reviewer