Skip to content

fix(token-sets): drop the two Zuiddrecht nav tokens nothing reads, refresh its reference page - #1105

Merged
rubenvdlinde merged 2 commits into
developmentfrom
fix/woo589-zuiddrecht-tokens
Oct 6, 2026
Merged

rubenvdlinde merged 2 commits into
developmentfrom
fix/woo589-zuiddrecht-tokens

Conversation

@WilcoLouwerse

Copy link
Copy Markdown

Why

PHPUnit fails on all 8 legs of development, and with it the release PR #1080 (development → beta). There are two failures, both from #1099 (the Zuiddrecht hero and nav tokens):

  1. TokenSetVocabularyTest::testEveryShippedSetIsCompleteOrAllowListed: zuiddrecht: 2 declared name(s) nothing reads (--nldesign-website-nav-current-color, --nldesign-website-nav-current-in-line). No stylesheet in thematiq reads them, and a code search finds no reader in the portal repos either.
  2. TokenReferenceDocsTest::testTheCommittedPagesAreCurrent: docs/reference/token-sets/zuiddrecht.md is stale.

What

The allow-list is not used: its own comment reserves it for sets the converter still has to regenerate.

If the "current menu item as a blue piece of the line under the menu" look from #1099 is still wanted, it needs a reader first, e.g. in public-bridge.css or the portal's navigation styles. The two tokens can come back together with that reader.

Test

  • phpunit tests/Unit/TokenSetVocabularyTest.php → OK (8 tests).
  • phpunit tests/Unit/TokenReferenceDocsTest.php → OK (3 tests).
  • phpunit -c phpunit.token-sets.xml → OK (344 tests).

🤖 Generated with Claude Code

…fresh its reference page

#1099 gave Zuiddrecht --nldesign-website-nav-current-in-line and
--nldesign-website-nav-current-color, but no stylesheet in thematiq (or
the portal) reads either name, so TokenSetVocabularyTest failed on every
PHPUnit leg, and the committed reference page still listed 82 tokens
instead of the set's current ones. Removing the two unread names changes
nothing on screen. The dark variant is regenerated from the set, and
docs/reference/token-sets/zuiddrecht.md from composer docs:token-reference,
so it now also lists the hero tokens from #1099 that public-bridge.css
does read.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@WilcoLouwerse WilcoLouwerse left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Quick review (self-review, posted as a comment). Nothing definitely broken.

  • The two tokens it removes, --nldesign-website-nav-current-in-line and --nldesign-website-nav-current-color, have no reader in thematiq's CSS, and an org-wide code search finds none in the portal repos either. Removing them changes nothing on screen.
  • The four hero tokens from #1099 stay, because public-bridge.css reads them.
  • css/tokens/dark/zuiddrecht.css comes from its own generator (generate-dark-variants.php, new source hash), and the reference page from composer docs:token-reference.

Checks run locally:

  • TokenSetVocabularyTest → OK (8 tests).
  • TokenReferenceDocsTest → OK (3 tests).
  • phpunit.token-sets.xml → OK (344 tests).

Gates: CI on 64edc8ae is still running at the time of this review. The PHPUnit legs are the real check, because they are what is red on development and on #1080.

Review assisted with Claude AI.

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/thematiq @ e2323bf

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-manifest ✅
test-l10n ✅
format ✅
test-fonts ✅
test-token-set-coverage ✅
composer ✅ ✅ 107/107
npm ✅ ✅ 2/2
app:check-code ⏭️
info.xml ✅
REUSE ✅
lockfile sync ✅
PHPUnit ✅
Newman ✅
Playwright ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development.
Hydra gates ✅

Quality workflow — 2026-10-06 13:01 UTC

Download the full PDF report from the workflow artifacts.

…ddrecht-tokens

# Conflicts:
#	css/tokens/dark/zuiddrecht.css
#	css/tokens/zuiddrecht.css
#	docs/reference/token-sets/zuiddrecht.md
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/thematiq @ 0b82b70

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-manifest ✅
test-l10n ✅
format ✅
test-fonts ✅
test-token-set-coverage ✅
composer ✅ ✅ 107/107
npm ✅ ✅ 2/2
app:check-code ⏭️
info.xml ✅
REUSE ✅
lockfile sync ✅
PHPUnit ✅
Newman ✅
Playwright ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development.
Hydra gates ✅

Quality workflow — 2026-10-06 22:15 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit a396a5e into development Oct 6, 2026
43 checks passed
rubenvdlinde added a commit that referenced this pull request Oct 7, 2026
…o ground; the menu mark restored (#1145)

Website tokens for Zuiddrecht from the Kop, Home and Contentpagina boards: a
1328px page with a 24px gutter, a 1280px header row and band, 17px text, a 21px
lead, 600 buttons through --nldesign-website-button-font-weight (700 for every
other set), a website grey #4A4A4A and a #D9E3EF photo ground.

Restore --nldesign-website-nav-current-in-line and -color, which #1105 removed
as unread although the portal reads them. Every --nldesign-* name portaliq's
site-theme.css reads now has a role in public-bridge.css, held by a measured
fixture and tests/vitest/portalReaders.spec.js. Dark variant and token
reference regenerated for zuiddrecht only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants