Skip to content

feat(walkthrough): a first-visit tour built from this app's own menu - #484

Merged
rubenvdlinde merged 4 commits into
developmentfrom
feat/walkthrough
Aug 29, 2026
Merged

feat(walkthrough): a first-visit tour built from this app's own menu#484
rubenvdlinde merged 4 commits into
developmentfrom
feat/walkthrough

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

ADR-043. This app had no walkthrough, so a new user arrived on a shell with no orientation at all.

Every target is read from this manifest, never invented

CnWalkthrough resolves target: {kind: "nav-item", ref} by querying [data-cn-route="<ref>"],
which CnAppNav emits from the route. A tour pointing at a menu id that differs from its route
still renders — as a centred, anchorless coachmark that points at nothing. That failure is invisible
in every way that matters: the step appears, advances correctly, and reads fine; it just stops doing
the one thing it exists for.

So every ref here is taken from the manifest's own route, and I verified that each one resolves
to a page this manifest declares
.

Translation keys are not sentences

Where a menu label is a key rather than prose, the copy uses the key's last segment as a word.
Printing launchpad.menu.dashboards at a user is worse than useless — and the obvious fallback, the
route, gives "Click dashboard-detail in the menu": an internal identifier read aloud.

Verified

  • manifest validates against schema 2.26.0
  • 0 target refs that do not resolve to a declared page
  • 0 raw translation keys reaching user-visible copy
  • manifest-only change

Scope note

thematiq and planninq are deliberately excluded: both declare zero pages and zero menu
entries
, so a tour would have nothing to point at. A walkthrough there would be the same anchorless
coachmark this PR exists to avoid.

Conduction Release Bot added 2 commits August 28, 2026 21:48
ADR-043. The app had no walkthrough, so a new user arrived on a shell with no
orientation at all.

Every step targets a route READ FROM THIS MANIFEST, never invented.
CnWalkthrough resolves target {kind: nav-item, ref} by querying
[data-cn-route=<ref>], which CnAppNav emits from the ROUTE - so a tour pointing
at a menu id that differs from its route still renders, as a centred anchorless
coachmark that points at nothing. Verified: every ref resolves to a page this
manifest declares.

Where a menu label is a translation KEY rather than a sentence, the copy uses
the key's last segment as a word. Printing launchpad.menu.dashboards at a user
is worse than useless, and falling back to the route gives 'Click
dashboard-detail in the menu' - an internal identifier read aloud.

Verified: manifest validates against schema 2.26.0, every target ref resolves,
and no raw translation key reaches the copy.
gate-70 walkthrough-flows-stop - this app ships a flows page and no tour step
targeted it. The gate's own measurement is why it exists: across 20 manifests,
12 apps shipped a flows page and exactly ONE tour mentioned it, so the
automation surface was discoverable only by someone who already knew.

Detected the way the gate detects it - type:'flows' OR the migrated form, an
index over config.entitySource:'flows'. Matching on type alone would miss every
app the flow-page migration touched, which is all of them.

The step advances on route-match and sets allowManualNext. The same gate fails a
step advancing only on object-created: that turns 'here is where flows live'
into 'build an automation before you may continue', and a tour nobody can finish
is worse than one that never mentions flows.

gate-102 manifest-l10n-coverage - the tour's titles, bodies and tasks had no
l10n/nl.json key. Added, and the browser catalogue rebuilt where the app ships
one, because the browser reads nl.js rather than nl.json.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/keepiq @ 78eda1d

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
test-l10n
format
check-l10n-js
check-schema-l10n
composer ✅ 111/111
npm ✅ 550/550
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman
Playwright 🚨 NO VERDICT — enabled but never ran
Hydra gates

Quality workflow — 2026-08-28 20:12 UTC

Download the full PDF report from the workflow artifacts.

@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/keepiq @ 022d78a

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
test-l10n
format
check-l10n-js
check-schema-l10n
composer ✅ 111/111
npm ✅ 550/550
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman
Playwright 🚨 NO VERDICT — enabled but never ran
Hydra gates

Quality workflow — 2026-08-28 20:24 UTC

Download the full PDF report from the workflow artifacts.

gate-96 (manifest-copy-style) fails this: voice.md section 8 is unambiguous —
no em-dashes, Conduction strips them fleet-wide.

The nl.json KEY is the English string, so it is renamed with it; leaving the
key behind would orphan the Dutch translation of a string that still ships.
The Dutch VALUE carried the same em-dash and gets the same treatment — the
gate only counts manifest strings, but it is the same user-facing sentence.

Note for a later sweep: this app's nl.json still holds 16 other em-dash
strings from Vue copy. They are outside this gate's scope (it reads manifest
strings) and outside this PR's.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/keepiq @ f62cf5e

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
test-l10n
format
check-l10n-js
check-schema-l10n
composer ✅ 111/111
npm ✅ 550/550
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman
Playwright 🚨 NO VERDICT — enabled but never ran
Hydra gates

Quality workflow — 2026-08-29 02:00 UTC

Download the full PDF report from the workflow artifacts.

check:l10n-js reported 'Stale browser catalogue: l10n/nl.js'. Renaming the key
in nl.json without nl.js leaves the two halves of the same catalogue
disagreeing, and the runtime reads the .js one.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/keepiq @ 736f9ca

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
test-l10n
format
check-l10n-js
check-schema-l10n
composer ✅ 111/111
npm ✅ 550/550
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman
Playwright 🚨 NO VERDICT — enabled but never ran
Hydra gates

Quality workflow — 2026-08-29 02:44 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 6478ced into development Aug 29, 2026
44 checks passed
@rubenvdlinde
rubenvdlinde deleted the feat/walkthrough branch August 29, 2026 02:58
rubenvdlinde added a commit that referenced this pull request Aug 29, 2026
…t a leak (#488)

The first-visit tour added in #484 made CnAppRoot resolve
`walkthrough_completed_version`, and that fetch runs before any route
resolves — so it fires behind the lock screen and the vault-unlock
invariant failed closed, exactly as designed.

Judged rather than silenced. It is the same shell layer, the same
GenericPreferencesController and the same `pref_`-namespaced session-user
UI flag as the `support-dialog-seen` entry directly above it, and it
carries no vault material. The a11y snapshot from the failing run shows
the bare unlock form with no walkthrough card, so only the probe reaches
the wire; a tour that painted over the lock screen would still fail this
test on its own traffic.

The `\b` anchor keeps the entry from widening onto neighbouring paths.

Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
rubenvdlinde added a commit that referenced this pull request Aug 29, 2026
… instead (#490)

Two sessions fixed the same failure minutes apart. #486 withheld the
walkthrough from the manifest while the vault is locked, so CnAppRoot never
resolves the completion preference and the probe cannot reach the wire.
#488 then allowlisted that probe — which the file's own note calls pure
masking surface: an entry that cannot fire would silently swallow a real
regression of exactly this shape. #486 is the better fix; the entry goes.

What is NOT fixed by #486 is the order-dependence. The tour's seen state is
per user, not per test, so whichever spec runs first wears it — every spec
here unlocks, and the tour is offered on the first unlocked visit by design.
audit-trail.spec.ts started needing a retry the moment #484 landed, having
never retried in the three preceding development runs. Seeding the marker
in global-setup, as dossiq already does, removes that.

Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
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