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(storefront): BCTHEME-459 fix product quantity change error #2052

Merged
merged 3 commits into from
May 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- Use https:// for schema markup. [#2039](https://github.com/bigcommerce/cornerstone/pull/2039)
- Update focus tooltip styles contrast to achieve accessibility AA Complaince. [#2047](https://github.com/bigcommerce/cornerstone/pull/2047)
- Apple pay button displaying needs to be fixed. [#2043](https://github.com/bigcommerce/cornerstone/pull/2043)
- Fixed NaN error on increase/decrease product quantity by adding field validation. [#2052](https://github.com/bigcommerce/cornerstone/pull/2052)

## 5.4.0 (04-26-2021)
- Incorrect focus order for product carousels. [#2034](https://github.com/bigcommerce/cornerstone/pull/2034)
Expand Down
39 changes: 39 additions & 0 deletions assets/js/theme/common/models/forms.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,45 @@ const forms = {
notEmpty(value) {
return value.length > 0;
},

/**
* validates a field like product quantity
* @param value
* @returns {boolean}
*
*/
numbersOnly(value) {
const re = /^\d+$/;
return re.test(value);
},

/**
* validates increase in value does not exceed max
* @param {number} value
* @param {number} max
* @returns {number}
*
*/
validateIncreaseAgainstMaxBoundary(value, max) {
const raise = value + 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use incremented and decremented instead of raise and decline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's better to keep a noun as name for variable. incremented || incremented is a verb or adjective.

Copy link
Contributor

Choose a reason for hiding this comment

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

When I was reading this code I have decoded var names to this when understood their purpose. Maybe incrementedVal and decrementedVal. But I believe its not really crucial, so its up to u


if (!max || raise <= max) return raise;
return value;
},

/**
* validates decrease in value does not fall below min
* @param {number} value
* @param {number} min
* @returns {number}
*
*/
validateDecreaseAgainstMinBoundary(value, min) {
const decline = value - 1;

if (!min || decline >= min) return decline;
return value;
},
};

export default forms;
58 changes: 36 additions & 22 deletions assets/js/theme/common/product-details.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import 'foundation-sites/js/foundation/foundation.reveal';
import ImageGallery from '../product/image-gallery';
import modalFactory, { showAlertModal } from '../global/modal';
import { isEmpty, isPlainObject } from 'lodash';
import nod from '../common/nod';
import { announceInputErrorMessage } from '../common/utils/form-utils';
import forms from '../common/models/forms';
import { normalizeFormData } from './utils/api';
import { isBrowserIE, convertIntoArray } from './utils/ie-helpers';
import bannerUtils from './utils/banner-utils';
Expand All @@ -23,6 +26,12 @@ export default class ProductDetails extends ProductDetailsBase {
this.storeInitMessagesForSwatches();

const $form = $('form[data-cart-item-add]', $scope);

this.addToCartValidator = nod({
submit: $form.find('input#form-action-addToCart'),
tap: announceInputErrorMessage,
});

const $productOptionsElement = $('[data-product-option-change]', $form);
const hasOptions = $productOptionsElement.html().trim().length;
const hasDefaultOptions = $productOptionsElement.find('[data-default]').length;
Expand All @@ -41,7 +50,10 @@ export default class ProductDetails extends ProductDetailsBase {
}
};

$(window).on('load', () => $.each($productSwatchLabels, placeSwatchLabelImage));
$(window).on('load', () => {
this.registerAddToCartValidation();
$.each($productSwatchLabels, placeSwatchLabelImage);
});

if (context.showSwatchNames) {
this.$swatchOptionMessage.removeClass('u-hidden');
Expand All @@ -65,7 +77,11 @@ export default class ProductDetails extends ProductDetailsBase {
});

$form.on('submit', event => {
this.addProductToCart(event, $form[0]);
this.addToCartValidator.performCheck();

if (this.addToCartValidator.areAll('valid')) {
this.addProductToCart(event, $form[0]);
}
});

// Update product attributes. Also update the initial view in case items are oos
Expand All @@ -85,6 +101,19 @@ export default class ProductDetails extends ProductDetailsBase {
this.previewModal = modalFactory('#previewModal')[0];
}

registerAddToCartValidation() {
this.addToCartValidator.add([{
selector: '[data-quantity-change] > .form-input--incrementTotal',
validate: (cb, val) => {
const result = forms.numbersOnly(val);
cb(result);
},
errorMessage: this.context.productQuantityErrorMessage,
}]);

return this.addToCartValidator;
}

storeInitMessagesForSwatches() {
if (this.swatchGroupIdList.length && isEmpty(this.swatchInitMessageStorage)) {
this.swatchGroupIdList.each((_, swatchGroupId) => {
Expand Down Expand Up @@ -308,35 +337,20 @@ export default class ProductDetails extends ProductDetailsBase {
const quantityMin = parseInt($input.data('quantityMin'), 10);
const quantityMax = parseInt($input.data('quantityMax'), 10);

let qty = parseInt($input.val(), 10);

let qty = forms.numbersOnly($input.val()) ? parseInt($input.val(), 10) : quantityMin;
// If action is incrementing
if ($target.data('action') === 'inc') {
BC-tymurbiedukhin marked this conversation as resolved.
Show resolved Hide resolved
// If quantity max option is set
if (quantityMax > 0) {
// Check quantity does not exceed max
if ((qty + 1) <= quantityMax) {
qty++;
}
} else {
qty++;
}
qty = forms.validateIncreaseAgainstMaxBoundary(qty, quantityMax);
} else if (qty > 1) {
// If quantity min option is set
if (quantityMin > 0) {
// Check quantity does not fall below min
if ((qty - 1) >= quantityMin) {
qty--;
}
} else {
qty--;
}
qty = forms.validateDecreaseAgainstMinBoundary(qty, quantityMin);
}

// update hidden input
viewModel.quantity.$input.val(qty);
// update text
viewModel.quantity.$text.text(qty);
// perform validation after updating product quantity
this.addToCartValidator.performCheck();
});

// Prevent triggering quantity change when pressing enter
Expand Down
5 changes: 3 additions & 2 deletions assets/js/theme/common/utils/form-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,9 @@ function announceInputErrorMessage({ element, result }) {
}
const activeInputContainer = $(element).parent();
// the reason for using span tag is nod-validate lib
// which does not add error message class while initialising form
const errorMessage = $(activeInputContainer).find('span');
// which does not add error message class while initialising form.
// specific class is added since it can be multiple spans
const errorMessage = $(activeInputContainer).find('span.form-inlineMessage');

if (errorMessage.length) {
const $errMessage = $(errorMessage[0]);
Expand Down
4 changes: 4 additions & 0 deletions assets/scss/components/citadel/forms/_forms.scss
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,10 @@
text-align: center;
vertical-align: middle;
width: remCalc(35px);

.form-field--success & {
float:none;
}
}


Expand Down
4 changes: 4 additions & 0 deletions assets/scss/components/stencil/productView/_productView.scss
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,10 @@
font-size: 0; // 2
margin-bottom: 2rem;

&--error > .form-inlineMessage {
font-size: 1rem;
}

// scss-lint:disable SelectorDepth, NestingDepth
> .form-checkbox + .form-label
{
Expand Down
1 change: 1 addition & 0 deletions lang/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,7 @@
"change_product_options": "Change options for {name}",
"quantity_decrease": "Decrease Quantity of {name}",
"quantity_increase": "Increase Quantity of {name}",
"quantity_error_message":"The quantity should contain only numbers",
"purchase_units": "{quantity, plural, =0{0 units} one {# unit} other {# units}}",
"max_purchase_quantity": "Maximum Purchase:",
"min_purchase_quantity": "Minimum Purchase:",
Expand Down
3 changes: 3 additions & 0 deletions templates/components/amp/css/form.html
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,9 @@
.form-field--warning .form-input {
float: left
}
.form-field--success[data-quantity-change] .form-input {
float:none;
}
.form-field--success .form-input,
.form-field--success .form-select,
.form-field--success .form-checkbox + .form-label::before,
Expand Down
1 change: 1 addition & 0 deletions templates/components/products/add-to-cart.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<div id="add-to-cart-wrapper" {{#unless product.can_purchase}}style="display: none"{{/unless}}>
{{#if theme_settings.show_product_quantity_box}}
{{inject 'productQuantityErrorMessage' (lang 'products.quantity_error_message')}}
<div class="form-field form-field--increments">
<label class="form-label form-label--alternate"
for="qty[]">{{lang 'products.quantity'}}</label>
Expand Down