Conversation
akademy
left a comment
akademy
left a comment
There was a problem hiding this comment.
This looks great. Just a few minor suggestions.
| }) => { | ||
| const numberRegex = Modes[numberMode]; | ||
|
|
||
| const helperText = `A ${numberMode} number. Limits: ${minValue} to ${maxValue}`; |
There was a problem hiding this comment.
Can you make it optional to display this helper text?
There was a problem hiding this comment.
Helper text is default, but can now be turned off
| value={numberText} | ||
| onChange={(e) => handleInputChange(e.target.value)} | ||
| onKeyDown={handleKeyDown} | ||
| onBlur={handleBlur} |
There was a problem hiding this comment.
If you do not set either commitOnBlur or commitOnReturn to false and hit return you get in a loop. Enter -> Blur
There was a problem hiding this comment.
This loop seems to have only been an issue due to alert stealing focus from the original element. I've now added logic to blur the element if commitOnBlur is true (thus using the onBlur action to trigger the handleCommit when the enter key is pressed), otherwise handleCommit for onKeyDown happens as normal.
I tried it out a few times without managing to trigger a loop, but let me know if this doesn't work.
| ? "" | ||
| : `A ${numberMode} number. Limits: ${minValue} to ${maxValue}`; | ||
|
|
||
| const handleInputChange = (value: string) => { |
There was a problem hiding this comment.
isValid and isInLimits are both derived from props and numberText so we can remove those two states and move their calculation out of handleInputChange. handleInputChange then only needs setNumberText(value)
| onKeyDown={handleKeyDown} | ||
| onBlur={handleBlur} | ||
| error={!isValid || !isInLimits} | ||
| helperText={ |
There was a problem hiding this comment.
Can calculate helperText outside of return
3 participants
Fixes #154.
Adds a number input component, which validates various number types, high limits, and low limits.