primefaces / primereact

The Most Complete React UI Component Library
https://primereact.org
MIT License
6.87k stars 1.04k forks source link

InputText/InputNumber missing documentation / types #1364

Closed MagicLegend closed 2 years ago

MagicLegend commented 4 years ago

I'm submitting a ... (check one with "x")

[X] bug report
[X] feature request
[ ] support request => Please do not submit support request here, instead see https://forum.primefaces.org/viewforum.php?f=57

Codesandbox Case (Bug Reports) Please fork the codesandbox below and create a case demonstrating your bug report. Issues without a codesandbox have much less possibility to be reviewed.

https://codesandbox.io/s/primereact-test-bwqj7?file=/src/index.js

Current behavior

There seems to be a lot of undocumented features in the InputText component. Given how much options the InputNumber passes to the internally used InputText; there appears to be a few undocumented features. (e.g. the ability to use Regex on InputText)

https://github.com/primefaces/primereact/blob/09b558cf5d1f293124249b25770c67a2b0cc672b/src/components/inputnumber/InputNumber.js#L701-L707

Expected behavior

Complete and accurate documentation and types.

Minimal reproduction of the problem with instructions

https://codesandbox.io/s/primereact-test-bwqj7?file=/src/index.js

Note that the regex check actually works on the InputText component, even though this is undocumented in both the typings and [https://primefaces.org/primereact/showcase/#/inputtext](in the official documentation).

Please tell us about your environment:

KDE Neon, VSCode, NPM

VsevolodGolovanov commented 4 years ago

The showcase also uses onChange instead of onInput. It's not clear what the difference is.

MagicLegend commented 4 years ago

The showcase also uses onChange instead of onInput. It's not clear what the difference is.

A quick Google could have helped you with that question: In React, what's the difference between onChange and onInput?

VsevolodGolovanov commented 4 years ago

I've seen this in InputText's code: image and concluded that PrimeReact has its own opinions about those.

MagicLegend commented 4 years ago

Please include the code like follows; makes it easier to trace:

https://github.com/primefaces/primereact/blob/45cf01cbefbff1d7e3496ed23521378d58430fb7/src/components/inputtext/InputText.js#L45-L61

I've made a test for you; using both hooks in various forms:

https://codesandbox.io/s/primereact-test-sm83i?file=/src/index.js

From this I can say that the element behaves identically when either onInput or onChange is defined; just like mentioned in the SO answer above. When both are specified, it seems that onChange takes preference over onInput. Note that, in line with the code mentioned above, if onChange is not specified, the p-filled class gets added to the element.

melloware commented 2 years ago

@MagicLegend @VsevolodGolovanov are you talking about keyfilter using a regex that was undocumented? I am trying to figure out if this ticket is still valid?

MagicLegend commented 2 years ago

Hi @melloware! I have not touched PrimeReact in the last two years, but looking back at this ticket my issue was that the typings and docs were incomplete. Looking at the new docs link, there still is no mention of the availability of a RegEx prop on the component. That was my issue, and looking at the docs (I didn't check the typings) that's still an issue.

melloware commented 2 years ago

OK @MagicLegend let me look.

melloware commented 2 years ago

OK its because it extends the standard React extends Omit<React.DetailedHTMLProps<React.InputHTMLAttributes<HTMLInputElement>, HTMLInputElement>

All those undocumented properties are inherited from the standard React component. They are documented here: https://use-form.netlify.app/interfaces/_node_modules__types_react_index_d_.react.inputhtmlattributes.html I wonder if this page should contain a link.

MagicLegend commented 2 years ago

@melloware No, that's not really what I was going after. What I meant was that the InputNumber component renders an InputText component using a lot of props that aren't documented - pattern being one of them:

Updated reference: https://github.com/primefaces/primereact/blob/ca99ec0e3e62c981a81c5e455279998e40245adc/components/lib/inputnumber/InputNumber.js#L938-L944

Which is because InputText renders a default HTML5 <input> element: https://github.com/primefaces/primereact/blob/ca99ec0e3e62c981a81c5e455279998e40245adc/components/lib/inputtext/InputText.js#L48

Which in turn would accept a scala of attributes: https://devdocs.io/html/element/input

But my point is that the InputNumber docs don't mention most of the props that are passed in the first snippet. Perhaps a link to MSDN docs stating that all mentioned attributes work would already be a helpful nudge :)

melloware commented 2 years ago

Ahhh OK then your ticket is misleading you said InputText is not documented its really InputNumber has missing properties in the document.

melloware commented 2 years ago

Wait pattern is documented in InputNumber ???

The pattern attribute specifies a regular expression that the element's value is checked against on form submission.

So I am still confused to the issue. Besides adding a link to InputText to point to all HTML5 attributes what else can be done here?