Improve mobile input navigation #4647

[skjulestad]

Handle mobile completion across input types, preserve IME composition, and move focus without changing occurrences.

[Copilot]

Pull request overview

Improves mobile form navigation in form2 by introducing a consistent “mobile completion” callback for leaf inputs, preserving IME composition behavior, and moving focus forward (including across occurrences) without implicitly adding/removing occurrences.

Changes:

• Added mobile accessibility utilities to detect the next appropriate focus target and to handle Enter/Next key completion while respecting IME composition.
• Extended input-type props with onMobileComplete and updated multiple input components to opt into mobile “Next” behavior (enterKeyHint, onKeyDown, and data-mobile-focus-target).
• Updated InputField/OccurrenceList to wire mobile completion through occurrences and added new/updated Vitest coverage for the navigation behavior.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/main/resources/assets/admin/common/js/form2/utils/accessibility.ts	Adds mobile focus target discovery + Enter/IME-safe completion helper.
src/main/resources/assets/admin/common/js/form2/utils/accessibility.test.ts	Unit tests for new mobile accessibility helpers.
src/main/resources/assets/admin/common/js/form2/types.ts	Adds onMobileComplete to shared input component props.
src/main/resources/assets/admin/common/js/form2/components/time-input/TimeInput.tsx	Wires mobile completion to Enter key + picker confirmation.
src/main/resources/assets/admin/common/js/form2/components/text-line-input/TextLineInput.tsx	Adds mobile “Next” behavior and marks explicit focus target.
src/main/resources/assets/admin/common/js/form2/components/text-line-input/TextLineInput.test.ts	Tests enterKeyHint and Enter completion behavior.
src/main/resources/assets/admin/common/js/form2/components/text-area-input/TextAreaInput.tsx	Marks textarea as a mobile focus target.
src/main/resources/assets/admin/common/js/form2/components/radio-button-input/RadioButtonInput.tsx	Ensures a primary radio is the focus target/ref for mobile navigation.
src/main/resources/assets/admin/common/js/form2/components/occurrence-list/OccurrenceList.tsx	Threads onMobileComplete through occurrence rendering.
src/main/resources/assets/admin/common/js/form2/components/MobileInputBehavior.test.tsx	Adds integration-style tests for picker completion + mobile native controls.
src/main/resources/assets/admin/common/js/form2/components/long-input/LongInput.tsx	Adds mobile “Next” behavior and explicit focus target.
src/main/resources/assets/admin/common/js/form2/components/instant-input/InstantInput.tsx	Wires mobile completion to Enter key + picker confirmation.
src/main/resources/assets/admin/common/js/form2/components/input-field/InputField.tsx	Detects mobile via external store and moves focus on mobile completion (incl. within occurrences).
src/main/resources/assets/admin/common/js/form2/components/input-field/InputField.test.ts	Adds tests for mobile completion focus movement and non-occurrence changes.
src/main/resources/assets/admin/common/js/form2/components/geo-point-input/GeoPointInput.tsx	Adds mobile “Next” behavior and explicit focus target.
src/main/resources/assets/admin/common/js/form2/components/double-input/DoubleInput.tsx	Adds mobile “Next” behavior and explicit focus target.
src/main/resources/assets/admin/common/js/form2/components/date-time-input/DateTimeInput.tsx	Renders native datetime-local input on mobile and forces non-native picker on desktop.
src/main/resources/assets/admin/common/js/form2/components/date-input/DateInput.tsx	Renders native date input on mobile and forces non-native picker on desktop.
src/main/resources/assets/admin/common/js/form2/components/checkbox-input/CheckboxInput.tsx	Marks checkbox as a mobile focus target and forwards ref.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

[edloidas]

I ran this against the issue rather than reading it — Storybook at mobile viewports, reading rendered DOM and tabIndex values rather than JSX, plus the resolved @enonic/ui 1.1.1 bundle for what the pickers actually do with native. pnpm check is green on the branch: types, Biome, and 1105 tests.

The core mechanism holds up, and it is the right mechanism. data-mobile-focus-target genuinely beats the sortable list's tabIndex = -1 — I confirmed the explicit branch finds an input the fallback rejects, which is the whole point of adding it. The IME guard is correct and placed where every wired input goes through it. Completion never touches add or remove. And getInputRef already populates inputRefsRef for list mode, so the next-occurrence hop works wherever a leaf forwards its ref.

Three findings need a change before this ships and one is a scope question. Two of them are the same shape: the mechanism is right, but two of the input types it was built for do not participate — DateInput and DateTimeInput never get the marker or the completion callback, and everything added to InstantInput is inside a subtree the picker discards on mobile. The third is the mobile detection itself.

One withdrawal, because it shaped my first read: I had a reviewer report that TimeInput loses its value on mobile the same way, and that is wrong. TimePicker.Root returns composed children early and never consults native, so TimeInput keeps its own input and confirm button — measured at 375px, data-mobile-focus-target="true" present. DatePicker.Root has no such escape hatch, which is why only its consumers are affected.

Deliberately excluded: Copilot's open thread about [contenteditable="true"] — the selector is inconsistent with TagInput and SortableGridList, but nothing in this repo renders a bare contenteditable, so I have no failing case for it. The HH:MM truncation on the mobile DateTime path is pre-existing and shared with the desktop picker. TagInput and ComboBoxInput also lack the focus marker, but this branch does not touch them and that belongs in a follow-up.

One comment per finding, on the line it concerns. The two I would not merge without are the missing focus target on the Date/DateTime native branches and the unreachable wiring in InstantInput — both land directly on behaviour the issue names.

[edloidas]

The native date branch is missing the mobile contract, so a multi-occurrence Date field hands focus to the sortable row

// src/main/resources/assets/admin/common/js/form2/components/date-input/DateInput.tsx:84
type='date'
value={display}
onChange={handleInputChange}

Every other leaf input this branch touched got data-mobile-focus-target; the new native branches in DateInput and DateTimeInput did not, and neither component destructures onMobileComplete.

That marker is load-bearing. getMobileFocusTarget has two branches: the explicit one ignores tabIndex, the fallback requires tabIndex >= 0. Once a field has two or more occurrences, SortableGridList sets target.tabIndex = -1 on every navigable descendant (SortableGridList.tsx:302, gated on items.length >= 2) while the row wrapper keeps tabIndex 0. So without the marker the fallback rejects the inputs and picks the row.

Measured on Form/OccurrenceList → Required multiple at 375px — both occurrence inputs report tabIndex: -1 (saved original 0), the first row div reports tabIndex: 0. Running your own selector logic over that DOM:

const MFT = '[data-mobile-focus-target="true"]';
const f = document.querySelector('#storybook-root');
const pick = c => [...c.querySelectorAll(MFT)][0] ?? [...c.querySelectorAll('input, button, [tabindex]')].find(e => e.tabIndex >= 0);
pick(f).outerHTML.slice(0, 60);                      // -> <input ... data-mobile-focus-target="true"
f.querySelectorAll(MFT).forEach(e => e.removeAttribute('data-mobile-focus-target'));
pick(f).outerHTML.slice(0, 60);                      // -> <div tabindex="0" ...

Stripping the marker is exactly the DOM your mobile branch produces, and the result flips from the input to the row div. That is the failure the issue lists as current behavior to remove — "Focus may land on sortable rows or secondary controls instead of inner input" — still present for these two types.

Separately, with no onMobileComplete there is no enterKeyHint or handleMobileCompletionKeyDown, so Enter/Next does not leave a Date occurrence at all. ref={setInputRef} is wired here, so once completion is threaded the next-occurrence hop works — getInputRef already populates inputRefsRef for list mode.

Both branches are new on this branch (b110dec), so nothing here is pre-existing. Adding the marker to the two native inputs and threading onMobileComplete / enterKeyHint / handleMobileCompletionKeyDown the way DoubleInput and LongInput do would close it.

[edloidas]

On mobile none of this renders — DatePicker.Root discards its children unless native={false}

// src/main/resources/assets/admin/common/js/form2/components/instant-input/InstantInput.tsx:193
data-mobile-focus-target
aria-label={getInputAccessibleName(input, index)}
type='text'

In @enonic/ui 1.1.1 DatePicker.Root resolves native ?? getIsMobile() and then renders children: native ? <NativeInput/> : children. There is no escape hatch for composed children — TimePicker.Root has one, DatePicker.Root does not. DateInput and DateTimeInput got native={false} in this branch; InstantInput did not.

