fix(dashboard): handle empty input for min and max values (#3410, #3411)#334
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the dashboard Custom Props editor to properly treat empty min/max numeric inputs as “unset” instead of attempting to parse them, addressing issues #3410 and #3411.
Changes:
- Update
minnumeric input handler to setmintoundefinedwhen the input is emptied. - Update
maxnumeric input handler to setmaxtoundefinedwhen the input is emptied.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type="number" | ||
| value={editProps?.min || ""} | ||
| onChange={(e) => setEditProps({ ...editProps, min: parseInt(e.target.value) } as CustomProps)} | ||
| onChange={(e) => setEditProps({ ...editProps, min: e.target.value === "" ? undefined : parseInt(e.target.value) } as CustomProps)} | ||
| /> |
There was a problem hiding this comment.
value={editProps?.min || ""} will render 0 (and also NaN) as an empty string because of the || fallback. That makes the UI inconsistent with the underlying state (e.g., min=0 looks cleared) and can mask invalid values. Use a nullish check (e.g., ??) and/or explicitly handle NaN so only undefined/null becomes empty.
| value={editProps?.min || ""} | ||
| onChange={(e) => setEditProps({ ...editProps, min: parseInt(e.target.value) } as CustomProps)} | ||
| onChange={(e) => setEditProps({ ...editProps, min: e.target.value === "" ? undefined : parseInt(e.target.value) } as CustomProps)} | ||
| /> |
There was a problem hiding this comment.
The new handler maps empty string to undefined, but parseInt(...) can still yield NaN for certain transient/invalid inputs (and it also truncates decimals / ignores exponent notation). Saving NaN here can later serialize to null and break min/max constraints. Consider using e.target.valueAsNumber (or Number(...)) and treating NaN as undefined; if you keep parseInt, pass an explicit radix.
| type="number" | ||
| required={fieldType.maxRequired} | ||
| value={editProps?.max || ""} | ||
| onChange={(e) => setEditProps({ ...editProps, max: parseInt(e.target.value) } as CustomProps)} | ||
| onChange={(e) => setEditProps({ ...editProps, max: e.target.value === "" ? undefined : parseInt(e.target.value) } as CustomProps)} | ||
| /> |
There was a problem hiding this comment.
value={editProps?.max || ""} has the same issue as min: it will display 0 (and NaN) as empty due to ||, which can confuse users and hide invalid state. Prefer a nullish fallback (??) and/or sanitize NaN before storing/displaying.
| required={fieldType.maxRequired} | ||
| value={editProps?.max || ""} | ||
| onChange={(e) => setEditProps({ ...editProps, max: parseInt(e.target.value) } as CustomProps)} | ||
| onChange={(e) => setEditProps({ ...editProps, max: e.target.value === "" ? undefined : parseInt(e.target.value) } as CustomProps)} | ||
| /> |
There was a problem hiding this comment.
Same parsing concern for max: parseInt(...) can produce NaN (or silently truncate) and that value will be persisted in editProps. Treat NaN as undefined (and consider valueAsNumber) to ensure you never save an invalid numeric constraint.
|
Thanks! |
Close cloudreve/cloudreve#3410 and cloudreve/cloudreve#3411.