[EuiFieldNumber] Added any as a step option#3562
Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
@cchaos @chandlerprall Could you review this PR? |
|
Can you please update your PR description to match the changes you've made. I know this one is pretty simple, but at least removing the boilerplate summary would help. Thanks for helping out! |
|
@snide Yeah, I actually made another PR which I later closed and forgot to write the description for this one. |
any as a step option
|
Jenkins, test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3562/ |
Okay, I'll directly apply your suggestions Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
|
@cchaos any other improvements? |
|
jenkins test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3562/ |
|
Hmmm, the prop is getting passed, but it seems to not actually be working. I can't type any decimals when
|
This is only an issue in the docs. The eui/src-docs/src/views/form_controls/field_number.js Lines 9 to 12 in 1bc2b29 We should update the docs, though |
|
@cchaos Should I update some documentation? |
|
@shrey Yes, let's go ahead and remove the sanitization |
|
@cchaos So should we get rid of the onChange function all together? |
|
@cchaos @thompsongl I made the changes in the docs, could you review them? |
| value={value} | ||
| onChange={e => onChange(e)} | ||
| aria-label="Use aria labels when no actual label is in use" | ||
| step={3} |
There was a problem hiding this comment.
Let's not add a step like this as it could confuse consumers that this is the default.
There was a problem hiding this comment.
Oh I forgot to remove that, I was just testing it
|
retest |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3562/ |
|
@cchaos Could you review it now? |
|
retest |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3562/ |
|
@cchaos The tests passed :) any other changes? |
Previous commit didn't work
cchaos
left a comment
There was a problem hiding this comment.
Awesome. Checked the docs example and adding step="any" works.
|
retest |
Summary
Added input type "any" as an attribute of step of
EuiFieldNumber.Fixes #3550
Checklist
[ ] Added documentation examples[ ] Added or updated jest tests[ ] Checked for breaking changes and labeled appropriately[ ] Checked for accessibility including keyboard-only and screenreader modes@cchaos @chandlerprall
I've made the necessary changes, @cchaos learnt something new, thanks.