Skip to content
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

InputNumber: Doesn't allow minus sign in currency mode #4875

Closed
VasiliPodoplielov opened this issue Sep 4, 2023 · 17 comments · Fixed by #4881 or #4904
Closed

InputNumber: Doesn't allow minus sign in currency mode #4875

VasiliPodoplielov opened this issue Sep 4, 2023 · 17 comments · Fixed by #4881 or #4904
Assignees
Labels
Type: Bug Issue contains a defect related to a specific component.
Milestone

Comments

@VasiliPodoplielov
Copy link

Describe the bug

Hello.
I faced problem in InputNumber when I use this component like this:
<InputNumber mode="currency" currency="USD" />
It uses prefix in input under the hood. So the problem is related not only to the currency mode but also when I use any prefix.

When I populate some value to the field and after that want to make this value negative by adding - minus sign - it happens nothing. After some investigation I found out that problem might be related to this part of code:
image
because selectionStart=1, prefix has a position 0 and minus sign should be on -1 position.

Another problem, when I have a negative value with prefix inside an input, e.g. -$123 I cant remove value from input by pressing backspace. I need highlight the whole value and after that press backspace or delete button.

Reproducer

https://codesandbox.io/s/primereact-demo-forked-pj7tdw?file=/src/App.js

PrimeReact version

9.6.2

React version

18.x

Language

TypeScript

Build / Runtime

Create React App (CRA)

Browser(s)

No response

Steps to reproduce the behavior

  1. Type some number
  2. Set cursor at the start of number(before the number)
  3. Try press minus sign

Expected behavior

It should be possible to add minus sign when cursor is at the start(before the number).
It shuold be possible to remove negative number by pressing backspace.

@VasiliPodoplielov VasiliPodoplielov added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Sep 4, 2023
@melloware melloware added Type: Bug Issue contains a defect related to a specific component. and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Sep 4, 2023
akshayantony55 added a commit to qburst/primereact_qb_4 that referenced this issue Sep 6, 2023
akshayantony55 added a commit to qburst/primereact_qb_4 that referenced this issue Sep 6, 2023
akshayantony55 added a commit to qburst/primereact_qb_4 that referenced this issue Sep 6, 2023
akshayantony55 added a commit to qburst/primereact_qb_4 that referenced this issue Sep 6, 2023
@melloware melloware added this to the 10.0.0 milestone Sep 6, 2023
melloware added a commit that referenced this issue Sep 6, 2023
…y mode (#4881)

* Fix primefaces #4875 InputNumber: Doesn't allow minus sign in currency mode

* Fix primefaces #4875 InputNumber: Doesn't allow minus sign in currency mode

* Fix primefaces #4875 InputNumber: Doesn't allow minus sign in currency mode

* Fix primefaces #4875 InputNumber: Doesn't allow minus sign in currency mode

* Update components/lib/inputnumber/InputNumber.js

Co-authored-by: Melloware <[email protected]>

---------

Co-authored-by: akshayaqburst <[email protected]>
Co-authored-by: Melloware <[email protected]>
@VasiliPodoplielov
Copy link
Author

Hi. Does the solution fix the problem when I can't remove negative value from the field?
I mean there is -$1.00 in the field and the cursor is between 1 and .
When changes will be available to use?
Thanks.

@melloware
Copy link
Member

@akshayantony55 can you test the above scenario?

@akshayaqburst
Copy link
Contributor

@melloware Where can I test the recently merged changes?

@melloware
Copy link
Member

just do a git pull from master and run npm run dev it runs the full showcase with the current code.

@VasiliPodoplielov
Copy link
Author

@melloware
I cloned master, run npm install, run npm run dev, open http://localhost:3000/inputnumber/ and tried type - before an existed number.
image

@akshayaqburst
Copy link
Contributor

Seems there is a typo with the suggested code. I will create a new PR soon.

@VasiliPodoplielov
Copy link
Author

I added - and it is between dollar sign and numbers and after blur value in field formats and looks as expected.
Another problem - I can add - in any place, but after that the field is cleaned.
Case that I described in previos message - is not fixed.
image
image

@akshayantony55
Copy link
Contributor

@VasiliPodoplielov If the value is invalid, it cleans the input field. Thats an expected behaviour.
I will work on supporting removing - sign + the typo introduced with the previous pull request.

@VasiliPodoplielov
Copy link
Author

@akshayaqburst
Hm...I thought that input should be blocked from not allowed symbols.
Thanks.

@akshayaqburst
Copy link
Contributor

@melloware I have fixed the typo issue with the previous PR and also added the ability to remove - sign as requested by @VasiliPodoplielov.
Thanks,

@melloware melloware reopened this Sep 6, 2023
@github-actions github-actions bot added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Sep 6, 2023
@VasiliPodoplielov
Copy link
Author

Sorry guys, is it a new feature that now it is possible to fill in the InputNumber field by numbers and letters?
I saw previous comments that field value will be cleared. I just compare with current behaviour in the last version.
image

@melloware
Copy link
Member

it should not be. What was the last version that worked. To me it should NOT take characters.

@melloware
Copy link
Member

melloware commented Sep 6, 2023

@VasiliPodoplielov can you update my reproducer to show the issue? https://stackblitz.com/edit/react-fv1wob?file=src%2FApp.js

I can't reproduce so can you show me your exact settings that make it happen.

@melloware melloware removed the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Sep 6, 2023
akshayantony55 added a commit to qburst/primereact_qb_5 that referenced this issue Sep 6, 2023
@melloware
Copy link
Member

OK there are bigger issues here I need to investigate.

melloware added a commit to melloware/primereact that referenced this issue Sep 7, 2023
melloware added a commit to melloware/primereact that referenced this issue Sep 7, 2023
melloware added a commit to melloware/primereact that referenced this issue Sep 7, 2023
melloware added a commit to melloware/primereact that referenced this issue Sep 7, 2023
@melloware melloware assigned melloware and unassigned akshayaqburst Sep 7, 2023
@melloware
Copy link
Member

OK I think I got all the issues. only 1 minus sign. BACKSPACE works to remove the minus sign. It goes in the correct place etc.

@VasiliPodoplielov
Copy link
Author

@melloware looks great.
Thanks everyone who took part in fixing process.

Sorry, when these changes will be available to use?

@melloware
Copy link
Member

Thanks for testing! I think 10.0.0 is due out next week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment