NLDD alignment: the time field, the popup slot and the popover frame #235

Merged

robbertbos merged 4 commits from nldd-alignment into main 2026-08-06 17:44:34 +00:00

Conversation 1 Commits 4 Files changed 12 +343 -270

robbertbos commented 2026-08-06 06:25:34 +00:00

Owner

Copy link

Closes #182, closes #132, closes #170.

Four patterns we had built by hand go back to the design system, plus a test for the one hazard that made the work necessary in the first place.

What made this possible

NLDD 0.8.78 (released 2026-08-05) ships nldd-time-field and nldd-time-picker. That was the single thing #182 said the schedule popover could not do without: "cannot be fully NLDD-native because NLDD has no time-field, so the time input stays native/custom". It has one now, so the native input type="time" and the eighteen lines of chrome that copied its neighbour by hand are gone - border, corner radius, 16px tier, 44px control box, each with a comment naming the value it was matching.

What else went back

• The two toolbar menus move into their button's popup slot (#170). That removes the shared openMenu() helper, the anchorElement/showPopover bookkeeping, two toggle listeners, the manual aria-haspopup/aria-expanded pair, and the .heading/.moreBtn rules that reimplemented a 1.75rem transparent button - down to a comment saying "matches NLDD's button convention". The explicit hidePopover() calls go too: nldd-menu collapses its own chain on select.
• The plan popover's Terug/Plannen pair becomes two nldd-buttons. The primary one was painted with --semantics-links-color and a brightness(0.95) hover.
• The floating panels (plannen, bron-pad, "Aan") drop the 1px divider border they drew on top of the overlay shadow. nldd-menu frames itself with radius and shadow on the base surface and nothing else, so ours read as a different system than the menus beside them.
• The link bar's input takes nldd-text-field's border token and the global focus ring instead of a divider-coloured border and a hand-mixed color-mix() glow. It stays a native input: it is wired to a raw Tiptap reference.
• The block-level menu gains its icons (#132): paragraph-sign, heading-1..heading-6, verified against the icon registry first, since nldd-icon fails silently on an unknown name.

The guard, and why the reset stays as it is

#205 said our * { margin: 0; padding: 0 } was silently zeroing the padding of nldd-menu, nldd-banner and nldd-switch. Measured on 0.8.78: that is no longer true. NLDD has moved every affected box into a shadow wrapper (.menu__main, .banner, .switch__track) or shielded it with !important, and says why in its own source - "shields the row padding from consumer universal resets, which beat normal :host declarations per CSS Scoping".

The hazard itself has not moved. A probe element with :host { padding: 12px; margin: 7px } still computes to 0px/0px in the running app, while the same declarations inside its shadow root survive. So narrowing the reset would touch every page to fix nothing measurable, and instead there is now nlddHostBox.test.ts: it reads the tags our own templates use, resolves each to its component directory, and fails on a :host-only rule declaring padding or margin without !important.

Same shape as nlddTokens.test.ts, including an assertion that proves the detector fires on input that should fail it. That one earned its place immediately - the first version split declarations per line and so missed the padding in :host { display: block; padding: 12px }.

Verified in the browser, not only on green tests

• The picker wheel opens inside the popover's own focus trap and is not clipped; arrow keys move the hour; "Klaar" commits. A card snoozed through it landed on 2026-08-07T07:00:00Z, which is the 09:00 that was picked.
• Both toolbar buttons carry aria-haspopup="menu" and aria-expanded from first render - an 0.8.76 fix we now get for free. The menus anchor under their trigger, close on select, and a format applied through the slotted menu reaches the editor.
• The block-level menu shows its icons and its radio check-mark; the toolbar still collapses into "Meer" at 620px.

Gates

vitest 694, vue-tsc, npm run build, pytest 1669 on Postgres, pinned pre-commit, eslint 0 errors, Playwright 67 passed / 1 failed. That one failure is encrypted-export-roundtrip, which is permanently red on this machine and unrelated.

just lint fails on backend/tests/integration/test_migrations.py import order. That is local ruff 0.15 disagreeing with the 0.8.6 the hooks pin; the pinned run passes, and the file came in with the Postgres-only merge.

Not in this PR

• #205's actual reset narrowing - see above; the issue is closed with the measurements.
• #167 (avatar next to the composer) is a design question, left open with that scope.
• The hyphens half of #132 does not apply: we use nldd-rich-text nowhere, deliberately.

Closes #182, closes #132, closes #170. Four patterns we had built by hand go back to the design system, plus a test for the one hazard that made the work necessary in the first place. ## What made this possible NLDD 0.8.78 (released 2026-08-05) ships `nldd-time-field` and `nldd-time-picker`. That was the single thing #182 said the schedule popover could not do without: "cannot be fully NLDD-native because NLDD has no time-field, so the time input stays native/custom". It has one now, so the native `input type="time"` and the eighteen lines of chrome that copied its neighbour by hand are gone - border, corner radius, 16px tier, 44px control box, each with a comment naming the value it was matching. ## What else went back - **The two toolbar menus** move into their button's `popup` slot (#170). That removes the shared `openMenu()` helper, the `anchorElement`/`showPopover` bookkeeping, two toggle listeners, the manual `aria-haspopup`/`aria-expanded` pair, and the `.heading`/`.moreBtn` rules that reimplemented a 1.75rem transparent button - down to a comment saying "matches NLDD's button convention". The explicit `hidePopover()` calls go too: `nldd-menu` collapses its own chain on `select`. - **The plan popover's Terug/Plannen pair** becomes two `nldd-button`s. The primary one was painted with `--semantics-links-color` and a `brightness(0.95)` hover. - **The floating panels** (plannen, bron-pad, "Aan") drop the 1px divider border they drew on top of the overlay shadow. `nldd-menu` frames itself with radius and shadow on the base surface and nothing else, so ours read as a different system than the menus beside them. - **The link bar's input** takes `nldd-text-field`'s border token and the global focus ring instead of a divider-coloured border and a hand-mixed `color-mix()` glow. It stays a native input: it is wired to a raw Tiptap reference. - **The block-level menu gains its icons** (#132): `paragraph-sign`, `heading-1`..`heading-6`, verified against the icon registry first, since `nldd-icon` fails silently on an unknown name. ## The guard, and why the reset stays as it is #205 said our `* { margin: 0; padding: 0 }` was silently zeroing the padding of `nldd-menu`, `nldd-banner` and `nldd-switch`. Measured on 0.8.78: that is no longer true. NLDD has moved every affected box into a shadow wrapper (`.menu__main`, `.banner`, `.switch__track`) or shielded it with `!important`, and says why in its own source - "shields the row padding from consumer universal resets, which beat normal `:host` declarations per CSS Scoping". The hazard itself has not moved. A probe element with `:host { padding: 12px; margin: 7px }` still computes to `0px`/`0px` in the running app, while the same declarations inside its shadow root survive. So narrowing the reset would touch every page to fix nothing measurable, and instead there is now `nlddHostBox.test.ts`: it reads the tags our own templates use, resolves each to its component directory, and fails on a `:host`-only rule declaring padding or margin without `!important`. Same shape as `nlddTokens.test.ts`, including an assertion that proves the detector fires on input that should fail it. That one earned its place immediately - the first version split declarations per line and so missed the padding in `:host { display: block; padding: 12px }`. ## Verified in the browser, not only on green tests - The picker wheel opens inside the popover's own focus trap and is not clipped; arrow keys move the hour; "Klaar" commits. A card snoozed through it landed on `2026-08-07T07:00:00Z`, which is the 09:00 that was picked. - Both toolbar buttons carry `aria-haspopup="menu"` and `aria-expanded` from first render - an 0.8.76 fix we now get for free. The menus anchor under their trigger, close on select, and a format applied through the slotted menu reaches the editor. - The block-level menu shows its icons and its radio check-mark; the toolbar still collapses into "Meer" at 620px. ## Gates `vitest` 694, `vue-tsc`, `npm run build`, `pytest` 1669 on Postgres, pinned `pre-commit`, `eslint` 0 errors, Playwright 67 passed / 1 failed. That one failure is `encrypted-export-roundtrip`, which is permanently red on this machine and unrelated. `just lint` fails on `backend/tests/integration/test_migrations.py` import order. That is local ruff 0.15 disagreeing with the 0.8.6 the hooks pin; the pinned run passes, and the file came in with the Postgres-only merge. ## Not in this PR - #205's actual reset narrowing - see above; the issue is closed with the measurements. - #167 (avatar next to the composer) is a design question, left open with that scope. - The `hyphens` half of #132 does not apply: we use `nldd-rich-text` nowhere, deliberately.

robbertbos added 2 commits 2026-08-06 06:25:34 +00:00

Fail the tests on an NLDD host box our reset would zero ... 2f09f3357a

reset.css sets `* { margin: 0; padding: 0 }`, and `*` matches the host of a
custom element. Per CSS Scoping a declaration from the outer document beats one
from a shadow root regardless of specificity, so a component that puts its box
on :host loses it here - silently, while the component still lays out as if the
padding were there. This cost us a visibly broken banner once (#205).

Measured in the running app on 0.8.78: a probe element with
`:host { padding: 12px; margin: 7px }` computes to 0px/0px, while the same
declarations on an element inside its shadow root survive at 12px. So the
hazard is unchanged. What changed is NLDD, which has since moved every affected
box into a shadow wrapper (.menu__main, .banner, .switch__track) or shielded it
with !important, and says why in its own source: "shields the row padding from
consumer universal resets, which beat normal :host declarations per CSS
Scoping".

Nothing is broken today, so this narrows nothing in the reset. It watches
instead: scan the tags our own templates use, resolve each to its component
directory, and fail on a :host-only rule declaring padding or margin without
!important.

Three assertions, in the shape nlddTokens.test.ts already uses. The middle one
proves the detector fires on input that should fail it, and earned its place
immediately: the first version split declarations per line and so missed the
padding in `:host { display: block; padding: 12px }`.

Hand four hand-rolled patterns back to NLDD 0.8.78 ... 6adff5dc4a

0.8.78 ships nldd-time-field and nldd-time-picker. That was the one thing #182
said the schedule popover could not do without, so the native `input type=time`
next to nldd-date-field goes, and with it the eighteen lines of chrome that
copied the neighbouring field by hand - border, corner radius, 16px tier, 44px
control box, each with a comment explaining which value it was matching.

Along the same line, four more copies go back to the design system:

- The two toolbar menus move into their button's `popup` slot. That deletes the
  shared openMenu() helper, the anchorElement/showPopover bookkeeping, two
  toggle listeners and the manual aria-haspopup/aria-expanded pair, plus the
  .heading and .moreBtn rules that reimplemented a 1.75rem transparent button
  down to "matches NLDD's button convention" in a comment. The explicit
  hidePopover() calls go too: nldd-menu collapses its own chain on select. So
  does the "leave the menu open so several formats can be toggled in a row"
  comment, which described an intention the component has always overruled.
- The plan popover's Terug/Plannen pair becomes two nldd-buttons. The primary
  one was painted with --semantics-links-color plus a brightness(0.95) hover.
- The floating panels (plannen, bron-pad, "Aan") drop the 1px divider border
  they drew on top of the overlay shadow. nldd-menu frames itself with radius
  and shadow on the base surface and nothing else, so ours read as a different
  system than the menus beside them.
- The link bar's input takes nldd-text-field's border token and the global
  focus ring, instead of a divider-coloured border and a hand-mixed
  color-mix() glow. It stays a native input: it is wired to a raw Tiptap
  reference.

The block-level menu also gains its icons (paragraph-sign, heading-1..6), all
verified against the icon registry first, since nldd-icon fails silently on an
unknown name.

Verified in the browser on the preview build, not only on green tests: the
picker wheel opens inside the popover's own focus trap, arrow keys move the
hour, and Klaar commits - a card snoozed through it landed on 2026-08-07T07:00Z,
which is the 09:00 that was picked. Both toolbar buttons carry aria-haspopup and
aria-expanded from first render (an 0.8.76 fix we now get for free), the menus
anchor and close on select, and a format applied through the slotted menu
reaches the editor.

Two test seams shifted, both because a hand-rolled control became a real
component: the picker tests query nldd-time-field instead of input[type=time],
and ComposeOverlay can no longer say "the only nldd-button in the modal is the
discard one" now that the block-level dropdown is one too.

The banner token rename in 0.8.78 (--components-banner-padding ->
--components-banner-md-padding) does not touch us; we reference no
--components-banner-* token, and nlddTokens.test.ts would have caught it.

robbertbos added 1 commit 2026-08-06 06:51:17 +00:00

Keep the Playwright report when e2e fails in CI ... dd2d00a0dc

A red e2e job currently leaves nothing behind. The run logs are not reachable
through the API - /actions/runs/<id>/jobs/<n>/logs answers 404 even with a
token - so the only way to learn which test failed is to open the job in a
browser. That is a poor place to be when a nightly or an agent run goes red.

Upload playwright-report/ and test-results/ on failure only: the HTML report,
the failure screenshots and the traces, which is everything needed to open
`npx playwright show-trace` locally. Seven days is plenty for a CI artifact
nobody looks at when the job is green.

upload-artifact is pinned to v3 for the same reason security-scan.yaml pins it:
Forgejo emulates GHES and the v4+ transport does not work here.

robbertbos commented 2026-08-06 07:06:40 +00:00

Author

Owner

Copy link

CI note, for the record.

CI / e2e failed on 6adff5d and is green on dd2d00a, which is the same frontend plus one CI step. What I can back up:

• Locally the suite is green except encrypted-export-roundtrip, which is permanently red on this machine. That holds with CI=1 too, so the two retries CI uses are included.
• The same content passed on the second run.
• The control group fits: main ran an e2e of 12:46 at the same time, without these changes. A green run took 10:15-10:45 before that. The runner is slower than usual right now, which is what pushes a timing-sensitive test over the edge.

What I cannot back up is which test failed. /actions/runs/<id>/jobs/<n>/logs answers 404 even with a token, so there was nothing to read. Calling it a probable flake rather than a proven one.

Hence the third commit: the e2e job now keeps playwright-report/ and test-results/ as an artifact when it fails (7 days). That was a real gap - a red e2e left nothing behind unless you opened the job in a browser. Next time npx playwright show-trace is enough.

CI note, for the record. `CI / e2e` failed on `6adff5d` and is green on `dd2d00a`, which is the same frontend plus one CI step. What I can back up: - Locally the suite is green except `encrypted-export-roundtrip`, which is permanently red on this machine. That holds with `CI=1` too, so the two retries CI uses are included. - The same content passed on the second run. - The control group fits: main ran an e2e of **12:46** at the same time, without these changes. A green run took **10:15-10:45** before that. The runner is slower than usual right now, which is what pushes a timing-sensitive test over the edge. What I cannot back up is *which* test failed. `/actions/runs/<id>/jobs/<n>/logs` answers 404 even with a token, so there was nothing to read. Calling it a probable flake rather than a proven one. Hence the third commit: the e2e job now keeps `playwright-report/` and `test-results/` as an artifact when it fails (7 days). That was a real gap - a red e2e left nothing behind unless you opened the job in a browser. Next time `npx playwright show-trace` is enough.

robbertbos added 1 commit 2026-08-06 17:39:16 +00:00

Merge origin/main into nldd-alignment ... f327db0722

Two conflicts, both in files main had also been working in.

CHANGELOG.md: both sides had prepended a bullet to "Onder de motorkap". Kept
both.

ComposeOverlay.test.ts: took main's version whole. The conflicting hunks were
mine, and they only disambiguated a selector - `wrapper.get('nldd-button')`
picked the discard button on the assumption that it was the only nldd-button in
the modal, which stopped being true once the toolbar's block-level dropdown
became one. Main has since dropped the discard button from that composer
entirely (31cddbb), so the tests those hunks touched no longer exist. Checked
afterwards that no unqualified `nldd-button` selector is left in the file.

Also moved the time field and the heading icons out of "Onder de motorkap" and
into their own sections: both are things a user sees, and that heading is for
things they do not.

robbertbos merged commit 8e706ba9bb into main 2026-08-06 17:44:34 +00:00

robbertbos deleted branch nldd-alignment 2026-08-06 17:44:34 +00:00

robbertbos referenced this pull request 2026-08-06 18:00:57 +00:00

Give the composer footer a bottom padding #240

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

Something is not working

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

help wanted

Need some help

  

invalid

Something is wrong

  

nldd ds

  

question

More information is needed

  

wontfix

This won't be fixed

No labels

bug

duplicate

enhancement

help wanted

invalid

nldd ds

question

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

robbertbos/waggle!235

No description provided.