Skip to content

Adds missing props to InputBase types #228

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

Merged
merged 1 commit into from
Feb 27, 2025

Conversation

illusionalsagacity
Copy link
Contributor

@illusionalsagacity illusionalsagacity commented Feb 21, 2025

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
  2. onBeforeInput
  3. onInput
  4. onReset
  5. onSubmit
  6. onInvalid

resolves #224

Copy link
Contributor Author

@illusionalsagacity illusionalsagacity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@fhammerschmidt
Copy link
Member

Yes, please spread them into the TextField props if possible. There might be an overlap though.

@illusionalsagacity
Copy link
Contributor Author

illusionalsagacity commented Feb 25, 2025

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 = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, this should probably be in CommonProps for inputs / textarea props

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please move them.

@fhammerschmidt
Copy link
Member

Apart from the conflicts and the commented-out props that got overridden (I think you can just remove them), is this ready?

@illusionalsagacity illusionalsagacity force-pushed the input-base-props branch 2 times, most recently from 57c7ee8 to e11b18c Compare February 27, 2025 03:46
/**
* 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,
Copy link
Contributor Author

@illusionalsagacity illusionalsagacity Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fhammerschmidt

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it mainly depends on the type and thus is probably a good fit for unboxed.
Let's merge this first.

@illusionalsagacity illusionalsagacity marked this pull request as ready for review February 27, 2025 03:55
@illusionalsagacity illusionalsagacity force-pushed the input-base-props branch 2 times, most recently from 400a5b8 to a2e137d Compare February 27, 2025 03:59
`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
@fhammerschmidt fhammerschmidt merged commit 704ca97 into cca-io:master Feb 27, 2025
1 check passed
@illusionalsagacity illusionalsagacity deleted the input-base-props branch February 27, 2025 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

InputBase props missing maxLength, minLength, wrap props for text areas
2 participants