Adds missing props to InputBase types #228
Conversation
illusionalsagacity
left a comment
illusionalsagacity
left a comment
There was a problem hiding this comment.
One thing I noticed, the TextField didn't automatically get these like I thought it would--it extends FormControl.publicProps instead. Should I also add these to TextField.props explicitly or is there a different approach you all would prefer?
|
Yes, please spread them into the TextField props if possible. There might be an overlap though. |
@fhammerschmidt I've pushed a WIP commit that shows one way we could accomplish this, by creating more 'partial' record types and composing them more directly where they're being used |
| | @as("soft") Soft | ||
| | @as("off") Off | ||
|
|
||
| type inputBaseComponentPublicProps = { |
There was a problem hiding this comment.
IMO, this should probably be in CommonProps for inputs / textarea props
illusionalsagacity
force-pushed
the
input-base-props
branch
from
fbfa51b to
8996322
Compare
|
Apart from the conflicts and the commented-out props that got overridden (I think you can just remove them), is this ready? |
illusionalsagacity
force-pushed
the
input-base-props
branch
2 times, most recently
from
57c7ee8 to
e11b18c
Compare
| /** | ||
| * The maximum value the user can enter in the input. See [MDN](https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/max) for more details. | ||
| */ | ||
| max?: string, |
There was a problem hiding this comment.
The min and max props technically are not just int, maximum/minimum values can be other values too: https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/max
Additionally the step is probably better off as a float: https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/step#syntax
Since this is a breaking change, should I leave it out? Otherwise I can make it an unboxed type that is string | float
e: due to the timezone difference I've elected to remove the breaking change so you can merge if you wish. I can follow up in another PR if that's a desired change
There was a problem hiding this comment.
Yeah it mainly depends on the type and thus is probably a good fit for unboxed.
Let's merge this first.
illusionalsagacity
force-pushed
the
input-base-props
branch
from
e11b18c to
d600f6d
Compare
illusionalsagacity
force-pushed
the
input-base-props
branch
2 times, most recently
from
400a5b8 to
a2e137d
Compare
`InputBase.inputBaseComponentProps` was moved to `CommonProps.inputTextareaProps` and added: 1. `minLength` 2. `maxLength` 3. `wrap` `InputBase.publicProps` added: 1. `maxLength` 2. `minLength` 3. `wrap` `TextField.props` now includes the input / text area specific props `CommonProps.globalAttributes` now holds the HTML attributes that are valid for all elements. `CommonProps.clickableProps` was renamed to `CommonProps.eventHandlerProps` and added the following event handlers: 1. onChange 1. onBeforeInput 1. onInput 1. onReset 1. onSubmit 1. onInvalid resolves cca-io#224
illusionalsagacity
force-pushed
the
input-base-props
branch
from
aa917bd to
8128cd0
Compare
2 participants
InputBase.inputBaseComponentPropswas moved toCommonProps.inputTextareaPropsand added:minLengthmaxLengthwrapInputBase.publicPropsadded:maxLengthminLengthwrapTextField.propsnow includes the input / text area specific propsCommonProps.globalAttributesnow holds the HTML attributes that are valid for all elements.CommonProps.clickablePropswas renamed toCommonProps.eventHandlerPropsand added the following event handlers:resolves #224