Skip to content

fix(SelectMenu): honour searchInput autofocus false when the menu opens - #527

Merged
IgorShevchik merged 1 commit into
mainfrom
sync/nuxt-6caa6a95
Sep 1, 2026
Merged

IgorShevchik merged 1 commit into
mainfrom
sync/nuxt-6caa6a95

Conversation

@IgorShevchik

Copy link
Copy Markdown
Collaborator

Ports 6caa6a95. Cursor moves to it, which is nuxt/ui v4 HEAD — the sync is caught up again.

The bug

FocusScope trapped focuses the first focusable descendant when the menu opens, and that descendant is the search input — regardless of what searchInput.autofocus says. So :search-input="{ autofocus: false }" had no effect on open, and on a touch device the virtual keyboard came up anyway. The fix intercepts the scope's own event:

function onMountAutoFocus(event: Event) {
  if (searchInputProps.value.autofocus === false) {
    event.preventDefault()
  }
}

Applies verbatim. Our pre-image is upstream's down to the insertion point — onClear() immediately followed by const viewportRef = useTemplateRef('viewportRef') — and the docs page carries the same anchor paragraph with ### Content after it. Only the standing ui → b24ui class-binding rewrite applies.

The tests cover less than they look, and that is the finding

Upstream's two cases port structurally, with one selector change: this fork puts data-slot="input" on the control itself where upstream puts it on a wrapper, so [data-slot="input"] input matches nothing here and becomes input[data-slot="input"]. Left as upstream wrote it, the assertion fails on an empty wrapper rather than on the behaviour — red for the wrong reason.

The ported handler is not covered here. Measured, not assumed:

mutation result
remove @mount-auto-focus (revert the port) both green
invert the condition to !== false both green
preventDefault() unconditionally both green

Instrumenting onMountAutoFocus with a counter explains it: called zero times. Reka's FocusScope does not emit mountAutoFocus under happy-dom, so no test in this suite can reach that path.

What actually focuses the input here, read stage by stage:

stage document.activeElement
after mount BODY
after flushPromises DIV[data-slot=root]
after a macrotask INPUT[data-slot=input] — and only when autofocus is not false

Focus never lands on the input via FocusScope in this environment. It arrives later from Input.vue's own autofocus, and with autofocus: false that path simply does not fire — which is why upstream's symptom does not reproduce under happy-dom at all.

They are not vacuous, though — they guard a different thing. They pin the observable contract (searchInput: { autofocus: false } leaves the search input unfocused on open) through the Input.vue path. Verified by rewriting props.autofocus to true in Input.vue, which turns one of them red.

The port is still right. In a real browser FocusScope does focus that descendant, so upstream's bug reproduces here too; only the test environment cannot run the path. The handler is correct by construction and costs nothing. What is not claimed is coverage — the comment on the test block says so in as many words, so a later reader does not mistake green for verified.

Verification

lint · typecheck · build (3.86 MB) · docs:generate · test — 340 files, 7710 passed, 6 skipped · test:module.

docs:generate is in there because this commit touches a docs page and the ci gate never builds the docs site, so a break there would surface in the deploy rather than in the PR.


Generated by Claude Code

Ports `6caa6a95`.

`FocusScope trapped` focuses the first focusable descendant on open, which is the
search input, regardless of what `searchInput.autofocus` says — so
`:search-input="{ autofocus: false }"` had no effect there, and on a touch device
the virtual keyboard came up anyway. The handler intercepts the scope's own
event.

Applies verbatim: our pre-image is upstream's, down to the insertion point
between `onClear()` and `viewportRef`, and the docs page carries the same anchor.

The tests need saying plainly, because they cover less than they look. Upstream's
two cases port with one selector change — this fork puts `data-slot="input"` on
the control itself where upstream puts it on a wrapper, so the descendant form
matches nothing and would fail on an empty wrapper rather than on the behaviour.

But the ported handler is not covered here, and that was measured rather than
assumed: removing `@mount-auto-focus`, inverting its condition, and calling
`preventDefault()` unconditionally all leave both tests green, and instrumenting
`onMountAutoFocus` shows it called zero times — reka's `FocusScope` does not emit
`mountAutoFocus` under happy-dom. Focus reaches the input from `Input.vue`'s own
autofocus on a macrotask instead, which is why upstream's symptom does not
reproduce in this environment at all.

The tests are not vacuous; they guard a different thing. They pin the observable
contract — `autofocus: false` leaves the input unfocused on open — through the
Input.vue path, verified by rewriting `props.autofocus` to `true` there, which
turns one red.

The port is still right: in a browser `FocusScope` does focus that descendant, so
the bug reproduces here too and only the test environment cannot run the path.
Coverage is not claimed, and the comment on the test block says so, so a later
reader does not mistake green for verified.

Gate: lint, typecheck, build (3.86 MB), docs:generate, test (340 files, 7710
passed), test:module.
@IgorShevchik
IgorShevchik merged commit bdb8acb into main Sep 1, 2026
2 checks passed
@IgorShevchik
IgorShevchik deleted the sync/nuxt-6caa6a95 branch September 1, 2026 12:55
IgorShevchik added a commit that referenced this pull request Sep 1, 2026
`6caa6a95` went in with `pending-merge` in both `pr` and `b24ui_sha`, because
the entry is written in the same commit as the port. #527 squash-merged as
`bdb8acb2`; this points it there.

Bookkeeping only — no entry is added, the cursor does not move, and no decision
changes. The cursor is `6caa6a95`, which is nuxt/ui v4 HEAD, so the sync is
caught up and no entry carries the placeholder.
IgorShevchik added a commit that referenced this pull request Sep 5, 2026
Ports `c4ee0ea3` and `fbb9e220` — a contiguous pair taken in one PR per
PORTING.md §6 4b. The second is n/a and carries no change.

Nuxt 4.5 stopped prefetching `custom` links itself, and the regression here was
established from the installed source rather than a changelog: `nuxt-link.js` in
`nuxt@4.5.2` guards the visibility observer at :225 and the hover/focus handlers
at :297, both behind `!props.custom`. This component always renders
`<NuxtLink … custom>`, so neither fired — nothing errored and nothing warned,
navigations were simply slower than they had been. The same file exposes
`prefetched` and `shouldPrefetch` to the slot at :284, which makes this a
re-wiring rather than a reimplementation.

`src/runtime/utils/prefetch.ts` is upstream's, kept out of `utils/link.ts` so the
Vue builds — which share `LinkBase` and never render `NuxtLink` — do not load it.
Its three exports gained JSDoc blocks: upstream documents two with line comments
and the third not at all, which `jsdoc-coverage` rejects. Writing them pinned
down something the line comments left implicit — a missing handle in
`cancelIdleCallback` is a no-op, so an unmount before the callback was scheduled
needs no guard at the call site.

The tests can see this port, which is worth saying next to #527 earlier in this
queue, where the equivalent handler is never invoked under happy-dom. These
assert against the `link:prefetch` hook and drive the `IntersectionObserver` by
hand. Four mutations, each red on a different subset: reverting the port (3 of
5), disabling the visibility observer (1), dropping the caller's own listeners
from `mergeProps` (1), ignoring `shouldPrefetch('interaction')` (1).

`fbb9e220` is n/a: `src/runtime/components/prose/` holds 43 components and
`CodeTree` is not among them. Recorded in its log rather than acted on: this fork
has 43 prose components and no playground page for any of them, while upstream
now has one. That is a gap of ours, not a port.
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