Skip to content

fix(Modal,Slideover): render the actions slot when nothing else opens the header - #512

Merged
IgorShevchik merged 2 commits into
mainfrom
test/attach-and-shrink-snapshots
Aug 30, 2026
Merged

fix(Modal,Slideover): render the actions slot when nothing else opens the header#512
IgorShevchik merged 2 commits into
mainfrom
test/attach-and-shrink-snapshots

Conversation

@IgorShevchik

@IgorShevchik IgorShevchik commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

Linked issue

Refs #87 (first half — attaching to the document. The 27 MB corpus itself is not shrunk here; that is the second half and gets its own PR.)

Follow-ups filed from this work: #513 (293 hand-written mountSuspended calls are still detached) and #514 (the console gate's register is all-or-nothing per file).

Type of change

  • Documentation (updates to the documentation or readme)
  • Bug fix (a non-breaking change that fixes an issue)
  • Enhancement (improving an existing functionality)
  • New feature (a non-breaking change that adds functionality)
  • Chore (updates to the build process or auxiliary tools and libraries)
  • Revert (undoing a merged change — retitle this PR revert(Scope): ...)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

componentRender now mounts with attachTo: document.body and unmounts as soon as it has the markup. Both halves were doing work the suite could not see.

Attaching. reka-ui's dialogs check their own accessibility with document.getElementById(titleId) in onMounted. Against a detached tree that lookup fails, so every dialog spec rendering with portal: false warned that DialogContent requires a DialogTitle — with the title sitting in the markup it had just produced. Measured with the register in console-gate.ts emptied on both sides: 623 console messages from 62 tests before, 366 from 56 after.

It also moved 402 snapshot entries across 25 files toward what a browser renders: Header and Table gain the aria-hidden a browser puts on everything behind an open dialog, and CheckboxGroup/RadioGroup gain the aria-label reka-ui reads off the associated <label> rather than the raw value.

Unmounting is the half that found defects, because teardown had never run.

What was wrong
Modal, Slideover The actions slot was dropped whenever nothing else asked for a header. Slideover hit it on its own defaults — side: 'bottom' puts the close button outside the header, so <B24Slideover :actions> rendered nothing at all. Modal had the same hole twice, in the header and in contentWrapper, reachable with :close="false". Drawer had it right and is the reference.
Countdown The requestAnimationFrame handle lived in a ref, which deep-wraps an object value in reactive(). A browser returns a number there; happy-dom returns the Node Immediate it scheduled, so clearImmediate was handed a Proxy and unlinked a node that was in no queue — four uncaught TypeError: Cannot read properties of undefined (reading '_idleNext') the moment pause() first ran. Now a plain let, dropped as it is spent.
test/utils/mount.ts The vue project's router is a module singleton, and vue-router's install replaces app.unmount with one that resets currentRoute to START_LOCATION. Every mount after the first rendered against an un-navigated router, and Link to="/" silently stopped being active. The shim awaits router.isReady().

Three tests were asserting nothing and now assert something. Countdown's pauses counting never started the countdown, so it only watched cancelAnimationFrame be handed the initial 0. The two editor specs mocked registerPlugin without unregisterPlugin, which mattered only once teardown reached tiptap's beforeUnmount.

How the Modal/Slideover bug was found. The #454 collision guard: renders with actions slot was byte-identical to renders with open. The same guard also showed that four of its baseline groups were held apart by pointer-events: auto — which reka-ui renders from a module-global layer stack, so it recorded how many earlier cases were still mounted rather than anything under test. The baseline is regenerated and is smaller: 291 groups, 875 entries. Slideover never entered it, because the case was fixed instead of recorded.

One register entry goes: nuxt:components/Header.spec.ts. The rest stay because the register keys on files, and every dialog spec also mounts by hand outside componentRender — see #513.

Review

A second commit acts on review. continueProcess cancelled the rAF handle without dropping it, so the delay <= 0 branch carried an already-cancelled handle into stop() and cancelled it twice — the same shape the comment two lines above blames for the four TypeErrors. And the JSDoc on componentRender had grown to 33 lines against this repo's own twenty-line ceiling; the measurements and the note about mocks needing their teardown half moved to .github/contributing/testing.md.

One flagged item was left alone deliberately. DropdownMenu's and Select's with items cases are not newly vacuous — with items is the reference render of its group (29 KB, eleven real menu items), and what disappeared was one leaked pointer-events: auto byte that recorded how many earlier cases were still mounted. dropdown-menu's theme puts colour on the trigger, which these fixtures do not render, so this is the collision guard's own documented example of an unavoidable one.

Gate

lint, typecheck, 318 test files / 7474 tests, test:module, and coverage 71.98 / 69.99 / 71.18 / 71.55 against thresholds of 70 / 68 / 70 / 70.

Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

claude added 2 commits August 29, 2026 10:54
… the header

`componentRender` now mounts with `attachTo: document.body` and unmounts as
soon as it has the markup. Both halves were doing work the suite could not see.

Attached, reka-ui can answer the questions it asks the document. Its dialogs
check their own accessibility with `document.getElementById(titleId)` in
`onMounted`, which failed against a detached tree and warned that
`DialogContent` requires a `DialogTitle` with the title sitting in the markup
just produced. With the register in `console-gate.ts` emptied on both sides:
623 console messages from 62 tests before, 366 from 56 after. It also moved 402
snapshot entries across 25 files toward what a browser renders — `Header` and
`Table` gain the `aria-hidden` behind an open dialog, `CheckboxGroup` and
`RadioGroup` the `aria-label` read off the associated `<label>` rather than the
raw value.

