Skip to content

feat(NumberInput): a held stepper repeats into the draft and commits once - #8205

Merged
cixzhang merged 1 commit into
meta:mainfrom
vjeux:uikit/c04-stepper-hold
Nov 4, 2026
Merged

cixzhang merged 1 commit into
meta:mainfrom
vjeux:uikit/c04-stepper-hold

Conversation

@vjeux

@vjeux vjeux commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

User need

A stepper moves one step per click, so a far value takes many clicks. A held stepper should keep stepping, without one onChange per value.

Problem and solution fit

  • A mouse or pen press repeats the step from 0.5 s, then every 250 ms, each interval 0.9 of the last, down to 50 ms.
  • Each repeat shows in the field as its draft, without onChange.
  • Release or pointer cancel ends the hold and commits once through onChange. A field that turns disabled or read-only, or steppers that go away, end it with no commit. The pointer click after a release steps nothing; a pointerless click, such as a screen reader's, steps.
  • A press released before 0.5 s is a click; a finger keeps its single step; keyboard stepping is unchanged.

Current specification

component:NumberInput FR5 (steppers) now carries the owner's wording: "Holding a stepper with a mouse or a pen repeats the step, the first repeat after 0.5 s and then faster; each repeated value shows as the field's draft, and the field commits once, through onChange, when the hold ends by release or pointer cancel. A single click steps and commits at once." A verification row names the "a held stepper" suite; the longer FR5 cell re-pads the table (15 whitespace-only rows).

Public delta and caller burden

No new prop, type, default or theme surface. Callers see one onChange per mouse or pen hold.

Design and alternatives

Pointer capture on the two buttons, so a release anywhere ends the hold. Each hold keeps its pointer and draft; a pen's hold takes the shared selection lock. Rejected: a commit per repeat.

Evidence

  • Red first: on main the hold, cancel, bound and no-pointer-click cases fail. The mid-hold editability, steppers-removed, pen-lock and second-pointer cases fail on builds without their guards.
  • Real-Chromium probe, not part of this change ("With number steppers" story): a click goes 2 → 3; a hold reads 3, 4, 6, 7 at 0.3–1.3 s; a release 200 px off the button keeps 7; main stays at 3.

Scope

  • One capability is added.
  • Unrelated fixes, cleanup, and policy changes were removed or split.
  • Consumer docs, focused tests, and migration evidence ship with the feature.
  • Public text and artifacts contain no internal Meta context.

Writing rubric results

R1 narration: none; the owner's "as before" was dropped.
R2 research: none in the record.
R3 evidence: the new verification row names the "a held stepper" suite.
R4 rejected alternatives: none in the record.
R5 change references: none.
R6 open questions: none added.

Testing

At c2528eb on main 252b45b:

  • vitest run --project ui packages/core/src/NumberInput packages/core/src/InputGroup: 7 files, 370 tests passed.
  • vitest run --project node packages/cli/foundation/discovery packages/core/src/__tests__: 39 files, 1,005 tests passed.
  • Core tsc --noEmit, pnpm check:knowledge, check:changesets, Prettier and eslint (CI=true) pass. pnpm lint reports only the BaseProps unused-import warning that main already has. pnpm review:pre: 0 findings.

@vercel

vercel Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Nov 4, 2026 6:58pm UTC

Request Review

@astracat-bot

astracat-bot Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

AI review status for this pull request.

Review status Updated
🟢 Reviewed (for maintainers only) Nov 4, 2026, 7:23 PM UTC

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 24, 2026
@vjeux
vjeux force-pushed the uikit/c04-stepper-hold branch from bd65b4e to c76f574 Compare October 24, 2026 11:01
@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

🔎 Preview · c2528eb

Storybook · Sandbox · built from this PR's head

Modified components

Component Size LoC Visual · A11y · RTL
NumberInput · Storybook 56.5 kB (+1.3 kB) · 10.2 kB CSS (0) 1,339 (+194) ✓ · ✓ · ✓
Pagination 89.2 kB (+1.3 kB) · 16.5 kB CSS (0) — — · — · —
PowerSearch 244.9 kB (+871 B) · 22.2 kB CSS (0) — — · — · —

Package size: @astryxdesign/core 6.42 MB packed (+4.2 kB); no new dependencies.

API changes: none in this pull request

Committed API report behind this build: cli 8 rows, core 2 rows; run pnpm api:report.

Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.


Generated by PR Enrichment workflow | View full report

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs an engineering owner's decision before it can be approved, and it needs changes either way. The stepper hold and a held stepper calling onChange on every step rest only on the draft spec:AST-094 (FR1, FR2, FR11, FR16, FR20 and FR21); an engineering owner records approval of those claims by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently, please:

  • land the stacked predecessors first (or rebase onto them) so this pull request carries only its own 1,721-line commit instead of 26,788 lines;
  • attach the stepper column's press tracker whenever that node is created or replaced, because steppers shown after mount never get the hold;
  • stop the stepper's click guard from dropping a detail === 0 activation after a touch press that ended without a click.

[Automated review]

Comment on lines +937 to +943
if (
(pointerType === 'touch' || pointerType === 'pen') &&
stepperTrackedPressRef.current
) {
stepperTrackedPressRef.current = false;
return;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both refs are set at touch-down and cleared only here, by a click. A hold in Chromium, a slide off the column or a scroll ends with no click, so the next click with detail === 0 and no pointerdown (a programmatic click(), or assistive technology that sends no pointer events) is dropped: in Chromium touch emulation click() after a 1.6 s hold left the value at 5, where the base steps 2 to 3. Checking event.detail > 0 here, and clearing both refs at the lift or cancel when no click is owed, keeps those activations.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a76cd60. The guard skips only a click a pointer produced (detail > 0) and clears its flag at every click and when a touch is cancelled. A lift outside keeps the flag, because iOS Safari still sends the button its click after a short slide off. Tests that fail on the previous head: jsdom a click with no pointer after a touch that ended without one still steps and Chromium after a scroll takes the touch, a click with no pointer still steps. A guard against over-clearing: the click an engine sends after a lift outside adds nothing.

detach();
tracker.dispose();
};
}, [ref, tracker]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keyed [ref, tracker], this attaches only the node that is in the ref at mount. NumberInput's stepper column renders under hasNumberSteppers, so a column that appears later, or is removed and re-created, is never attached (a 1.2 s hold makes no step and no paint in a jsdom probe), and the removed node stays registered. A callback ref, or a child component that owns the column and calls usePressTracker, would follow node replacement.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a76cd60: a StepperColumn child owns the column and calls usePressTracker, so each column the field mounts gets its own tracker on its own node. Keying a new ref object per node in NumberInput can't work with this hook today, because its getBounds closure keeps the first ref object, so a node-level fix in the hook is the better long-term home (it is open separately). Tests that fail on the previous head: steppers turned on after mount follow a finger and steppers removed and shown again follow a finger, once.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on an engineering owner's decision on the draft spec:AST-094 (FR1, FR2, FR11, FR16, FR20 and FR21, including the stepper's 0 pt tracking margin against FR11's 70 pt) and on whether a held stepper should call onChange on every step; an engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision, these need changes:

  • usePressTracker attaches only to the stepper column present at mount, so steppers turned on later never follow a finger (useGestureRecognizer.ts:127-140).
  • After a touch press that ends without a click, the next detail === 0 activation is dropped (NumberInput.tsx:937-943), which the draft's own FR21 forbids.
  • Lint fails at NumberInput.stepperTouch.test.tsx:263, and the branch carries 26,824 changed lines of stacked work; landing #8052 to #8202 first leaves this pull request with the 1,738-line hold.

