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

Fix cart item quantity change rollback #1418

Merged
merged 1 commit into from
Jan 14, 2019
Merged

Fix cart item quantity change rollback #1418

merged 1 commit into from
Jan 14, 2019

Conversation

jbruni
Copy link
Contributor

@jbruni jbruni commented Jan 11, 2019

What?

This is a bug fix.

Context: change the quantity of a cart line item by manually editing the quantity input field value.

Currently, when a manual quantity change fails to succeed (for example, because it is greater than the maximum purchasable quantity), the quantity is reverted to zero in the input field.

It should rollback to the previous value. Not to zero.

This PR fixes this bug.


It is quite simple to see what is the problem in the code:

  • inside the quantity input focus event handler function, this is used as a reference to the event target element (jQuery binds the event target element to the event handler function context);
  • as an arrow function is being used for the event handler function, this is not referencing the event target element, as it should, but the enclosing context (Cart class instance)

Thus, the fix was also quite simple: only a matter of using a normal function. This way, we have the event target element properly bound to the function's this context. The preVal (previous value) variable is properly populated with the input's value.

@bigbot
Copy link

bigbot commented Jan 11, 2019

Autotagging @bigcommerce/storefront-team @davidchin

@karen-white
Copy link

Thanks @jbruni ! We'll get this reviewed.

Copy link
Contributor

@mattolson mattolson left a comment

Choose a reason for hiding this comment

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

Thanks @jbruni !

@mattolson
Copy link
Contributor

@jbruni Can you add a changelog entry, and then I'll merge this.

@junedkazi junedkazi merged commit f47f1d8 into bigcommerce:master Jan 14, 2019
@jbruni jbruni deleted the patch-2 branch January 15, 2019 00:08
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.

5 participants