Unmounting is the half that found defects, because teardown had never run:

- `Modal` and `Slideover` dropped the `actions` slot whenever nothing else
  asked for a header. `Slideover` hit it on its own defaults — `side: 'bottom'`
  puts the close button outside the header, so `<B24Slideover :actions>`
  rendered nothing at all. `Modal` had the same hole twice over, in the header
  and in `contentWrapper`, reachable with `:close="false"`. `Drawer` had it
  right and is the reference. The `#454` collision guard is what surfaced this:
  `renders with actions slot` was byte-identical to `renders with open`.

- `Countdown` kept its `requestAnimationFrame` handle in a `ref`, which deep-
  wraps an object value in `reactive()`. A browser returns a number there, but
  happy-dom returns the Node `Immediate` it scheduled, so `clearImmediate` was
  handed a Proxy and unlinked a node that was in no queue — four uncaught
  `TypeError: Cannot read properties of undefined (reading '_idleNext')` the
  moment `pause()` first ran. The handle is now a plain `let`, dropped as it is
  spent.

- The `vue` project's router is a module singleton, and vue-router's `install`
  replaces `app.unmount` with one that resets `currentRoute` to
  `START_LOCATION`. Every mount after the first rendered against an
  un-navigated router; `Link to="/"` silently stopped being active. The shim
  awaits `router.isReady()`.

Three tests were asserting nothing and now assert something: `Countdown`'s
`pauses counting` never started the countdown, so it watched
`cancelAnimationFrame` be handed the initial `0`; the two editor specs mocked
`registerPlugin` without `unregisterPlugin`, which only mattered once teardown
reached tiptap's `beforeUnmount`.

Four `#454` baseline groups were held apart by `pointer-events: auto`, which
reka-ui renders from a module-global layer stack — so it recorded how many
earlier cases were still mounted, not anything under test. The baseline is
regenerated and is smaller: 291 groups, 875 entries.

One register entry goes: `nuxt:components/Header.spec.ts`. The rest stay
because the register keys on files and every dialog spec also mounts by hand,
outside `componentRender`. That is 293 call sites across 112 files and wants
its own change.

Refs #87
Review found one real defect and one rule violation.

`continueProcess` cancelled `requestId` without dropping it, so the
`delay <= 0` branch carried an already-cancelled handle into `stop()` and
`pause()` cancelled it a second time — the same double-clear the comment two
lines above blames for four uncaught TypeErrors. Harmless in a browser, and it
contradicted the invariant that comment states.

The JSDoc on `componentRender` had grown to 33 lines, over this repo's own
twenty-line ceiling for a hover hint. The measurements, the reason mocks now
need their teardown half, and the note about the 293 hand-written mounts move
to `.github/contributing/testing.md`; the JSDoc keeps twelve lines and a
pointer. The same material is no longer duplicated in `console-gate.ts`.

Not changed: the three cases review flagged as newly vacuous. `with items` is
the reference render of its group, not a member that lost its distinction —
29 KB with eleven real menu items. It was held apart from `with color primary`
by one leaked `pointer-events: auto` byte, which recorded how many earlier
cases were still mounted rather than anything a colour does. `dropdown-menu`'s
theme puts colour on the trigger, which these fixtures do not render, so this
is the guard's own example of a genuine collision — a colour that only tints.
The siblings were already in the baseline for that reason.

Refs #87
@IgorShevchik
IgorShevchik merged commit 8c4ef59 into main Aug 30, 2026
3 checks passed
@IgorShevchik
IgorShevchik deleted the test/attach-and-shrink-snapshots branch August 30, 2026 03:39
IgorShevchik added a commit that referenced this pull request Aug 30, 2026
… unmount them (#516)

#512 attached the cases `renderEach` builds; the 293 hand-written
`mountSuspended` calls across 112 files stayed detached. None of those call
sites is edited. Each project sets the default where it already had a seam: the
`vue` project in its shim, the `nuxt` project through a `resolveId` pointing
`@vue/test-utils` at `test/utils/attach-mount.ts`. `@nuxt/test-utils/runtime` is
left alone so `mockNuxtImport` and `mockComponent` keep being transpiled.

`enableAutoUnmount(afterEach)` in both setup files is the other half: without it
each case leaves its tree in `document.body` and reka-ui's focus scope steals
the focus a later case just set. It is registered after the console gate, so the
gate is still watching when teardown runs.

Two things only became reachable once trees were in the document. reka-ui keeps
a `CSSStyleDeclaration` in a `ref`, which deep-wraps it in a proxy that
happy-dom's getters reject — four uncaught TypeErrors per run, now avoided by
handing the declaration back with `markRaw`. And axe's `aria-hidden-focus`
fires on `Select` and `Table`: measured on the DOM, the trigger sits inside an
`aria-hidden` region and stays focusable in both portal configurations, so the
rule is disabled with that recorded and with the note that the condition has
not gone away.

The console gate's register drops from 37 entries to 25; the dialog family
leaves it entirely.

Closes #513
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