[Automated review]

@vjeux
vjeux force-pushed the uikit/c04-stepper-hold branch from 9159dba to 2d89560 Compare October 24, 2026 20:53
@vjeux vjeux changed the title feat(NumberInput): a finger held on a stepper keeps stepping, as UIStepper does feat(NumberInput): on iOS a finger held on a stepper keeps stepping, as UIStepper does Oct 24, 2026

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on an engineering owner's decision on the draft spec:AST-094 for NumberInput's touch stepper hold (FR1, FR2, FR11, FR13, FR16, FR20 and FR21, including the stepper's 0 pt tracking margin and its iOS-only gate), on whether a held stepper calls onChange on every step, and on whether FR4 stands beside the theme motion roles this branch carries; an engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision, these need changes:

  • usePressTracker attaches only to the stepper column present at mount, so steppers shown later never follow a finger (useGestureRecognizer.ts:131-144).
  • After a touch press that ends without a click, the next detail === 0 activation is dropped (NumberInput.tsx:935-941), which the draft's own FR21 forbids.
  • Lint fails at NumberInput.stepperTouch.test.tsx:291, three changesets say no component uses modules NumberInput now uses, and the branch carries 31,025 authored lines of stacked work; landing the predecessors first leaves this pull request with the 2,027-line hold.

[Automated review]

const {onChange, increment} = setup({initial: 8, max: 10});
const id = fingerDown(increment, INCREMENT);
wait(2600);
expect(onChange.mock.calls.map(([value]) => value)).toEqual([9, 10]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI's lint job fails here: @typescript-eslint/promise-function-async reads ([value]) => value as possibly returning a promise because mock.calls is untyped. Typing the parameter (or the mock) clears it.

Comment thread .changeset/gesture-host.md Outdated
'@astryxdesign/core': patch
---

[chore] Internal: the gesture system's DOM host (claims, click suppression, the browser's takeover, the pointer behind a contact). Not exported from the package entry point; no component uses it yet.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"no component uses it yet" no longer holds: NumberInput attaches the host through usePressTracker, and touch-experimental-subpaths.md in the same release exports the press tracker publicly. gesture-press-tracking.md and uikit-constants-table.md say the same; release readers get both statements.

@vjeux

vjeux commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Two notes on the reviews above. Lint: the strict promise-function-async error at NumberInput.stepperTouch.test.tsx is fixed in a76cd60 (typed onChange mock); ASTRYX_STRICT_LINT=1 eslint is clean on the NumberInput folder. Size: the diff is the stack, not this change. Each predecessor is its own open pull request, listed in merge order at the end of the description (#8270 with its chain, #8173, #8202). This pull request's own change is its last commit, about 2.1k lines with tests; once the predecessors merge, the diff is that commit alone.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on an engineering owner's decision on the draft spec:AST-094 for NumberInput's touch stepper hold (FR1, FR2, FR11, FR13, FR16, FR20 and FR21, including the stepper's 0 pt tracking margin and its iOS-only gate), on whether a held stepper calls onChange on every step, and on whether FR4 stands beside the theme motion roles this branch carries; an engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. The two input-loss issues from the previous revision are fixed and tested, and these still need changes whatever that decision is:

  • NumberInput's import grows by about 18.9 KB gzip of touch-only gesture code (NumberInput.tsx:57-68), which architecture:component-style-authoring INV6 asks to load lazily or justify.
  • The touch resolver keeps its own (width < 744px) / (height < 500px) numbers (touchPresentation.ts:75-79), where spec:AST-012 FR9 requires the theme's compact definition (width below the effective md point, 768 by default).
  • StepperColumn sits between NumberInput's doc comment and export function NumberInput, so the exported component loses its JSDoc (NumberInput.tsx:541-557).
  • Three changesets still say no component uses the gesture host, press tracking or the UIKit table, and the hold changeset leaves out the 50 ms phase.
  • The branch carries 31,196 authored lines of stacked work; landing the predecessors first leaves this pull request with the 2,199-line hold.

[Automated review]

export const TOUCH_QUERY = '(pointer: coarse)';

/** The viewport is compact: layout behaviours. */
export const COMPACT_QUERY = '(width < 744px), (height < 500px)';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the resolver's own compact numbers. The current spec:AST-012 FR9 defines compact as width below the theme's effective md point (768 by default) or height below 500, and says the web touch presentation resolver and Selector's adaptive presentation keep no size numbers of their own. With 744 here, a coarse-pointer viewport 744–767 px wide (an iPad mini in portrait) gets the popover where the shared definition gives it the sheet. Deriving the width from the theme's md point (the compact definition main now exports) would conform.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resolver belongs to the base (#8062, folded into #8076). At #8076 a71f4e5, which this PR now stacks on, its hook reads compact from the theme's md point, and the 744 is gone; the static media constant stays the default, which is the resolver's own question. NumberInput keys on the coarse pointer alone and reads no compact size.

type PointerKind,
type PressTracker,
type UsePressTrackerOptions,
} from '../gestures';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These static imports put the gesture host, arena, recognizers and UIKit tables into NumberInput's import for every consumer: an esbuild bundle of NumberInput grows from 34,464 to 53,860 bytes gzip. architecture:component-style-authoring INV6 flags a coarse-pointer-only feature above 2 KB gzip and asks the pull request to load it lazily or justify the cost.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The size report is in the description now: NumberInput's bundle (esbuild, minified + gzip, React and StyleX external) is 56.2 KB at the base and 76.0 KB with #8205 (478b4f9). About 2 KB of the difference is NumberInput's own touch code (NumberInputTouch.tsx, stepperTracking.ts and the stepper table); the rest is the shared gesture system (the host, arena, press tracker and the gesture table the host reads itself), which every touch control on a page shares. It stays a static import on purpose: a lazy chunk would add a network round trip at the first touch, on exactly the devices that need it, and the touch layer's single import site lets a desktop build drop it. The tables are now imported by name, from their own modules; NumberInput reads the stepper table and one row of the gesture table.

* <NumberInput label="Price" value={price} onChange={setPrice} min={0} step={0.01} />
* ```
*/
/**

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StepperColumn now sits between NumberInput's doc comment ("A number input component for collecting numeric user input…" with its @example, lines 541-551) and export function NumberInput, so that comment documents the private StepperColumn and the exported component has none in editor hovers or the emitted declarations. Moving StepperColumn above the doc comment, or below NumberInput, keeps it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed: StepperColumn is gone (the press tracker hook now attaches to a node that mounts later, so the column needs no component of its own), and the doc comment sits directly on NumberInput again (478b4f9).

this.steps += 1;
}
// Timed from after this step's work, as UIKit re-arms its timer.
this.arm(this.clock.now() + repeatInterval(this.steps));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This re-arms even when step refused (the field turned disabled or read-only mid-hold), and PressTracker reads isEnabled only when a touch begins, so the timer keeps firing and + stays painted pressed beside aria-disabled until the finger lifts. Cancelling the tracking and clearing the paint when the field becomes disabled during a press would release it (architecture:react-component-runtime INV5).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 478b4f9: when the field turns disabled or read-only during a press, the touch layer lets go of it at once. It cancels the tracking and clears the paint, and nothing tracks or paints again until the next touch. New test: a field turned disabled|read-only mid-hold lets go of the press at once, which fails on the previous head with the half still painted. A step refused at a bound still keeps the clock, so a slide onto the other half carries the cadence, as UIStepper's does.

Comment thread .changeset/number-input-stepper-hold.md Outdated
'@astryxdesign/core': patch
---

[feat] NumberInput: on iPhone and iPad, a finger held on a number stepper keeps stepping, as a native iOS stepper does: once after half a second, then every half second, and every tenth of a second after three seconds. A tap steps once, when the finger lifts; sliding onto the other stepper carries the repeat to it, and sliding off the steppers stops it. The pressed stepper lights at once and clears at once. A scroll that starts on a stepper still scrolls and steps nothing. Android and other touch screens keep the tap as before (Material has no stepper), and a mouse click is unchanged. A held finger selects no text on any touch screen: the stepper column leaves that to the gesture system's press tracking instead of styles of its own.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hold also drops to every 50 ms from the twentieth step (stepper.autorepeat.fastIntervalMs), which this line leaves out; release readers would expect a tenth of a second to be the top rate.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 478b4f9: the changeset now names the pen and the top rate (every twentieth of a second from the twentieth step).

vjeux added a commit to vjeux/astryx that referenced this pull request Oct 25, 2026
Android 15, Chrome 124 (emulator golden v2, recorded at 540 x 1200), on
this push's heads (meta#8205 40690a8, meta#8218 7615c44): meta#8205 beside main
(a tap steps once at the lift without focusing; a 3.45 s hold steps on
UIKit's cadence to 10), and meta#8218 beside meta#8205 (the capsule: - then +, a
1.3 s hold, and a tap on + while Gboard is up). Every take passes the gap
gate (no frame gap over 60 ms after the recorder warm-up); every frame
reviewed; the bands start below Chrome's address bar.
@vjeux
vjeux force-pushed the uikit/c04-stepper-hold branch from a76cd60 to 40690a8 Compare October 25, 2026 01:09
@vjeux vjeux changed the title feat(NumberInput): on iOS a finger held on a stepper keeps stepping, as UIStepper does feat(NumberInput): a finger held on a stepper keeps stepping, as UIStepper does Oct 25, 2026
@github-actions github-actions Bot added the needs:design-review Affects visuals — Design should review label Oct 25, 2026
@github-actions
github-actions Bot requested review from cvkxx and ernestt October 25, 2026 01:09
vjeux added a commit to vjeux/astryx that referenced this pull request Oct 27, 2026
vjeux added a commit to vjeux/astryx that referenced this pull request Oct 27, 2026
vjeux added a commit to vjeux/astryx that referenced this pull request Oct 27, 2026
…he column press follows it)

NumberInput.spec.md takes meta#8205's record (meta#8173's FR5 and the hold) with this
PR's capsule amendment re-applied (FR5 placement, the Steppers design row,
the verification row and verified_by).
vjeux added a commit to vjeux/astryx that referenced this pull request Oct 27, 2026
vjeux added a commit to vjeux/astryx that referenced this pull request Oct 27, 2026

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on an engineering owner's decision on this pull request's rewrite of component:NumberInput FR4 and FR5: the stepper hold and its single onChange at the release, the 250 ms/0.9 ramp and 70 px tracking area (which spec:AST-094 FR1 and FR2 allow only through a dated owner decision naming the UIStepper measurement they depart from), the focus rule by pointer kind, the desktop mouse hold, and presses that cross between the halves. An engineering owner records it by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), or says which behaviour must change. Whatever the decision, these still need changes:

  • Make FR4 match the code: under a finger the hold keeps stepping up to 70 px past the column, where FR4 says moving off pauses it.
  • Import UIKIT_CONTROL by name in useGestureRecognizer.ts:27, so NumberInput stops shipping every gesture row (architecture:component-style-authoring INV6).
  • Add real-device iOS Safari and Android Chrome runs of a tap, an edge tap and a hold (spec:AST-094 Platform support).
  • Land the predecessors first: the diff carries 32,441 authored lines, of which this pull request's own change is 3,556.

[Automated review]

| FR1 | The input is a native text input with `role="spinbutton"`, `inputMode` `numeric` when integer-only and `decimal` otherwise, `aria-valuemin` and `aria-valuemax` from the range, `aria-valuenow` from the committed `value`, and `aria-valuetext` set to the formatted value when `formatValue` is supplied. | Current tests |
| FR2 | Typing changes only a draft; `onChange` is not called while typing. A draft is valid when it parses for the active locale (grouping, decimal separator, digit script, and sign) and, for validity display, lies within the range and meets integer-only. An invalid draft colors the text secondary, sets `aria-invalid`, and fills a persistent assertive region with the localized invalid-number message. | `spec:AST-086` FR3; `architecture:internationalization`; current tests |
| FR3 | On blur the draft resolves once and ends: an empty draft clears (`onChange(null)` when clearable and a value exists) or reverts; an unparsable or wrong-kind draft reverts; otherwise the number clamps to the range and commits with `onChange(number)` when it differs from `value`. Enter resolves a valid draft the same way and clears an empty draft when clearable. Enter commits nothing for an empty draft without `hasClear`, which stays in the field and is not marked invalid, or for an unparsable or wrong-kind draft, which stays in the field and stays invalid. Enter ends the draft only when its commit clamped. In a field that is not disabled, every Enter outside composition then calls `onEnter`, whatever it committed. | Current tests |
| FR4 | ArrowUp and ArrowDown without modifiers, the wheel over the focused input (when `isWheelEnabled`), and a stepper press step by one grid position and commit at once. A held stepper repeats once after 0.5 s, then on a ramp whose interval starts at 250 ms and shortens to 0.9 of the last, down to 50 ms; the input shows each step, and the press commits once, at its release. Moving off the stepper pauses the repeat and moving back resumes it with no immediate step; sliding onto the other stepper carries it on; a held press selects no text and opens no callout (`spec:AST-094` FR41). The grid starts at `min` (when it suits integer-only) or 0. From an empty field or an emptied draft, up goes to `min` or 0 and down to `max` or 0. Results round to the finer of the step's and the grid start's precision (at most 12 decimal places), clamp to the range, and never leave integer-only; a stepper that cannot move does nothing, while an arrow or wheel step that cannot move still ends the draft, committing a valid draft's number and dropping an unparsable one. A caller `onKeyDown` that prevents an arrow cancels that step. A wheel step stops the page from scrolling; otherwise the page scrolls. | UIStepper's 0.5 s delay and 50 ms floor, measured on the iOS 26.2 simulator; `spec:AST-094` FR41; current tests |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under a finger the press tracker keeps this press inside up to 70 px past the column (UIKIT_CONTROL.trackingExpansion, PressTracker.ts:322-334), so the hold keeps stepping while the finger rests on the input's text; only a mouse pauses at the column's edge. State that area (or track within the column), and name the owner decision for the 250 ms/0.9 ramp in the Basis: the stepper table marks both as guesses pending review (M-STP20).


import {useEffect, useMemo, useRef, useState, type RefObject} from 'react';
import {useIsomorphicLayoutEffect} from '../hooks/useIsomorphicLayoutEffect';
import {UIKIT_GESTURES} from '../motion/uikit/gestures';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usePressTracker reads UIKIT_GESTURES.control (:298), and UIKIT_GESTURES holds every gesture group, so every NumberInput import ships the swipe, pinch, rotation, screen-edge and velocity rows (architecture:component-style-authoring INV6). Import UIKIT_CONTROL by name, as PressTracker.ts:47 does.

pressed fill (one frame on, one frame off, no animation) and the haptics
(none). NumberInput departs from three of them: leaving the frame pauses
the cadence and coming back resumes it (never a step at once), the press
keeps tracking within the control profile's expanded area (AST-094 FR11),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spec:AST-094 FR11 states no tracking area; cite UIKIT_CONTROL.trackingExpansion and M-STP20 here, as NumberInputTouch.tsx:243-248 does.

isReleased: false,
});
const latestRef = useRef(state);
useIsomorphicLayoutEffect(() => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Effect, the disposal at :167 and the tracker-ref Effect at :467 name no external system they synchronize (architecture:react-component-runtime INV3). One comment each, or move the ref writes into the handlers that read them.

vjeux added a commit to vjeux/astryx that referenced this pull request Oct 27, 2026
vjeux added a commit to vjeux/astryx that referenced this pull request Oct 27, 2026

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on an engineering owner's decision on this pull request's rewrite of component:NumberInput FR4 and FR5: the stepper hold and its single onChange at the release, the 250 ms/0.9 ramp and 70 px tracking area (which spec:AST-094 FR1 and FR2 allow only through a dated owner decision naming the UIStepper measurement they depart from), the focus rule by pointer kind, the desktop mouse hold, and presses that cross between the halves. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), or says which behaviour must change. Whatever the decision, these still need changes:

  • Give the stepper column's click handler (NumberInput.tsx:1022-1036) the same disabled, read-only and bound guard as a stepper's, since on desktop Chromium a mouse click lands on the column and commits a draft sitting at the bound through an aria-disabled stepper.
  • Make FR4 match the code: under a finger the hold keeps stepping up to 70 px past the column, where FR4 says moving off pauses it.
  • Import UIKIT_CONTROL by name in useGestureRecognizer.ts:27 (architecture:component-style-authoring INV6), and add real-device iOS Safari and Android Chrome runs of a tap, an edge tap and a hold (spec:AST-094 Platform support).
  • Land the predecessors first, since their List, checkbox and radio rows that grow under a finger also contradict spec:AST-094 FR42.

[Automated review]

focusAfterPress(pointerType, event);
}
const click = stepperTouch.takeClick(event.nativeEvent);
if (click === 'increment' || click === 'decrement') {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under a fine pointer the column captures the pointer (NumberInputTouch.tsx:409-411), so Chromium sends every mouse click here rather than to the button, and this path steps without the disabled, read-only and bound guard pressStepper applies (:1003-1009). In Chromium at 1280 px, a draft of 10 with max 10 and a mouse press on the aria-disabled + commits onChange(10); on main it does nothing (component:NumberInput FR4: "a stepper that cannot move does nothing"). The same guard here, and a browser test of a mouse click at a bound, would close it.

| FR1 | The input is a native text input with `role="spinbutton"`, `inputMode` `numeric` when integer-only and `decimal` otherwise, `aria-valuemin` and `aria-valuemax` from the range, `aria-valuenow` from the committed `value`, and `aria-valuetext` set to the formatted value when `formatValue` is supplied. | Current tests |
| FR2 | Typing changes only a draft; `onChange` is not called while typing. A draft is valid when it parses for the active locale (grouping, decimal separator, digit script, and sign) and, for validity display, lies within the range and meets integer-only. An invalid draft colors the text secondary, sets `aria-invalid`, and fills a persistent assertive region with the localized invalid-number message. | `spec:AST-086` FR3; `architecture:internationalization`; current tests |
| FR3 | On blur the draft resolves once and ends: an empty draft clears (`onChange(null)` when clearable and a value exists) or reverts; an unparsable or wrong-kind draft reverts; otherwise the number clamps to the range and commits with `onChange(number)` when it differs from `value`. Enter resolves a valid draft the same way and clears an empty draft when clearable. Enter commits nothing for an empty draft without `hasClear`, which stays in the field and is not marked invalid, or for an unparsable or wrong-kind draft, which stays in the field and stays invalid. Enter ends the draft only when its commit clamped. In a field that is not disabled, every Enter outside composition then calls `onEnter`, whatever it committed. | Current tests |
| FR4 | ArrowUp and ArrowDown without modifiers, the wheel over the focused input (when `isWheelEnabled`), and a stepper press step by one grid position and commit at once. A held stepper repeats once after 0.5 s, then on a ramp whose interval starts at 250 ms and shortens to 0.9 of the last, down to 50 ms; the input shows each step, and the press commits once, at its release. Moving off the stepper pauses the repeat and moving back resumes it with no immediate step; sliding onto the other stepper carries it on; a held press selects no text and opens no callout (`spec:AST-094` FR41). The grid starts at `min` (when it suits integer-only) or 0. From an empty field or an emptied draft, up goes to `min` or 0 and down to `max` or 0. Results round to the finer of the step's and the grid start's precision (at most 12 decimal places), clamp to the range, and never leave integer-only; a stepper that cannot move does nothing, while an arrow or wheel step that cannot move still ends the draft, committing a valid draft's number and dropping an unparsable one. A caller `onKeyDown` that prevents an arrow cancels that step. A wheel step stops the page from scrolling; otherwise the page scrolls. | UIStepper's 0.5 s delay and 50 ms floor, measured on the iOS 26.2 simulator; `spec:AST-094` FR41; current tests |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Moving off the stepper pauses the repeat" holds for a mouse, but under a finger the press tracker keeps the press inside up to 70 px past the column (UIKIT_CONTROL.trackingExpansion, PressTracker.ts:322-334), so the hold keeps stepping while the finger rests on the input's text. State that area (or track within the column), and name the owner decision for the 250 ms/0.9 ramp in the Basis: the stepper table marks both as proposed, pending review (M-STP20).


import {useEffect, useMemo, useRef, useState, type RefObject} from 'react';
import {useIsomorphicLayoutEffect} from '../hooks/useIsomorphicLayoutEffect';
import {UIKIT_GESTURES} from '../motion/uikit/gestures';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usePressTracker reads UIKIT_GESTURES.control (:298), and UIKIT_GESTURES holds every gesture group, so every NumberInput import ships the swipe, pinch, rotation, screen-edge and velocity rows (architecture:component-style-authoring INV6). Importing UIKIT_CONTROL by name, as PressTracker.ts:47 does, fixes it here.

},
}),
);
useEffect(() => () => tracking.dispose(), [tracking]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Effect, the latest-state Effect at :145-147 and the tracker-ref Effect at :467-469 name no external system they synchronize (architecture:react-component-runtime INV3); the two that let go of a press already do. One comment each, or move the ref writes into the handlers that read them.

pressed fill (one frame on, one frame off, no animation) and the haptics
(none). NumberInput departs from three of them: leaving the frame pauses
the cadence and coming back resumes it (never a step at once), the press
keeps tracking within the control profile's expanded area (AST-094 FR11),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spec:AST-094 FR11 states no tracking area; cite UIKIT_CONTROL.trackingExpansion and M-STP20 here, as NumberInputTouch.tsx:243-248 does.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on an engineering owner's decision on this pull request's rewrite of component:NumberInput FR4 and FR5: the stepper hold and its single onChange at the release, the 250 ms/0.9 ramp and 70 px tracking area (which spec:AST-094 FR2 allows only through a dated owner decision naming the UIStepper measurement they depart from), the focus rule by pointer kind, the desktop mouse hold, and presses that cross between the halves. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), or says which behaviour must change. Whatever the decision, these still need changes:

  • Give the stepper column's click handler (NumberInput.tsx:1022-1036) the same disabled, read-only and bound guard as a stepper's, since in Chromium a mouse click lands on the column and commits a draft sitting at the bound through an aria-disabled stepper.
  • Make FR4, the docs and the changeset match the code: under a finger the hold keeps stepping up to 70 px past the column, where they say moving off pauses it.
  • Land the predecessors first (the diff carries 32,733 authored lines, of which this change is 3,556), with their List, checkbox and radio row growth conformed to spec:AST-094 FR42.
  • Import UIKIT_CONTROL by name in useGestureRecognizer.ts:27, and add real-device iOS Safari and Android Chrome runs of a tap, an edge tap and a hold (spec:AST-094 Platform support).

[Automated review]

Comment on lines +1032 to +1035
const click = stepperTouch.takeClick(event.nativeEvent);
if (click === 'increment' || click === 'decrement') {
stepValue(click === 'increment' ? 1 : -1);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This steps without the guard pressStepper applies at :1001-1009. Under a fine pointer the column captures the pointer (NumberInputTouch.tsx:409-411), so in Chromium a mouse click on an aria-disabled stepper lands here, and stepValue commits a draft sitting at the bound, where component:NumberInput FR4 says "a stepper that cannot move does nothing". Apply the same disabled, read-only and bound guard here, with a Chromium test of a click at the bound with a draft.

| FR1 | The input is a native text input with `role="spinbutton"`, `inputMode` `numeric` when integer-only and `decimal` otherwise, `aria-valuemin` and `aria-valuemax` from the range, `aria-valuenow` from the committed `value`, and `aria-valuetext` set to the formatted value when `formatValue` is supplied. | Current tests |
| FR2 | Typing changes only a draft; `onChange` is not called while typing. A draft is valid when it parses for the active locale (grouping, decimal separator, digit script, and sign) and, for validity display, lies within the range and meets integer-only. An invalid draft colors the text secondary, sets `aria-invalid`, and fills a persistent assertive region with the localized invalid-number message. | `spec:AST-086` FR3; `architecture:internationalization`; current tests |
| FR3 | On blur the draft resolves once and ends: an empty draft clears (`onChange(null)` when clearable and a value exists) or reverts; an unparsable or wrong-kind draft reverts; otherwise the number clamps to the range and commits with `onChange(number)` when it differs from `value`. Enter resolves a valid draft the same way and clears an empty draft when clearable. Enter commits nothing for an empty draft without `hasClear`, which stays in the field and is not marked invalid, or for an unparsable or wrong-kind draft, which stays in the field and stays invalid. Enter ends the draft only when its commit clamped. In a field that is not disabled, every Enter outside composition then calls `onEnter`, whatever it committed. | Current tests |
| FR4 | ArrowUp and ArrowDown without modifiers, the wheel over the focused input (when `isWheelEnabled`), and a stepper press step by one grid position and commit at once. A held stepper repeats once after 0.5 s, then on a ramp whose interval starts at 250 ms and shortens to 0.9 of the last, down to 50 ms; the input shows each step, and the press commits once, at its release. Moving off the stepper pauses the repeat and moving back resumes it with no immediate step; sliding onto the other stepper carries it on; a held press selects no text and opens no callout (`spec:AST-094` FR41). The grid starts at `min` (when it suits integer-only) or 0. From an empty field or an emptied draft, up goes to `min` or 0 and down to `max` or 0. Results round to the finer of the step's and the grid start's precision (at most 12 decimal places), clamp to the range, and never leave integer-only; a stepper that cannot move does nothing, while an arrow or wheel step that cannot move still ends the draft, committing a valid draft's number and dropping an unparsable one. A caller `onKeyDown` that prevents an arrow cancels that step. A wheel step stops the page from scrolling; otherwise the page scrolls. | UIStepper's 0.5 s delay and 50 ms floor, measured on the iOS 26.2 simulator; `spec:AST-094` FR41; current tests |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Moving off the stepper pauses the repeat" holds for a mouse, but under a finger the press tracker keeps the press inside up to 70 px past the column (NumberInputTouch.tsx:243-248, PressTracker.ts:323-334), so the hold keeps stepping over the input's text; the browser test moves 100 px away to pause it. State the finger's tracking area here, in the docs and in the changeset, or pause at the column.


import {useEffect, useMemo, useRef, useState, type RefObject} from 'react';
import {useIsomorphicLayoutEffect} from '../hooks/useIsomorphicLayoutEffect';
import {UIKIT_GESTURES} from '../motion/uikit/gestures';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UIKIT_GESTURES holds every gesture group (motion/uikit/gestures.ts:303), so reading .control at :298 ships the swipe, pinch, rotation and screen-edge rows with every NumberInput (architecture:component-style-authoring INV6). Import UIKIT_CONTROL by name.

isReleased: false,
});
const latestRef = useRef(state);
useIsomorphicLayoutEffect(() => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Effect, the disposal at :167 and the tracker-ref Effect at :467-469 name no external system they synchronize (architecture:react-component-runtime INV3); the two that let go of a press already do. One comment each, or move the ref writes into the handlers that read them.

pressed fill (one frame on, one frame off, no animation) and the haptics
(none). NumberInput departs from three of them: leaving the frame pauses
the cadence and coming back resumes it (never a step at once), the press
keeps tracking within the control profile's expanded area (AST-094 FR11),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spec:AST-094 FR11 states no tracking area; cite UIKIT_CONTROL.trackingExpansion and M-STP20 here, as NumberInputTouch.tsx:243-248 does.

touchOneLine: {
minHeight: {
default: null,
[TOUCH]: listRowConsts.touchRowHeight,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spec:AST-094 FR42 (current) says the default look makes no row-height increase for touch and reaches 44 px through an invisible hit area; this touch-only minHeight, like Indicator/touchTarget.stylex.ts:30, grows rows on every phone. Conform in the predecessor before this diff carries it.

@cixzhang

Copy link
Copy Markdown
Contributor

Posted by Cindy's agent on her behalf: closed and reopened only to restart this PR's CI on the current workflow after tonight's queue-wide cancel; nothing in the PR changed.

@vjeux
vjeux marked this pull request as draft October 29, 2026 22:33

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on an engineering owner’s decision: this head rewrites the current component:NumberInput FR4 (a hold that commits once, at the release, on a 250 ms/0.9 ramp that departs from UIStepper’s measured 500 ms cadence) and FR5 (focus by press kind), adds a mouse and pen hold and a finger tracking area 70 px past the steppers, and adds claimed-drag selection rules to spec:AST-094 FR41. Any engineering owner, the author included, can record that decision by commenting /approve-spec <full head SHA> on this pull request, or say which behaviour must change. These changes are needed either way:

  • Check the half actually stepped in handleColumnClick and pressStepper: a mouse click on an aria-disabled stepper now commits a typed draft at the bound (NumberInput.tsx:1037-1051).
  • Make the proposed FR4 name the finger’s 70 px tracking area, since it says moving off the stepper pauses the repeat.
  • Land or split the predecessor pull requests so this diff carries only the stepper hold, and attach real iOS Safari and Android Chrome runs.

[Automated review]

Comment on lines +1047 to +1050
const click = stepperTouch.takeClick(event.nativeEvent);
if (click === 'increment' || click === 'decrement') {
stepValue(click === 'increment' ? 1 : -1);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under a fine pointer every mouse and pen press is captured on the column (NumberInputTouch.tsx:408-411), so its click lands here, and this steps the lift’s half with no check that it can move (stepValue returns early only for a disabled or read-only field). At a bound stepValue still drops the draft and calls onChange when the draft differs from value: with value 5, max 10 and a typed 10, a click on the aria-disabled + commits 10, against current component:NumberInput FR4 (“a stepper that cannot move does nothing”). pressStepper checks the clicked half but steps the lift’s half (:1018-1029). Checking the half actually stepped in both handlers, with a test that clicks an aria-disabled half, would close it.

| FR1 | The input is a native text input with `role="spinbutton"`, `inputMode` `numeric` when integer-only and `decimal` otherwise, `aria-valuemin` and `aria-valuemax` from the range, `aria-valuenow` from the committed `value`, and `aria-valuetext` set to the formatted value when `formatValue` is supplied. | Current tests |
| FR2 | Typing changes only a draft; `onChange` is not called while typing. A draft is valid when it parses for the active locale (grouping, decimal separator, digit script, and sign) and, for validity display, lies within the range and meets integer-only. An invalid draft colors the text secondary, sets `aria-invalid`, and fills a persistent assertive region with the localized invalid-number message. | `spec:AST-086` FR3; `architecture:internationalization`; current tests |
| FR3 | On blur the draft resolves once and ends: an empty draft clears (`onChange(null)` when clearable and a value exists) or reverts; an unparsable or wrong-kind draft reverts; otherwise the number clamps to the range and commits with `onChange(number)` when it differs from `value`. Enter resolves a valid draft the same way and clears an empty draft when clearable. Enter commits nothing for an empty draft without `hasClear`, which stays in the field and is not marked invalid, or for an unparsable or wrong-kind draft, which stays in the field and stays invalid. Enter ends the draft only when its commit clamped. In a field that is not disabled, every Enter outside composition then calls `onEnter`, whatever it committed. | Current tests |
| FR4 | ArrowUp and ArrowDown without modifiers, the wheel over the focused input (when `isWheelEnabled`), and a stepper press step by one grid position and commit at once. A held stepper repeats once after 0.5 s, then on a ramp whose interval starts at 250 ms and shortens to 0.9 of the last, down to 50 ms; the input shows each step, and the press commits once, at its release. Moving off the stepper pauses the repeat and moving back resumes it with no immediate step; sliding onto the other stepper carries it on; a held press selects no text and opens no callout (`spec:AST-094` FR41). The grid starts at `min` (when it suits integer-only) or 0. From an empty field or an emptied draft, up goes to `min` or 0 and down to `max` or 0. Results round to the finer of the step's and the grid start's precision (at most 12 decimal places), clamp to the range, and never leave integer-only; a stepper that cannot move does nothing, while an arrow or wheel step that cannot move still ends the draft, committing a valid draft's number and dropping an unparsable one. A caller `onKeyDown` that prevents an arrow cancels that step. A wheel step stops the page from scrolling; otherwise the page scrolls. | UIStepper's 0.5 s delay and 50 ms floor, measured on the iOS 26.2 simulator; `spec:AST-094` FR41; current tests |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“Moving off the stepper pauses the repeat” holds for a mouse (NumberInputTouch.tsx:364-375), but a finger keeps the press, and keeps stepping, until it is 70 px past the column (NumberInputTouch.tsx:243-248). The record should name both areas, and the 250 ms and 0.9 values need the dated owner decision spec:AST-094 FR2 asks for when a value departs from a measurement.

Comment on lines +42 to +45
* @source GUESS — proposed, pending owner review: UIStepper measured 500 ms between its first repeats (+514, 1017, 1520, 2023, 2525, 3028 ms), iOS 26.2 sim, Oct 23 2026
* @measure M-STP20
*/
firstIntervalMs: 250,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spec:AST-094 FR1 asks every component to use the measured value, and FR2 lets an owner decision depart from it only when it names its date and the measurement it departs from. Until an engineering owner records that decision, firstIntervalMs and acceleration stay guesses against UIStepper’s measured 500 ms phases.

@cixzhang

cixzhang commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 12ed699

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested. An engineering owner approved the amended component:NumberInput FR5 at this head, so the hold itself is settled; these need fixing:

  • The hold's repeat timer keeps stepping the shown value after the field turns disabled or read-only mid-hold, and climbs with no bound (then commits on blur) when the steppers are removed mid-hold; end the hold in both cases and test them.
  • The changeset, the hasNumberSteppers doc and JSDoc, and FR5 say holding a stepper repeats, but a finger hold steps once; say "with a mouse or a pen" in all four places.
  • FR41 of the touch-presentations record applies to a pen's stepper hold, and nothing cancels the context menu during a hold or tests a pen long press; add both.

Non-blocking: FR4 and the record's Behavior row still say the steppers commit at once, the new FR5 verification row lacks its failure and audit cells, and two new comments restate names. Rewording FR5 needs a fresh /approve-spec <full head SHA> from an engineering owner on the new head, and CI has not run on this draft head yet.

[Automated review]

held: null,
intervalMs: STEPPER_HOLD.firstIntervalMs,
};
const repeat = () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This closure never re-reads isDisabled or isReadOnly, and only this button's own pointer events end the hold. A field turned disabled or read-only mid-hold keeps stepping its shown value (7 to 26 in 2 s in a probe), and steppers removed mid-hold leave it climbing with no bound until a blur commits it (7 to 186 in 10 s). End the hold in both cases (a callback ref on the stepper column covers removal and unmount) and add tests for them.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in c64283f: the hold now ends with no commit, and drops its draft, when the field turns disabled or read-only (an effect syncing the repeat timer with editability) and when the stepper column goes away (a callback-ref cleanup that also covers unmount). New cases cover both; each fails on 12ed699.

Comment thread .changeset/number-input-stepper-hold.md Outdated
'@astryxdesign/core': patch
---

[feature] Holding a NumberInput stepper repeats the step: the first repeat comes after 0.5 s, and later ones come faster. The field shows each repeated value as its draft and calls `onChange` once, when the hold ends by release or pointer cancel. A click still steps and commits at once.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A finger hold steps once (NumberInput.tsx:900-907, test "keeps the single step of a finger press"), so "Holding a NumberInput stepper repeats the step" promises more than ships. Say "with a mouse or a pen" here, in the hasNumberSteppers doc and JSDoc, and in FR5.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in c64283f for the changeset, the hasNumberSteppers doc and the JSDoc ("with a mouse or a pen"). FR5 keeps the owner's approved wording for now; the qualifier there is with the record owner.

| AR5 | `NumberInput.a11y.chromium.spec.ts` with its recorded known failure | Read-only | The known failure stops matching when the gap closes. | `audit:NumberInput/accessibility` |
| FR11 | `NumberInput.test.tsx` "does not step or commit on a composing keydown (IME)" and "waits for IME composition to finish before committing" | Composing arrows and Enter | A step or commit during composition fails. | `audit:NumberInput/behavior` |
| FR5, FR13, AR4 | `NumberInput.coarse-pointer.a11y.chromium.spec.ts` | Coarse/non-coarse pointers at 390 and 1280 px, LTR/RTL, non-default themed end padding | Visible coarse-pointer steppers, hidden fine-pointer steppers, wrong end padding, or lost direct/keyboard entry fail. | `audit:NumberInput/behavior` |
| FR5 | `NumberInput.test.tsx` "a held stepper" suite | Hold and release; pointer cancel; a bound; clicks; a finger press |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: this row has three of the table's five cells. Add the failure expectation (for example, a hold that commits per repeat, keeps repeating after release, or steps after the field turns disabled fails) and audit:NumberInput/behavior.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in c64283f: the row now has its failure expectation and audit:NumberInput/behavior.

@cixzhang

cixzhang commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

/approve-spec ef404f5

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested. An engineering owner approved the amended component:NumberInput FR5 at this head, so the hold itself is settled; one thing still needs fixing:

  • The hold runs for a pen, but startStepperHold takes no text-selection lock, cancels no context menu, and no test holds a pen, so spec:AST-094 FR41 (a pen's hold on a stepper starts no selection or callout) is unproven; take the shared lockSelection from utils/selectionLock.ts for pen presses and add the pen long press that FR41's verification row lists.

Also worth a look (non-blocking):

  • Miss: startStepperHold replaces a running hold without ending it, so a second pointer pressing a stepper mid-hold leaves the first timer stepping until the next blur commits its value; call cancelStepperHold() before arming.
  • FR4 and the record's Behavior row still say the steppers "commit at once"; pointing them at FR5's hold keeps the record consistent.

CI has not run its test, lint or build jobs on this draft head.

[Automated review]

// mouse or pen press. A finger keeps the single step of its click.
if (
event.button !== 0 ||
event.pointerType === 'touch' ||

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A pen press passes this gate and starts the hold, but nothing takes a selection lock or cancels contextmenu for it, and no test holds a pen. spec:AST-094 FR41 asks that a pen's hold on a stepper start no text selection or callout. Take lockSelection (gated by locksSelectionFor(event.pointerType)) from utils/selectionLock.ts here, release it when the hold ends, and add a pen long-press test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in a8878aa: a pen's hold takes lockSelection (gated by locksSelectionFor) and releases it when the hold ends or is cancelled; a live hold also refuses contextmenu. A new pen long-press case checks both, during and after the hold; it fails on the previous head.

);
};
hold.timer = setTimeout(repeat, STEPPER_HOLD.delayMs);
stepperHoldRef.current = hold;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking (Miss): this replaces a hold that may still be running. With a mouse holding + and a pen pressing −, the + timer keeps stepping after both releases (7 to 88 in 5 s in a probe) and the next blur commits it. Call cancelStepperHold() before arming the new hold.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in a8878aa: any new press cancels a running hold first, only the holding pointer can end a hold, and a cancelled hold's own click steps nothing. New cases cover a mouse hold replaced by a pen (either release order) and by a finger tap.


// A held stepper repeats into the draft and commits once, when the hold ends.
const stepperHoldRef = useRef<StepperHold | null>(null);
// The click that follows a held press's release steps nothing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking (INV29): this restates the ref's name and the comment in the click handler. Drop it; the click handler's comment already gives the reason.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in a8878aa: dropped.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested. An engineering owner approved the amended component:NumberInput FR5, so the hold itself is settled; one thing needs fixing:

  • The steppers' onContextMenu (NumberInput.tsx, both steppers) refuses the context menu during any hold, a mouse's included, so on macOS a Control-click on a stepper loses its menu and, if held, steps and commits the value: this desktop behavior change needs an owner decision and contradicts spec:AST-094 FR41's mouse clause, so refuse contextmenu only during a pen's hold, end a mouse hold instead, and add a test for a Control-press.

Not blocking: FR4 and the record's Behavior row still say the steppers "commit at once"; pointing them at FR5's hold keeps the record consistent.

[Automated review]

Comment on lines +1267 to +1272
onContextMenu={event => {
// A held press opens no callout menu.
if (stepperHoldRef.current != null) {
event.preventDefault();
}
}}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This refuses contextmenu for every running hold, a mouse's too. On macOS a Control-click is a primary-button press, so it starts a hold and then loses the context menu; held, it steps and commits (2 to 4 in Chromium). spec:AST-094 FR41 keeps a mouse's context request as released. Refuse it only for a hold that took the selection lock (a pen), and for a mouse hold end the hold with no commit (cancelStepperHold()) and let the menu open. The decrement stepper's handler needs the same change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 6f560a5 for both steppers: only a hold that took the selection lock (a pen) refuses contextmenu; a mouse hold, such as a macOS Control-click, ends with no commit and the menu opens. A new case checks the mouse path; it fails on the previous head.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested. An engineering owner approved the amended component:NumberInput FR5 for this record text, so the hold itself is settled, and the context-menu fix from the last round works; one thing needs fixing:

  • Miss: while a hold runs, the field's validity and the steppers' disabled state still follow the typed draft and the committed value instead of the held value, so after an unparsable or out-of-range typed draft a valid held number shows as invalid (aria-invalid, the invalid colour and "Invalid number"), and a hold down from the max shows 8 while + stays aria-disabled (or shows + enabled at the bound); derive isInputValid and canIncrement/canDecrement from holdDraft while it is set, and add a test for each.

Not blocking: FR4 and the record's Behavior row still say the steppers "commit at once"; pointing them at FR5's hold keeps the record consistent.

[Automated review]

}, [formattedValue, holdDraft, isFocused, locale, pendingInput, value]);

// Check if current pending input is valid (for styling purposes)
const isInputValid = useMemo(() => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While a hold runs the field shows holdDraft, but this still reads pendingInput, so after an unparsable or out-of-range typed draft a valid held value keeps aria-invalid="true", the invalid colour and "Invalid number" until the release. canIncrement/canDecrement (line 1133) have the same gap: a hold down from the max shows 8 while + stays aria-disabled. Suggest if (holdDraft !== null) return true; here and holdDraft ?? valueForStepping for the two can* checks, each with a test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in c2528eb: while a hold shows a value, the field reads valid, and both steppers' enabled state follows that value. Two new cases (an invalid typed draft, then a hold; a hold down from the max) each fail on the previous head.

Holding a stepper with a mouse or pen repeats the step, the first repeat
after 0.5 s and then on a ramp from 250 ms down to 50 ms. Each repeat
writes the field's draft, and the hold commits once through onChange,
on release or pointer cancel. The click that follows a release steps
nothing, so a hold makes one commit. A press released before 0.5 s is
a click: it steps and commits at once. A finger keeps its click's single
step. FR5 of the NumberInput record states the hold, with a
verification row.

@astracat-bot astracat-bot Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved pending CI: CI has not run on this head yet. Next step: a maintainer adds the ci:run label so CI runs; until then the required checks report only that they wait for it.

The held stepper matches the amended component:NumberInput FR5, which an engineering owner approved for this record text, and the last round's fix works: while a hold shows a value, the field reads valid and each stepper's disabled state follows that value, with tests that fail on the previous version.

Non-blocking:

  • FR4 and the record's Behavior row still say the steppers "commit at once"; pointing them at FR5's hold keeps the record consistent with itself.
  • The StepperHold doc comment restates its fields; keeping only what the names cannot say (for example "A frozen hold commits nothing.") follows the repository's comment rule.

[Automated review]

Comment on lines +481 to +485
/**
* A held stepper press: its pointer, its next repeat, the value its repeats
* reached, whether the field stopped taking them, and the release of its
* selection lock.
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: this comment restates five of the type's six fields. Keeping only what a name cannot say reads better, for example:

/** A frozen hold commits nothing. */

| FR1 | The input is a native text input with `role="spinbutton"`, `inputMode` `numeric` when integer-only and `decimal` otherwise, `aria-valuemin` and `aria-valuemax` from the range, `aria-valuenow` from the committed `value`, and `aria-valuetext` set to the formatted value when `formatValue` is supplied. | Current tests |
| FR2 | Typing changes only a draft; `onChange` is not called while typing. A draft is valid when it parses for the active locale (grouping, decimal separator, digit script, and sign) and, for validity display, lies within the range and meets integer-only. An invalid draft colors the text secondary, sets `aria-invalid`, and fills a persistent assertive region with the localized invalid-number message. | `spec:AST-086` FR3; `architecture:internationalization`; current tests |
| FR3 | On blur the draft resolves once and ends: an empty draft clears (`onChange(null)` when clearable and a value exists) or reverts; an unparsable or wrong-kind draft reverts; otherwise the number clamps to the range and commits with `onChange(number)` when it differs from `value`. Enter resolves a valid draft the same way and clears an empty draft when clearable. Enter commits nothing for an empty draft without `hasClear`, which stays in the field and is not marked invalid, or for an unparsable or wrong-kind draft, which stays in the field and stays invalid. Enter ends the draft only when its commit clamped. In a field that is not disabled, every Enter outside composition then calls `onEnter`, whatever it committed. | Current tests |
| FR4 | ArrowUp and ArrowDown without modifiers, the wheel over the focused input (when `isWheelEnabled`), and the steppers step by one grid position and commit at once. The grid starts at `min` (when it suits integer-only) or 0. From an empty field or an emptied draft, up goes to `min` or 0 and down to `max` or 0. Results round to the finer of the step's and the grid start's precision (at most 12 decimal places), clamp to the range, and never leave integer-only; a stepper that cannot move does nothing, while an arrow or wheel step that cannot move still ends the draft, committing a valid draft's number and dropping an unparsable one. A caller `onKeyDown` that prevents an arrow cancels that step. A wheel step stops the page from scrolling; otherwise the page scrolls. | Current tests |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: FR4 (and the Behavior row at line 47) still says the steppers "commit at once", while FR5 now makes a hold commit once, at its end. A pointer such as "the steppers step by one grid position and commit at once (a held stepper follows FR5)" keeps the record consistent with itself.

This branch was successfully deployed

1 active deployment
Preview — c2528eb4 Deployed Nov 4, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants