BL-15962 Fix empty input and backspace handling in SmallNumberPicker

[nabalone]

---

---

This change is 

[devin-ai-integration]

Devin Review found 3 potential issues.

View 2 additional findings in Devin Review.

[devin-ai-integration]

🚩 Removal of custom &:disabled CSS changes the Add Page button's disabled appearance

The PR removes the &:disabled { color: ${kBloomBuff}; border: medium solid ${kBloomBuff}; } CSS rules from the BloomButton in selectedTemplatePageControls.tsx. Previously, when the button was disabled, it had a specific gray (#d2d2d2) color and border. Now it falls back to MUI Button's default disabled styling (typically reduced opacity). This is a visual change. If the original custom styling was intentional to match the dialog's design language, this regression may be undesirable. Worth confirming this was deliberate rather than an accidental deletion while editing nearby lines.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[nabalone]

I'm not sure why those custom styles were originally there. They only make things look funny -- the text on the button invisible and the size change so that things jump around a little bit. I'm guessing they were leftovers from an age before we were using the mui disabled styling? The default MUI disabled styling is much better

[devin-ai-integration]

🟡 isWithinLimits accepts 0 when minLimit is undefined, despite component being for positive integers only

When minLimit is not provided (it's optional), isWithinLimits at line 29 only checks value < props.minLimit when props.minLimit !== undefined. This means 0 passes validation and handleChange(0) is called with onValidityChange(true). The old code rejected 0 via !newNum (since 0 is falsy in JS), but the new code has no such implicit guard. The component's JSDoc at line 16 states "positive integers only" and the initialValue defaults to 1 (line 22), but the validation doesn't enforce a minimum of 1 when minLimit is omitted. No current caller omits minLimit, so the impact is latent, but the minLimit prop is declared optional in INumberChooserProps (line 9), so a future caller could hit this.

Suggested change

	const isWithinLimits = (value: number): boolean => {
	if (value > props.maxLimit) {
	return false;
	}
	if (props.minLimit !== undefined && value < props.minLimit) {
	return false;
	}
	return true;
	};
	const isWithinLimits = (value: number): boolean => {
	if (value > props.maxLimit) {
	return false;
	}
	const effectiveMin = props.minLimit !== undefined ? props.minLimit : 1;
	if (value < effectiveMin) {
	return false;
	}
	return true;
	};

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 Behavioral contract change: handleChange no longer called on invalid input

The old SmallNumberPicker always called props.handleChange(initialValue) when input was invalid, ensuring the parent's state was reset to a known-good value. The new code does NOT call handleChange during invalid states — it only calls onValidityChange(false). This means the parent retains the last valid value in its state during invalid input. Both production callers (selectedTemplatePageControls.tsx:296-297 and numberChooserDialog.tsx:62-63) now pass onValidityChange and disable their submit buttons accordingly, so stale values can't be submitted. However, onValidityChange is optional in the interface (smallNumberPicker.tsx:11), so a future caller that omits it would have no way to detect invalid input — the parent would silently hold a stale value. The Storybook story at react_components/stories/misc.stories.tsx:77-82 already demonstrates this: it doesn't pass onValidityChange.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

📝 Info: The newString === "e" guard is dead code for type="number" inputs

On line 32, the check newString === "e" will never be true. For type="number" inputs in Chromium-based browsers (WebView2 112), when the user types just "e", event.target.value is always "" (empty string), not "e". The input is actually handled correctly by the subsequent if (newString === "") check on line 37-39, which returns early. So there's no functional bug, but this guard is misleading about what it protects against.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Devin Review found 1 new potential issue.

View 5 additional findings in Devin Review.

[devin-ai-integration]

🔴 Keystroke-level validity.valid check blocks typing valid multi-digit numbers when minLimit > 1

The handleNumberChange handler rejects input whenever event.target.validity.valid is false (line 28). The HTML constraint validation API evaluates rangeUnderflow against min on every keystroke, not just on the final value. So when minLimit=2 (as in DuplicateManyDialog at src/BloomBrowserUI/bookEdit/duplicateManyDialog.tsx:23), typing "1" as the first digit produces validity.rangeUnderflow=true → validity.valid=false, causing the handler to return early and React to snap the input back. This makes it impossible for the user to type any number whose first digit is less than minLimit: with min=2, max=999, users cannot type 10–19 or 100–199. The storybook example (dialogs.stories.tsx:101) also uses min: 2, max: 777 and is similarly affected.

Suggested approach

Replace the validity.valid check with a more targeted check that only rejects truly bad input (e.g. validity.badInput for non-numeric characters), and defer the min/max range validation to the handleBlur handler. This way intermediate typing states like "1" (on the way to "15") are allowed, and out-of-range values are corrected when the user finishes editing.

Prompt for agents

In src/BloomBrowserUI/react_components/smallNumberPicker.tsx, the handleNumberChange function (lines 22-42) uses event.target.validity.valid to reject input on every keystroke. This causes rangeUnderflow/rangeOverflow to fire on intermediate typing states (e.g. typing '1' when min=2, on the way to typing '15'). Fix this by:

1. In handleNumberChange (around line 27-32), replace the !event.target.validity.valid check with event.target.validity.badInput, which only fires for truly non-numeric input. Keep the 'e' check. Remove the reliance on min/max validity at the keystroke level.

2. In handleBlur (around line 46-50), add range validation: if the input value is non-empty, parse it and check if it's within [minLimit, maxLimit]. If it's out of range or NaN, restore lastValidValue and do NOT call props.handleChange. This ensures the final committed value is always in range.

3. Also in handleNumberChange (around line 40-41), before calling setLastValidValue and props.handleChange, add a range check: only update if newNum >= (props.minLimit ?? 1) && newNum <= props.maxLimit. This prevents out-of-range values from being propagated to the parent while the user is still typing, but doesn't block the keystrokes from appearing in the display.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[andrew-polk]

@andrew-polk reviewed 6 files and all commit messages, resolved 4 discussions, and dismissed @devin-ai-integration[bot] from 5 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion.