Skip to content

Add buffered NumberInput for decimal-friendly width editing#1919

Merged
kmcginnes merged 5 commits into
mainfrom
width-input-decimal-editing
Jul 10, 2026
Merged

Add buffered NumberInput for decimal-friendly width editing#1919
kmcginnes merged 5 commits into
mainfrom
width-input-decimal-editing

Conversation

@kmcginnes

@kmcginnes kmcginnes commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Description

Editing width values in the style dialogs was awkward: the fields bound a controlled input directly to a parsed number, so every keystroke round-tripped through Number()String(), collapsing intermediate editing states ("1." became "1", destroying the decimal point mid-edit). The steppers also moved by whole units, which is a big jump in Cytoscape.

This adds a NumberInput component using a focused-draft pattern: while the user is editing, a raw draft string owns the display (so "1." and "" survive); on blur the draft is discarded and the display derives directly from the value prop. Because there is no persistent local buffer, external changes — Reset to Default, loading a styles file, another tab — always show without any sync logic. All five numeric fields across the node and edge style dialogs now use it, and the three width/thickness fields get step={0.5} (opacity keeps step={0.1}).

How to read

  1. NumberInput.tsx — the new component; one draft state and a ?? is the whole mechanism
  2. NumberInput.test.tsx — regression tests; they fire raw change-event buffers directly since happy-dom can't drive native number-input keyboard editing
  3. NodeStyleDialog.tsx and EdgeStyleDialog.tsx — mechanical adoption at the five call sites

Validation

  • Regression tests cover decimal preservation, single-press clearing, external resets (including through useDeferredValue, which the style hooks use), edits that don't change the parsed value, and blur canonicalisation.
  • Manually verified in the running app: deleting a digit from 1.4 no longer drops the decimal point, clearing takes one delete, steppers move by 0.5, and Reset to Default restores the fields.
  • pnpm checks and pnpm test (2078 tests) pass.

Related Issues

Check List

  • I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • I have verified pnpm checks passes with no errors.
  • I have verified pnpm test passes with no failures.
  • I have covered new added functionality with unit tests if necessary.
  • I have updated documentation if necessary.

kmcginnes added 5 commits July 9, 2026 19:31
Width/thickness fields in the style dialogs bound a controlled input
directly to a parsed number, which destroyed intermediate editing states
("1." collapsed to "1" because Number("1.") === 1). Replace all five
numeric fields (border width, line thickness, opacity) with a new
NumberInput component that maintains a local string buffer during
editing and canonicalises on blur. Also set step=0.5 on width/thickness
fields so steppers move in useful increments.
Exclude onBlur from NumberInputProps so callers can't silently replace
the buffer reformat, use the typed getByLabelText generic instead of a
cast, and drop the attribute-passthrough test that asserted wiring
rather than behavior.
The previous parseNumberSafely(buffer) !== value guard was intended to
distinguish own-edit echoes from external resets, but it could match
incorrectly when useDeferredValue batched the transition. Replace with
an explicit editing flag (React state, not a ref — the compiler can't
track refs during render) that is set true in onChange and cleared after
resync. When editing is true, the buffer is preserved (keeps "1." and
"" intact during typing); when false, any value prop change forces a
buffer rewrite (handles Reset to Default).
The editing-state flag got stuck when an edit did not change the parsed
value (e.g. "1.40" → still 1.4), causing subsequent external resets to
be ignored. Replace with reportedValue — the last parsed value sent
upward. On resync, compare value against reportedValue: match means our
own echo (keep buffer); mismatch means external change (force buffer).

Add regression tests for the stuck-flag case, external non-default
updates, and rapid sequential edits.
Both prior sync mechanisms (value-echo guard, then reportedValue
comparison) tracked prop history to decide when to overwrite the local
buffer, and both had states where an external reset was misread as an
edit echo, leaving the field stale after Reset to Default.

Remove the machinery entirely: a draft string exists only while the
user is actively editing (set on change, cleared on blur). Whenever no
draft is active the display derives straight from the value prop, so
any external change — reset, style load, another tab — shows
unconditionally. Clicking Reset blurs the input, which ends the draft.
@kmcginnes kmcginnes marked this pull request as ready for review July 10, 2026 16:51
@kmcginnes kmcginnes merged commit cced211 into main Jul 10, 2026
6 checks passed
@kmcginnes kmcginnes deleted the width-input-decimal-editing branch July 10, 2026 16:51
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.

Style dialog width inputs: fractional stepping and decimal-friendly editing

1 participant