refactor: split concepts.page into 6 sections, fix dead highlighting (RD-24)
The page held six teaching sections and a 142-line `styles:` block, at 471 effective lines against a limit of 250. It is now 36 lines of composition. Angular scopes a component's CSS to markup that component rendered, so the split had to move each rule to its owner. `concept-card` owns the card vocabulary and renders it. `.app-code`, `.app-lead`, `.app-cols` and `.app-note` become globals, because their targets are projected or arrive through `[innerHTML]`. That constraint exposed a live bug. The syntax-highlighting rules compiled to `pre[_ngcontent-%COMP%] .k[_ngcontent-%COMP%]`, but `highlight-ts` injects the `.k`/`.s`/`.c` spans through `[innerHTML]`, so they carry no scope attribute and the rule never matched. Keywords, strings and comments have always rendered in the plain foreground colour. The rules are global now, on five new `--app-code-*` tokens. Widen the colour guard while here: it scanned only `*.component.ts`, so every `*.page.ts`, `*.section.ts` and `*.step.ts` was invisible to it. That is how this page collected 21 hardcoded colours. One other file needed a fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Sonnet 5
@@ -0,0 +1,230 @@
|
||||
# RD-24 — Split `concepts.page.ts`, and fix the highlighting it has never rendered
|
||||
|
||||
Status: done
|
||||
Source: PLAN.md 3g, order step 6
|
||||
|
||||
## Why
|
||||
|
||||
`concepts.page.ts` measures ~471 effective lines against a limit of 250, and carries
|
||||
`/* eslint-disable max-lines */`. It is one template with six teaching sections and a 142-line
|
||||
`styles:` block.
|
||||
|
||||
A per-section split alone does not fix the styles, because Angular scopes a component's CSS to
|
||||
its own template. Splitting without moving the CSS by owner would leave every section unstyled.
|
||||
|
||||
**And measuring that constraint turned up a live bug** (decision 1).
|
||||
|
||||
## Read first
|
||||
|
||||
- `apps/ssp/src/app/showcase/concepts.page.ts` — the whole file: styles at 48-189, template at
|
||||
190-439.
|
||||
- `libs/shared/styles.scss:105-130` — the `--app-devpanel-*` tokens and the existing
|
||||
`.app-stack` / `.app-section` / `.app-text-subtle` globals. The new globals go beside them,
|
||||
and the file's own comment says it exists to centralise exactly these idioms.
|
||||
- `apps/ssp/src/app/showcase/highlight-ts.ts:42-45` — the `<span class="k|s|c">` markup whose
|
||||
colours decision 1 restores.
|
||||
- `scripts/check-tokens.sh:14` — the guard, and its `--include` glob.
|
||||
|
||||
## Decisions (pre-made, don't relitigate)
|
||||
|
||||
1. **The syntax highlighting is dead today. Fix it by making those rules global.** Verified
|
||||
against the built output, not inferred:
|
||||
|
||||
```
|
||||
pre[_ngcontent-%COMP%] .k[_ngcontent-%COMP%]{color:#c792ea}
|
||||
```
|
||||
|
||||
The `.k`/`.s`/`.c` spans arrive through `[innerHTML]`, so they never carry an `_ngcontent`
|
||||
attribute, and the rule cannot match. `highlight-ts.ts` computes the spans, its spec passes,
|
||||
and every keyword, string and comment renders in the plain foreground colour. No component in
|
||||
this repository uses `ViewEncapsulation.None`, and there is no global rule for `.k`, `.s` or
|
||||
`.c`.
|
||||
|
||||
So `.app-code .k|.s|.c` becomes **global**, in `libs/shared/styles.scss`. A component cannot
|
||||
own a rule that targets markup it did not render.
|
||||
|
||||
2. **Five new tokens, beside `--app-devpanel-*`**, which exist for this same reason:
|
||||
|
||||
```scss
|
||||
--app-code-bg: #1e2430;
|
||||
--app-code-fg: #e6e9ef;
|
||||
--app-code-keyword: #c792ea;
|
||||
--app-code-string: #c3e88d;
|
||||
--app-code-comment: #7e8aa0;
|
||||
```
|
||||
|
||||
`styles.scss` is the token bridge and the guard's one exempt file, so these literals belong
|
||||
here and nowhere else.
|
||||
|
||||
3. **Four new globals in `libs/shared/styles.scss`**, named with the existing `.app-` prefix:
|
||||
|
||||
| Global | Replaces | Why not a component |
|
||||
| ----------- | -------- | ------------------------------------------------------------------------------------------------------------------------------ |
|
||||
| `.app-code` | `pre` | must style `[innerHTML]` children (decision 1) |
|
||||
| `.app-lead` | `.lead` | a page-level typography idiom, used by all six sections |
|
||||
| `.app-cols` | `.cols` | same |
|
||||
| `.app-note` | `.note` | its content includes markup (`<code>`), so it must be projected, and projected content keeps the _declaring_ component's scope |
|
||||
|
||||
**Delete `.section` entirely** — the global `.app-section` already exists and does the job.
|
||||
|
||||
4. **`concept-card.component.ts` owns the card vocabulary and renders it.** New component in
|
||||
`apps/ssp/src/app/showcase/`. It owns `.card`, `.card--good`, `.card--bad`, `.tag`, its three
|
||||
modifiers and both `::before` rules, plus `.linked` and `.linked .src`.
|
||||
|
||||
Its API, driven by what the 12 current usages need:
|
||||
|
||||
```ts
|
||||
variant = input<'good' | 'bad' | 'plain'>('plain'); // card--good / card--bad / tag colour
|
||||
tag = input.required<string>(); // the uppercase label
|
||||
code = input<string | undefined>(); // pre [innerHTML], optional
|
||||
src = input<string | undefined>(); // figcaption; wraps code in figure.linked
|
||||
```
|
||||
|
||||
Everything else is projected through `<ng-content />`. The card **renders the `<pre>` itself**
|
||||
when `code` is set — that is what keeps `.app-code`'s box styling working without relying on
|
||||
projection.
|
||||
|
||||
5. **Six section components, one per `<section>`**, in `apps/ssp/src/app/showcase/`:
|
||||
|
||||
| File | Class | Heading | Own CSS |
|
||||
| ------------------------- | -------------------- | --------------------------- | --------------------------------------------- |
|
||||
| `unions.section.ts` | `UnionsSection` | 1 · Discriminated unions | none |
|
||||
| `remote-data.section.ts` | `RemoteDataSection` | 2 · RemoteData fold | none |
|
||||
| `parse.section.ts` | `ParseSection` | 3 · Parse, don't validate | none |
|
||||
| `form-machine.section.ts` | `FormMachineSection` | 4 · Form als state machine | `.machine`, `.node`, `.node.on` |
|
||||
| `vragenlijst.section.ts` | `VragenlijstSection` | 5 · Vragenlijst | `.steplist`, `.pill`, `.pill.extra`, `.arrow` |
|
||||
| `pii.section.ts` | `PiiSection` | 6 · PII — maskeren & parsen | none |
|
||||
|
||||
The "Own CSS" column is measured: those selectors appear in exactly one section each. Every
|
||||
other selector is now a global or lives in the card.
|
||||
|
||||
6. **The page keeps only what composes.** After the split `concepts.page.ts` holds its heading,
|
||||
its intro, and six elements. It keeps no `styles:` block. `code` and `src` (the generated
|
||||
snippets) move to whichever sections use them — each section imports
|
||||
`snippets.generated.ts` directly.
|
||||
|
||||
7. **Widen the colour guard, and fix the one file that widening catches.**
|
||||
`scripts/check-tokens.sh:14` greps `--include='*.component.ts'`, so **every `*.page.ts`,
|
||||
`*.section.ts` and `*.step.ts` in the repository is invisible to it** — including the six
|
||||
sections this ticket creates and the six `*.step.ts` files RD-22 and RD-23 just added. That
|
||||
is why this page accumulated 21 hardcoded colours unnoticed.
|
||||
|
||||
Change the include to `--include='*.ts'` and exclude specs and stories, which legitimately
|
||||
show colour swatches:
|
||||
|
||||
```bash
|
||||
hits=$(grep -rnE '#[0-9a-fA-F]{3,8}\b|rgba?\(|hsla?\(' apps libs --include='*.ts' \
|
||||
| grep -vE '\.(spec|stories)\.ts:' | grep -v 'token-ok' || true)
|
||||
```
|
||||
|
||||
Measured: this newly catches exactly one other line, `libs/beheer/src/ui/audit.page.ts:39`
|
||||
(`var(--rhc-color-rood-600, #a30000)`). Fix it by dropping the fallback, as decision 8 does
|
||||
for this page. **Leave the CIBG-GAP marker check at `*.component.ts`** — a gap extension is a
|
||||
component concept (ADR-0003).
|
||||
|
||||
8. **Drop every `var(--rhc-…, #hex)` fallback.** All the referenced tokens are defined in the
|
||||
bridge, so the fallback is dead weight that also trips the widened guard. `.card`'s
|
||||
`background: #fff` becomes `var(--rhc-color-wit)` — verified: that token is defined in
|
||||
`styles.scss`.
|
||||
|
||||
9. **No stories.** `showcase` is a teaching page, not a feature, and it has no story today.
|
||||
Adding six is not this ticket's job.
|
||||
|
||||
10. **Delete `/* eslint-disable max-lines */` from the page.** Mandatory — the rules pin each
|
||||
other in both directions.
|
||||
|
||||
## Files
|
||||
|
||||
- `libs/shared/styles.scss` — 5 tokens, 4 globals
|
||||
- `scripts/check-tokens.sh` — one line (decision 7)
|
||||
- `libs/beheer/src/ui/audit.page.ts` — one fallback (decision 7)
|
||||
- `apps/ssp/src/app/showcase/concept-card.component.ts` (new)
|
||||
- `apps/ssp/src/app/showcase/{unions,remote-data,parse,form-machine,vragenlijst,pii}.section.ts` (new)
|
||||
- `apps/ssp/src/app/showcase/concepts.page.ts`
|
||||
|
||||
## Steps
|
||||
|
||||
1. Add the tokens and the four globals to `libs/shared/styles.scss` (decisions 2 and 3).
|
||||
2. Write `concept-card.component.ts` (decision 4).
|
||||
3. Move each `<section>` into its own file, replacing every `<div class="card …">` with
|
||||
`<app-concept-card>`, `class="lead|cols|note"` with the `.app-*` names, and `<pre>` with
|
||||
either the card's `code` input or `<pre class="app-code">` for the four dynamic result blocks.
|
||||
4. Reduce the page to composition, with no `styles:` block.
|
||||
5. Widen the guard and fix `audit.page.ts` (decision 7).
|
||||
6. Delete the disable (decision 10).
|
||||
7. `git add -A`, then run the acceptance commands.
|
||||
8. Update this ticket's `Status:` to `done` and the README's RD-24 row to `done`.
|
||||
9. Commit all of it together.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
Measured against the tree before handover. Run after `git add -A`.
|
||||
|
||||
```bash
|
||||
D=apps/ssp/src/app/showcase
|
||||
git ls-files "$D/*.section.ts" | wc -l # is 0 -> MUST be 6
|
||||
git ls-files "$D/concept-card.component.ts" | wc -l # is 0 -> MUST be 1
|
||||
git grep -c "eslint-disable max-lines" -- $D/concepts.page.ts # is 1 -> MUST be 0
|
||||
git grep -c "styles:" -- $D/concepts.page.ts # is 1 -> MUST be 0
|
||||
```
|
||||
|
||||
The colours left the page, and the guard now covers it:
|
||||
|
||||
```bash
|
||||
git grep -cE "#[0-9a-fA-F]{3,6}" -- $D/concepts.page.ts # is 21 -> MUST be 0
|
||||
git grep -c "include='\*\.component\.ts'" -- scripts/check-tokens.sh # is 2 -> MUST be 1 (the CIBG-GAP check keeps it)
|
||||
npm run check:tokens # exits 0
|
||||
```
|
||||
|
||||
The highlighting rules are global, where innerHTML children can reach them (decision 1):
|
||||
|
||||
```bash
|
||||
git grep -c "app-code" -- libs/shared/styles.scss # MUST be >= 4
|
||||
git grep -c "app-code" -- $D/concepts.page.ts # MUST be 0
|
||||
```
|
||||
|
||||
The teaching content did not change while being moved:
|
||||
|
||||
```bash
|
||||
git grep -ho "code\['[a-zA-Z]*'\]" -- $D/ | sort -u | wc -l # is 9 -> MUST still be 9
|
||||
```
|
||||
|
||||
```bash
|
||||
npm run ci --full # exits 0
|
||||
```
|
||||
|
||||
## Verification
|
||||
|
||||
**`--full` is required** — this edits `libs/shared/styles.scss`, which every story renders
|
||||
against.
|
||||
|
||||
**Look at the page.** This is the one ticket in the arc whose main fix is invisible to every
|
||||
automated check: no test asserts a computed colour. Run `npm start`, open `/concepts`, and
|
||||
confirm that keywords, strings and comments in the code blocks are now coloured — purple, green
|
||||
and grey-italic against the dark background. If they are still monochrome, the rules are still
|
||||
scoped to a component.
|
||||
|
||||
**Do not add a line-count command.** `npm run lint` is the exact check.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Changing any teaching copy, snippet or demo. This is a move, not a rewrite.
|
||||
- `highlight-ts.ts` itself. Its output is correct; only the CSS was unreachable.
|
||||
- Adding stories (decision 9).
|
||||
- The `--app-devpanel-*` tokens, and any other page's colours.
|
||||
|
||||
## Risks
|
||||
|
||||
- **Angular does not style projected or `[innerHTML]` content from the receiving component.**
|
||||
This is the constraint that shapes decisions 1, 3 and 4. If you find yourself moving a rule
|
||||
into a component and its markup comes from somewhere else, the rule belongs in the global
|
||||
sheet.
|
||||
- **The `.app-note` case is subtle**: a note's text contains `<code>` markup, so it must be
|
||||
projected — which is exactly why it cannot be styled by the card. Global it is.
|
||||
- **The four dynamic `<pre>` blocks** (the ok/err demo output in sections 3 and 6) are not code
|
||||
snippets and have no `src`. Give them `class="app-code"` directly rather than forcing them
|
||||
through the card's `code` input.
|
||||
- **Widening the guard is a two-line change with a measured blast radius of one other file**
|
||||
(decision 7). If it catches more than `audit.page.ts:39`, stop and report — something landed
|
||||
since this ticket was written.
|
||||
- **Deleting the disable is mandatory** (decision 10).
|
||||
Reference in New Issue
Block a user