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(themes): Enhance product options validation in cart page #500

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

eslamoo
Copy link
Member

@eslamoo eslamoo commented Dec 19, 2024

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

What is the current behaviour? (You can also link to an open issue here)

What is the new behaviour? (You can also link to the ticket here)

Does this PR introduce a breaking change?

Screenshots (If appropriate)

@SallaDev SallaDev marked this pull request as draft December 19, 2024 14:20
@eslamoo
Copy link
Member Author

eslamoo commented Dec 19, 2024

/autoupdate

@SallaDev
Copy link
Contributor

The branch bugfix/TD-3578-enhance-product-options-validation-in-cart of #500 Branch is up-to-date..✅

@eslamoo eslamoo marked this pull request as ready for review December 19, 2024 15:32
});
}
});
observer.observe(quantityComponent, { childList: true, subtree: true });
Copy link
Contributor

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: You have a misspelled word: subtree on Identifier

The issue reported by the ESLint linter indicates that the word "subtree" is misspelled in the identifier within the observer.observe method call. This is likely a false positive, as "subtree" is a valid option in the MutationObserver API, but the linter might be configured with a custom dictionary that does not recognize it.

To address the issue, you can add a comment to disable the specific ESLint rule for that line, which will prevent the linter from flagging it as an error. Here’s the suggested code change:

Suggested change
observer.observe(quantityComponent, { childList: true, subtree: true });
observer.observe(quantityComponent, { childList: true, subtree: true }); // eslint-disable-line no-undef

This change allows you to keep the code as is while silencing the linter warning. If the linter is indeed misconfigured, you may also want to review the ESLint settings to ensure that valid identifiers like "subtree" are recognized.


This comment was generated by an experimental AI tool.

@@ -1,4 +1,5 @@
import BasePage from './base-page';
import {validateProductOptions} from './partials/validate-product-options';
Copy link
Contributor

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: Import path mush be a path alias

The issue described by the ESLint linter indicates that the import path for validateProductOptions is not using a path alias, which is likely a requirement set in the project's ESLint configuration. Path aliases help to simplify imports and make the codebase more maintainable by avoiding relative paths.

To fix this issue, you would need to replace the relative import path with the appropriate path alias defined in your project's configuration. Assuming the alias for the partials directory is set to @partials, the line can be modified as follows:

Suggested change
import {validateProductOptions} from './partials/validate-product-options';
import {validateProductOptions} from '@partials/validate-product-options';

This comment was generated by an experimental AI tool.

overlay.classList.add('loading-overlay', 'absolute', 'inset-0', 'z-50', 'flex', 'justify-center', 'items-center');

const background = document.createElement('div');
background.classList.add('absolute', 'inset-0', 'bg-white', 'opacity-40');
Copy link
Contributor

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: You have a misspelled word: bg on String

The issue identified by the ESLint linter indicates that the word "bg" in the class name "bg-white" is considered a misspelling. This is likely due to ESLint's dictionary not recognizing "bg" as a valid abbreviation for "background."

To resolve this issue, we can replace "bg-white" with "background-white" if that class exists in the CSS framework being used. However, if "bg-white" is indeed a valid class (for example, in Tailwind CSS), we might want to ignore this specific ESLint rule for this line instead.

Assuming "bg-white" is correct and we want to ignore the ESLint warning, here’s the code suggestion:

Suggested change
background.classList.add('absolute', 'inset-0', 'bg-white', 'opacity-40');
background.classList.add('absolute', 'inset-0', 'bg-white', 'opacity-40'); // eslint-disable-line spelling/spell-checker

If "bg-white" is not valid and we need to change it, it would be:

Suggested change
background.classList.add('absolute', 'inset-0', 'bg-white', 'opacity-40');
background.classList.add('absolute', 'inset-0', 'background-white', 'opacity-40');

Please choose the appropriate suggestion based on your CSS framework.


This comment was generated by an experimental AI tool.

@@ -1,4 +1,5 @@
import BasePage from './base-page';
import {validateProductOptions} from './partials/validate-product-options';
Copy link
Contributor

Choose a reason for hiding this comment

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

Codacy has a fix for the issue: Expected 'multiple' syntax before 'single' syntax.

Suggested change
import {validateProductOptions} from './partials/validate-product-options';
import BasePage from './base-page';

updateCartItemState(cartItems, currentProduct);
})
.then(() => removeLoadingOverlay())
.catch(error => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Codacy has a fix for the issue: Expected parentheses around arrow function argument.

Suggested change
.catch(error => {
.catch((error) => {

.getCurrentCartId(false, "salla-product-options")
.then((cartId) => salla.cart.details(cartId, ['options']))
.then(({ data: { cart: cartDetails } }) => {
const currentProduct = cartDetails.items.find(item => Number(item.id) === Number(itemId));
Copy link
Contributor

Choose a reason for hiding this comment

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

Codacy has a fix for the issue: Expected parentheses around arrow function argument.

Suggested change
const currentProduct = cartDetails.items.find(item => Number(item.id) === Number(itemId));
const currentProduct = cartDetails.items.find((item) => Number(item.id) === Number(itemId));

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.

2 participants