fix(dashboard): handle empty input for min and max values (#3410, #3411)#334
fix(dashboard): handle empty input for min and max values (#3410, #3411)#334YUDONGLING wants to merge 1 commit intocloudreve:masterfrom
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.
Close cloudreve/cloudreve#3410 and cloudreve/cloudreve#3411.