Measured at 375px — the entire rendered content of the Instant field is one element:

<input data-component="DatePicker.NativeInput" type="date" aria-label="Select date" min="1926-01-01" max="2036-12-31">

No text input, no trigger, no confirm button, data-mobile-focus-target absent, zero buttons in the subtree. So on mobile:

• setInputRef never fires, so the occurrence never lands in inputRefsRef. handleMobileComplete iterates that map, finds nothing for Instant occurrences, and falls through to getNextMobileFocusTarget — it leaves the field instead of moving to the next occurrence, against "Mobile Next/Enter moves to next existing occurrence, then next input field".
• data-mobile-focus-target, enterKeyHint, onKeyDown and the new onMobileComplete branch in handleSetTime are all unreachable, because the Input and the confirm Button are the discarded children.

Worth checking the test too: MobileInputBehavior.test.tsx's Instant case passes because it mocks getIsMobile to false while passing onMobileComplete, and InputField gates that prop on isMobile — so it asserts a prop combination production never produces, over a subtree that does not render on mobile.

To attribute it fairly: the native-only rendering is pre-existing, master omits native here as well, and losing time entry on mobile is not a regression you introduced. What is new on this branch is wiring that cannot run and a test standing guard over it. native={false} on line 168, matching the two siblings, would make the code you added reachable.

[edloidas]

isMobile and the picker library disagree on Chromium, so the feature is absent exactly where it will be tested

// src/main/resources/assets/admin/common/js/form2/components/input-field/InputField.tsx:195
const isMobile = useSyncExternalStore(subscribeToMobileChanges, getIsMobile);

useSyncExternalStore throws away the listener's argument and re-reads getSnapshot on every notification (preact/compat/src/hooks.js: subscribe(() => { if (didSnapshotChange(_instance)) forceUpdate(...) })). So the value is whatever getIsMobile() says, always — and the two halves of that pair do not use the same criterion:

• getIsMobile() returns navigator.userAgentData.mobile whenever it is a boolean, and only otherwise falls back to the media queries.
• subscribeToMobileChanges always reports (pointer: coarse) || (max-width: 640px).

In Chromium userAgentData.mobile is false on desktop no matter the viewport, so getSnapshot is pinned and no notification can move it. Measured at a 375px viewport, varying only that flag:

navigator.userAgentData	this isMobile	library's own
{mobile: true} (Android Chrome)	true	true
absent (iOS Safari, Firefox)	true	true
{mobile: false} (desktop Chromium, DevTools emulation)	false	true

Real phones are fine. The third row is desktop Chromium at ≤640px, Chromium tablets, and Chrome DevTools device emulation — and it is a split state, not just an inactive one: at 375px I measured DateInput and DateTimeInput rendering their desktop branch while @enonic/ui's own DatePicker went native in the same page, because the library tracks the subscription rather than getIsMobile. With isMobile false, onMobileComplete is undefined and no input gets the handler at all.

Attribution: the pattern is pre-existing — TagInput.tsx:843 on master already pairs these two the same way. What is new here is that it now gates the entire feature across three more components.

Mirroring what the library does internally (useState(getIsMobile) plus useEffect(() => subscribeToMobileChanges(setIsMobile), [])) demonstrably tracks the media queries. Since these two are shipped as a pair, it may be worth raising with the @enonic/ui owners rather than working around it four times.

[edloidas]

Was replacing the Date/DateTime widget on mobile part of the intent?

// src/main/resources/assets/admin/common/js/form2/components/date-time-input/DateTimeInput.tsx:146
type='datetime-local'
step={60}

The issue asks for completion and focus behaviour; nothing in it asks to change the control. This swaps the custom picker for the OS one on mobile, dropping the placeholder, the trigger and the popover.

It also leaves the four date/time types on three different mobile paradigms, which I measured at 375px: Date and DateTime go native, Instant renders a native date-only input (pre-existing, see the other comment), and Time keeps the custom popover — because TimePicker.Root returns composed children early and so never consults native.

If the swap is deliberate it is defensible; a native control is the reliable way to get a usable picker and a soft keyboard on a phone. Two things worth doing either way: say so in the PR description, since it is a visible change the issue did not request, and decide about Time — being the one type that behaves differently is the complaint the issue opens with. Not blocking, your call.