chore(Number Input): Convert examples to TypeScript #7980
Conversation
gefgu
changed the title
|
Preview: https://patternfly-react-pr-7980.surge.sh A11y report: https://patternfly-react-pr-7980-a11y.surge.sh |
gefgu
force-pushed
the
chore/convert-number-examples-to-ts
branch
from
e47dc9b to
f7e384b
Compare
nicolethoen
left a comment
nicolethoen
left a comment
There was a problem hiding this comment.
Could you update the file names of the examples to be NumberInput<ExampleName>.tsx?
packages/react-core/src/components/NumberInput/examples/NumberCustomStep.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| export const UnitNumberInput: React.FunctionComponent = () => { | ||
| const [value1, setValue1] = React.useState(90); | ||
| const [value2, setValue2] = React.useState(Number((1.0).toFixed(2))); |
There was a problem hiding this comment.
So I'm noticing here that the 1.00 which was part of the original example is now displaying at 1 - and it's actually because by passing (1.0).toFixed(2) to a Number constructor, you are casting a string to a number and losing the extra trailing two zeros.
This is necessary because the type of the value prop on NumberInput is number | null, so it cannot accept a string 1.00 value. It's possible we could update the number input value to accept strings and cast them to numbers internally. Not sure what side effects there'd be for that.
@tlabaj I'm not sure if you have thoughts about how we can handle this. I can open a follow up issue if we want - only if we are happy to the update to the example
There was a problem hiding this comment.
I think getting rid of Number here will fix the issue.
There was a problem hiding this comment.
But the value prop has the type Number, and type of (1.0).toFixed(2) is string
There was a problem hiding this comment.
I would say it is fine as is then. The previous implementation was technically incorrect since it had value2: (1.0).toFixed(2)
gefgu
force-pushed
the
chore/convert-number-examples-to-ts
branch
from
f7e384b to
24a4df0
Compare
|
@nicolethoen thanks for the kind feedback. This really helps my learning journey :) Once you decide about the issue with |
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
4 participants
What: Closes #7672
Convert Number Input examples to TypeScript.
Additional issues: