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

Only load Vue Serp component if available #10361

Merged
merged 1 commit into from Sep 10, 2018

Conversation

Projects
None yet
4 participants
@matks
Contributor

matks commented Sep 10, 2018

Questions Answers
Branch? develop
Description? Fix JS error on BO pages where Serp Vue page was used although not active
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket?
How to test? Go on BO catalog page /admin/index.php/sell/catalog/products on develop. Use Developer tools to check there is a JS error (see below). This PR fixes it.

Tech bug description

Serp Vue component does not activate if there is no selector #serp-app on the page:
https://github.com/PrestaShop/PrestaShop/blob/develop/admin-dev/themes/new-theme/js/app/utils/serp/index.js#L38

however it was being used in Product page index.js:
https://github.com/PrestaShop/PrestaShop/blob/develop/admin-dev/themes/new-theme/js/product-page/index.js#L44

so this triggered a JS error:

main.bundle.js:69817 Uncaught TypeError: Cannot read property '$refs' of undefined
    at HTMLDocument.<anonymous> (main.bundle.js:69817)
    at c (main.bundle.js:68392)
    at u (main.bundle.js:68392)

This PR only uses the Vue component if it is active


This change is Reviewable

@matks

This comment has been minimized.

Show comment
Hide comment
@matks

matks Sep 10, 2018

Contributor

Screenshot of the JS error:
capture d ecran 2018-09-10 a 11 18 48

Contributor

matks commented Sep 10, 2018

Screenshot of the JS error:
capture d ecran 2018-09-10 a 11 18 48

@matks matks requested a review from Quetzacoalt91 Sep 10, 2018

@matks

This comment has been minimized.

Show comment
Hide comment
@matks

matks Sep 10, 2018

Contributor

@Quetzacoalt91 I think you know this topic well (because of #9444) so you're the best reviewer for this PR ;)

Contributor

matks commented Sep 10, 2018

@Quetzacoalt91 I think you know this topic well (because of #9444) so you're the best reviewer for this PR ;)

@Quetzacoalt91

This comment has been minimized.

Show comment
Hide comment
@Quetzacoalt91

Quetzacoalt91 Sep 10, 2018

Member

Okay, I did not know the scripts were also loaded on the products list page.

Thanks @matks

Member

Quetzacoalt91 commented Sep 10, 2018

Okay, I did not know the scripts were also loaded on the products list page.

Thanks @matks

@ntiepresta ntiepresta added QA ✔️ and removed waiting for QA labels Sep 10, 2018

@matks matks merged commit cb4c857 into PrestaShop:develop Sep 10, 2018

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@matks matks deleted the matks:fix-js-catalog-page-serp branch Sep 10, 2018

@matks matks added this to the 1.7.5.0 milestone Sep 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment