Skip to content

feat(app-theming): logo + 3-color + header-style theming with WCAG contrast guardrail - #15

Merged
rubenvdlinde merged 2 commits into
developmentfrom
wip/app-theming
Jul 24, 2026
Merged

feat(app-theming): logo + 3-color + header-style theming with WCAG contrast guardrail#15
rubenvdlinde merged 2 commits into
developmentfrom
wip/app-theming

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Summary

Implements app-theming (OpenSpec change, PURELY ADDITIVE): a lightweight logo + 3-color + header-style theming layer for OpenBuild virtual apps that are not on a mandated NL Design System token set, with a hard, non-bypassable WCAG contrast guardrail. Reuses nldesign-theme-selection's scoped [data-openbuild-theme-scope] CSS-variable applier mechanism and app-icon-management's icon/iconDark fields unmodified.

What's implemented (13 tasks, all checked in tasks.md)

  • runtime.appTheme manifest blocklogoRef, primaryColor, secondaryColor, accentColor, headerStyle (default|compact|branded), validated in src/services/manifestValidation/appTheme.js (mirrors theme.js's conventions exactly). Additive under runtime's existing additionalProperties: true — no schema change needed, no new OR register (matches nldesign-theme-selection precedent — nothing added under lib/Settings/register.d/).
  • checkThemeContrast.js — pure WCAG relative-luminance + contrast-ratio function. Checks primary-as-text (≥4.5:1) and primary/secondary/accent-as-UI-element (≥3:1) against a pinned #FFFFFF background (NC's --color-main-background light default). No override/bypass anywhere in the call chain.
  • AppCustomThemeSection.vue — theme editor: native <input type=color> + labeled NcTextField hex value per color, NcSelect header-style picker (inputLabel set), logo picker (defaults to the app icon, opt-in dedicated upload), live swatch preview, inline per-pair contrast-failure list (pair, computed ratio, required threshold).
  • useAppCustomTheme.js — scoped CSS-variable applier, same [data-openbuild-theme-scope] mechanism, managed <style data-openbuild-app-theme> element, torn down on leave.
  • AppBrandedHeader.vue — the headerStyle: "branded" logo strip.
  • Wired into PageDesignerHost.vue (editor + Save-time hard gate) and BuilderHost.vue (runtime render + injection-order coordination + lazy Application fetch for the branded header).

CSS variables targeted (pinned against the REAL fetched nldesign token CSS, not guessed)

Inspected the actual nldesign/css/tokens/*.css files (e.g. amsterdam.css) and nldesign/css/systems/nldesign/{theme,overrides}.css in the sibling repo. Finding that changed the plan: nldesign's scoped per-app token files (the exact asset nldesign-theme-selection's applier fetches and rewrites) declare only --nldesign-*-prefixed properties — never --color-* directly. The --color-primary: var(--nldesign-color-primary) !important mapping only exists in nldesign's global, unscoped, instance-wide theme.css/overrides.css — a separate mechanism this per-app feature doesn't touch.

So:

  • primaryColor--color-primary, --color-primary-element, --color-primary-element-hover, --color-primary-element-text, --color-primary-text — the real Nextcloud-standard names every NcButton/etc. reads — via a var(--nldesign-color-primary, <literal>) fallback chain, not a bare literal.
  • secondaryColor/accentColor → app-scoped --ob-theme-secondary/--ob-theme-accent (no native NC var exists for these roles; used only by AppBrandedHeader.vue).

Deviation: precedence mechanism (design.md D3)

Design.md's D3 assumed pure DOM injection order ("later declaration wins by cascade") would make "nldesign wins for any shared variable name" true. Against the real fetched CSS this doesn't hold — nldesign's scoped applier never sets --color-primary in-scope, so there's no shared property name for injection order to act on. The applier instead uses the var(--nldesign-color-primary, <fallback>) chain above: when nldesign's scoped <style> (still injected second, per D3, for future-proofing) also defines --nldesign-color-primary in the same scope, the var() resolves to its value; otherwise it falls back to the appTheme literal. Proven with a dedicated integration test (tests/composables/useAppCustomTheme.spec.js "nldesign precedence" suite) using a small CSS-cascade+var() resolver against a real amsterdam.css-shaped fixture (jsdom's getComputedStyle doesn't resolve custom properties, so this is the closest honest proxy for a unit test).

Deviation: theme editor location

Design.md guessed AppSettingsModal.vue. That modal only owns publish/permissions/data-registers and has no manifest access at all. The established, actually-reused pattern for runtime.* theme editing in this codebase is a manifest-driven, controlled sibling section wired into PageDesignerHost.vue (see the existing ThemeSection.vue). Followed that precedent instead — AppCustomThemeSection.vue sits right beside ThemeSection.vue.

Deviation: headerStyle: "branded" binding

CnAppRoot (installed @conduction/nextcloud-vue beta.219, confirmed by reading src/components/CnAppRoot/CnAppRoot.vue in the sibling checkout) exposes no dedicated top-bar logo/branding slot — only #header-actions (toolbar buttons beside the router-view) and #menu (left-rail nav). Implemented AppBrandedHeader.vue as an OpenBuild-side binding, rendered above the nested CnAppRoot inside BuilderHost.vue's existing [data-openbuild-theme-scope] wrapper — never touching the library.

WCAG guardrail — no bypass

  • Live, per-pair inline failures in AppCustomThemeSection.vue as the developer edits.
  • Hard block at the actual persist boundary: PageDesignerHost.save() re-checks checkThemeContrast() directly and refuses to call the API when it fails (belt-and-braces against any future caller of save() that bypasses the button). The "Save pages" button is also disabled with an explanatory title.
  • Covered the known fleet gotcha explicitly: secondary/accent (status-adjacent, non-primary) colors get their own UI-element contrast check too, not just primary — checkThemeContrast runs all three (primary/secondary/accent) as UI-element pairs, not only the primary palette.

Test results (exact commands)

docker run --rm -v "$PWD":/app -w /app nextcloud:34.0.0-apache php vendor/bin/phpunit -c phpunit-unit.xml
# → 699/699 passing, 2122 assertions (baseline unchanged — no PHP files touched)

npx vitest run
# → 135 files, 1338/1338 passing (baseline was 1268 — +70 new tests: checkThemeContrast.spec.js,
#   appThemeValidation.spec.js, useAppCustomTheme.spec.js, AppCustomThemeSection.spec.js,
#   AppBrandedHeader.spec.js, BuilderHost.spec.js, plus 4 added to PageDesignerHost.spec.js)

npx eslint src   # 0 errors (pre-existing @spec-tag JSDoc warnings only, unchanged baseline)
npx stylelint "src/components/AppCustomThemeSection.vue" "src/components/AppBrandedHeader.vue" \
  "src/views/BuilderHost.vue" "src/views/PageDesignerHost.vue"   # 0 errors, 0 warnings

openspec validate "app-theming" --type change --strict   # valid

bash <hydra>/scripts/run-hydra-gates.sh --scope-to-diff
# → 38/39 green. Sole failure: gate-46 spec-anchor-existence — pre-existing dangling
#   @spec references to now-archived change dirs in BuilderHost.vue/PageDesignerHost.vue/
#   useManifestValidator.js, verified byte-identical on origin/development BEFORE this
#   change (git show origin/development:<file> | grep matched the exact same targets).
#   Not introduced by this PR.

Unverified / deferred

  • Live E2E: no deploy to the shared dev instance per policy. tests/e2e/spec-coverage/app-theming.spec.ts is written with @e2e app-theming::* tags but test.skip()'d with the same Conduction/openbuild#41 quarantine reason the sibling nldesign-theme-selection.spec.ts already carries (the page-designer/builder admin UI doesn't render in this build) — logic coverage delegated to the vitest suites listed above, matching established precedent.
  • Version-snapshot/promotion/export losslessness (task 5.1) verified by code inspection (plain manifest field, same PATCH/PUT path every other runtime.* block already uses), not a live round-trip test.

Vendor hazard

vendor/ is .gitignore'd in this repo (confirmed via git check-ignore -v vendor) — git status vendor stayed clean throughout, and git show --stat HEAD on both commits confirms no vendor/ path was ever staged or committed.

🤖 Generated with Claude Code

Conduction Release Bot added 2 commits July 24, 2026 17:29
…ntrast guardrail

Adds the runtime.appTheme manifest block (logoRef, primaryColor,
secondaryColor, accentColor, headerStyle) as a lightweight, non-nldesign
theming layer for virtual apps, reusing nldesign-theme-selection's scoped
[data-openbuild-theme-scope] applier mechanism and app-icon-management's
icon/iconDark fields unmodified.

- appTheme manifest validation (services/manifestValidation/appTheme.js)
- checkThemeContrast.js — pure WCAG relative-luminance contrast guardrail
  (4.5:1 text, 3:1 UI element), no override/bypass
- AppCustomThemeSection.vue — theme editor, sibling of ThemeSection.vue in
  PageDesignerHost.vue; Save blocked at the actual persist boundary when
  contrast fails
- useAppCustomTheme.js — scoped CSS-variable applier; maps colors onto
  --color-primary/--color-primary-element via a
  var(--nldesign-color-primary, <fallback>) chain (verified against the
  ACTUAL fetched nldesign token CSS, which only ever sets --nldesign-*
  names) so an active nldesign theme genuinely takes precedence
- AppBrandedHeader.vue — OpenBuild-side branded-header binding (CnAppRoot
  has no logo/branding slot), wired into BuilderHost.vue

See the PR description for the full list of deviations from design.md's
implementation-time open questions and their rationale.
…rage gaps

- Fixed the "Logo defaults..." requirement anchor (apostrophe slugifies to
  "-s-", not "s") across all three files that reference it.
- Added missing @SPEC tags to every changed method/computed flagged by
  hydra gate-16 (spec-coverage): AppBrandedHeader.vue, AppCustomThemeSection.vue
  (including the headerStyleOption get/set pair), useAppCustomTheme.js,
  checkThemeContrast.js helper functions.

Result: 38/39 hydra gates green (--scope-to-diff); the sole remaining
failure (gate-46 spec-anchor-existence) is pre-existing dangling @SPEC
references to archived change directories, verified identical on
origin/development before this change.
@rubenvdlinde
rubenvdlinde merged commit 01ccb3e into development Jul 24, 2026
2 checks passed
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.

1 participant