-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: Add NumericControl
#27
Conversation
src/antd/InputNumber.tsx
Outdated
const value = props.data === undefined || isDataEmptyObj ? defaultValue : props.data as number | ||
|
||
const numberType = schema.type | ||
const isInteger = (typeof numberType === "string" && numberType === "integer") || (Array.isArray(numberType) && numberType.includes("integer")) |
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.
is there a test case for the Array.isArray(numberType) bit?
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.
I just added a missing test that asserts that numbers get rounded on input for integer. I tested schemas with type: "integer"
and type: ["integer"]
. I decided that type arrays with more types than "integer"
will cause this variable to get set to false
.
src/antd/InputNumber.tsx
Outdated
} | ||
} | ||
|
||
const addonAfter = props.uischema.options?.addonAfter as string | undefined |
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.
is this cast still necessary?
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.
Yes, it's considered an any
until I do this cast. It's essentially the same as what we decided to do here.
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.
I added a new type InputNumberOptions
to ui-schema.ts
to precisely follow the pattern established by AlertControl
. There is still a cast here though, the cast is just on options
now and it passes through the props types.
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.
ah, it's a lot easier to read if you pull off an object and type it:
const options: Maybe<InputNumberOptions> = props.uischema.options as InputNumberOptions
const addonAfter = options?.addonAfter
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.
I'm sorry, how is this different than what I did?
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.
typos like props.uischema.options?.adonafter as string | undefined
doesn't get caught but it would in my example
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.
Ok I don't think you've seen the updated version. This is now outdated.
…ations/jsonforms-antd-renderers into f/FE-59/add-numeric-control
InputNumber
rendererNumericControl
NumericControl
testsNumericControl
stories