Support from Acquia helps fund testing for Drupal Acquia logo

Comments

thejacer87 created an issue. See original summary.

thejacer87’s picture

Issue summary: View changes
sorabh.v6’s picture

I also faced the same bug. I believe it is because we are using input type number. When I changed the input type to text then I did not saw that bug again. Please refer the attached gif -

mglaman’s picture

Wouldn't the proper fix to use `parseInt` in the JavaScript?

        if (_this.data('keybind') !== undefined) {
          self.output += parseInt(_this.data('keybind'));
        }
mglaman’s picture

Status: Active » Needs review
FileSize
485 bytes

Give it a try

Status: Needs review » Needs work

The last submitted patch, 5: 2933461-5.patch, failed testing. View results

mglaman’s picture

Okay, that doesn't work because it doesn't append. So the input needs to be changed to text, as specified in #3

sorabh.v6’s picture

@mglaman Changing this field to text will remove the validation of type number. Previously this field was a textfield and to apply validation I had to change that to number field. If we make it a textfield then we will have to apply custom validation.

sorabh.v6’s picture

@mglaman This is the issue where the field was changed from `text` to `number` - https://www.drupal.org/project/commerce_pos/issues/2916647

deepakaryan1988’s picture

Assigned: Unassigned » deepakaryan1988
deepakaryan1988’s picture

Status: Needs work » Needs review
FileSize
975 bytes

Hello,
I have made the patch for attaching .0 at the end. Now, I am uploading it but there is still lots of things to do.

I'll update the patch as I further progress.

Status: Needs review » Needs work

The last submitted patch, 11: payment-amount-disappear-2933461-11.patch, failed testing. View results

deepakaryan1988’s picture

Status: Needs work » Needs review
FileSize
1.92 KB

Thanks to @travis.
Submitting a patch now.

For later on, I'll disable " . " once it used once.

travis-bradbury’s picture

Status: Needs review » Needs work

This needs a test that fails without the fix.

TimRutherford’s picture

TimRutherford’s picture

Assigned: TimRutherford » Unassigned

After investigation, it does not seem possible to fix this in a clean way. The current solution in patch 13 has its issues and looks a bit confusing having a random 0 show up then get removed on next key press. I'd propose a couple solutions:

  1. Change the field type to text as mentioned by @mglaman. Then validate the number on submission (and possibly add validation when the element changes). Unfortunately you lose the step functionality and other things like the tablet keypad.
  2. Figure out a way to remove the validation from the number field. I couldn't figure a out a way to do this though.
  3. Only add the decimal once a number is pressed after it. This is not super intuitive though.
  4. Have the field default to $00.00 and as you enter numbers it will replace the 0's. Deleting would place a 0 and move the pointer back. This may be a bit confusing if the user enter's a 0 though as you cant see the "pointer" move.

Personally I think #1 is the way to go. Its easy and pretty clean. We would need to make sure we fix the issues in https://www.drupal.org/project/commerce_pos/issues/2916647 though.

subhojit777’s picture

Assigned: Unassigned » subhojit777
subhojit777’s picture

Status: Needs work » Needs review
FileSize
1.98 KB

Agree with Tim et al that the textfield approach looks much cleaner.

subhojit777’s picture

There is a problem with #18. Entering 1.22000000 also throws error.

subhojit777’s picture

Not neat :( but fixes the problem. If we want to make it absolutely neat, then I would recommend providing a message to the end-user that the amount will rounded up to two digits, and use round().

subhojit777’s picture

Assigned: subhojit777 » Unassigned

Lets decide whether to follow the JS/PHP approach to fix this problem, and then we can write tests.

jnrfred’s picture

Patch in #20 looks good. The amount does not disappear temporarily when you use the keypad.

travis-bradbury’s picture

Status: Needs review » Needs work

$form_state->setError($form['keypad']['amount'], $this->t('The amount should be of two digit precision.'));

Instead of "two" this should say the number from $fraction_digit.

jnrfred’s picture

This patch addresses #23 fixes.

gmem’s picture

Seeing as this is still an issue, I went ahead and rerolled the patch, as well as tweaked the styling on the input to limit the width - otherwise it overlaps the add cash amount button.

gmem’s picture

Status: Needs work » Needs review
smccabe’s picture

Status: Needs review » Needs work

The max width setting is overridden to 100% for me in both Chrome and FF, also I think it is better to set this via the field size from the form, not the CSS.

smccabe’s picture

gmem’s picture

Removed the styling and set the size attribute to 10 with the same effect.

Status: Needs review » Needs work
gmem’s picture

gmem’s picture

Status: Needs work » Needs review

  • smccabe committed 46f60b9 on 8.x-2.x authored by gmem
    Issue #2933461 by gmem, subhojit777, deepakaryan1988, mglaman, jnrfred,...
smccabe’s picture

sorabh.v6’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.