-
Notifications
You must be signed in to change notification settings - Fork 392
Add Number Input (validation) #849
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
bbfcee4 to
c98d203
Compare
|
@jvalkeal this looks like a good addition - any reason why it has not been merged ? Or is there a way we can add custom input handlers |
huyuanxin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks great, could you please merge this pr? @jvalkeal
|
This is a nice addition indeed! I will plan it for the next major release 4.0. Can you please update and rebase the PR on the latest Thank you upfront. |
24f267a to
fc1febf
Compare
cf89d78 to
ce584d3
Compare
Updated, but I couldn't rengenerate properly antora nor regenerate some asciinema (no WSL/linux at disposal), so did the best I can rn |
|
No problem, I will take care of that. Thank you for the updates! I will check in details once we start working on v4. |
|
Hi @Nico-DF I think we can proceed and merge this PR now. Can you please rebase it on the latest Please exclude the changes in samples, they are outdated. Thank you upfront. |
Signed-off-by: Nicola Di Falco <[email protected]>
Signed-off-by: Nicola Di Falco <[email protected]>
ce584d3 to
50691a8
Compare
|
Aaand also done. I'll check if there is any issue more likely tomorrow |
|
Thank you for the updates! Rebased and merged. Thank you for your contribution 👍 |
Adding a new type of input: NumberInput
It defaults to Integer and is basically a StringInput with a validation (similar to ConfirmInput)