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 swatch-renderer.js 843:866 #8105

Closed
wants to merge 1 commit into from
Closed

Fix swatch-renderer.js 843:866 #8105

wants to merge 1 commit into from

Conversation

aholovan
Copy link
Contributor

@aholovan aholovan commented Jan 11, 2017

Hello,
There is an error in console when product has more than one kind of swatches:
Uncaught TypeError: Cannot read property 'oldPrice' of undefined - swatch-renderer.js:843

Step to reproduce:

  1. Install Sample Data
  2. Set Special Price for some Simple of the Configurable product which has more than one kind of swatches, for example SKU: WJ03
  3. Go to PDP (../augusta-pullover-jacket.html)
  4. Select some Color and look to the console - there You will be able to see the error. This is because 'result' is null before all of the swatches are selected.

Some fixes have been added. Please, test.
Thanks.

Hello,
There is an error in console when product has more than one swatch: 
Uncaught TypeError: Cannot read property 'oldPrice' of undefined - swatch-renderer.js:843

Step to reproduce:
1) Install Sample Data
2) Go to PDP where product has  more than one swatch, for example ../augusta-pullover-jacket.html
3) Select some Color and look to the console - there You will be able to see the error. This is because 'result' is null before all of the swatches are selected.

Some fixes have been added. Please, test.
Thanks.
@vrann vrann requested review from zetlen and vrann March 1, 2017 21:24
@vrann vrann self-assigned this Mar 1, 2017
@vrann vrann added this to the March 2017 milestone Mar 1, 2017
Copy link

@zetlen zetlen left a comment

Choose a reason for hiding this comment

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

Just one logic question and one style change request.

} else {
$(this.options.slyOldPriceSelector).hide();
}
if (typeof(result) != "undefined") {
Copy link

Choose a reason for hiding this comment

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

Though the typeof operator can be used with parentheses, it is not recommended, because it can confuse linters and reviewers. This should be typeof result.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, it make sence.

$(this.options.tierPriceBlockSelector).html(tierPriceHtml).show();
}
} else {
$(this.options.tierPriceBlockSelector).hide();
Copy link

Choose a reason for hiding this comment

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

If result.tierPrices.length != 0 and !this.options.tierPriceTemplate, then this.options.tierPriceBlockSelector will be neither shown nor hidden. Is this the desired behavior?

Copy link
Contributor Author

@aholovan aholovan Mar 15, 2017

Choose a reason for hiding this comment

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

Thanks, I'll pull new code, investigte it and fix if needed.

@vrann
Copy link
Contributor

vrann commented Mar 13, 2017

@aholovan thanks for contribution! Can you please update PR per the comments?

@aholovan
Copy link
Contributor Author

I've created new pull request #8923.
So, that one can be closed.

@aholovan aholovan closed this Mar 17, 2017
magento-devops-reposync-svc pushed a commit that referenced this pull request Mar 14, 2023
…sues_29Jan22

Hammer release regression issues 29 jan22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants