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

Remove duplicate call to $(document).ready #42

Merged
merged 1 commit into from Oct 9, 2019

Conversation

@GCDeveloper
Copy link
Contributor

GCDeveloper commented Apr 19, 2019

Questions Answers
Branch? develop
Description? This seemed to cause a bug for some users who could not reach the checkout page. This duplicate call would register the 'updateCart' event listener twice, therefore making 2 ajax post requests.
Type? bugfix
BC breaks? no
Deprecations? no
Fixed ticket?
How to test? Please check everything during the cart process works fine as we removed an unnecessary piece of code.
@GCDeveloper GCDeveloper changed the base branch from master to dev Apr 19, 2019
@matks

This comment has been minimized.

Copy link
Contributor

matks commented Jun 14, 2019

Seems to make sense 👌
@PierreRambaud any idea of a usecase where a double $(document).ready(function () { would be useful ?

@matks

This comment has been minimized.

Copy link
Contributor

matks commented Jul 19, 2019

Seems to make sense 👌
@PierreRambaud any idea of a usecase where a double $(document).ready(function () { would be useful ?

@PierreRambaud ping 😄

@GCDeveloper

This comment has been minimized.

Copy link
Contributor Author

GCDeveloper commented Jul 19, 2019

@PierreRambaud

This comment has been minimized.

Copy link
Contributor

PierreRambaud commented Jul 19, 2019

I think the problem come from elsewhere.
$(document).ready can be used many times, it's just here to say "Run when dom is fully loaded".
More because sometimes you need global variables available store in the dom.
You can do

$(document).ready(prepareTabs); 
$(document).ready(prepareTables); 
$(document).ready(prepareMenu); 

As you said, the problem is this callback is called twice

@GCDeveloper

This comment has been minimized.

Copy link
Contributor Author

GCDeveloper commented Jul 19, 2019

The $(document).ready is nested inside another $(document).ready, it's clearly illogical and should be removed regardless.

@PierreRambaud

This comment has been minimized.

Copy link
Contributor

PierreRambaud commented Jul 19, 2019

ohhh my bad, didn't see the nesting :D
In this case, yes it's useless :D

@matks matks force-pushed the GCDeveloper:patch-1 branch from 9710e82 to 6ac2fca Oct 7, 2019
@matks matks changed the title remove duplicate call to $(document).ready Remove duplicate call to $(document).ready Oct 7, 2019
This seemed to cause a bug for some users who could not reach the checkout page.
This duplicate call would register the 'updateCart' event listener twice, therefore making 2 ajax post requests.
@matks matks force-pushed the GCDeveloper:patch-1 branch from 6ac2fca to 9f90e64 Oct 7, 2019
@matks

This comment has been minimized.

Copy link
Contributor

matks commented Oct 7, 2019

PR rebased

@matks
matks approved these changes Oct 7, 2019
@matks matks added the Waiting for QA label Oct 7, 2019
@Robin-Fischer-PS Robin-Fischer-PS self-assigned this Oct 9, 2019
@Robin-Fischer-PS

This comment has been minimized.

Copy link

Robin-Fischer-PS commented Oct 9, 2019

Thanks @GCDeveloper for the PR, it's good for me :)

@matks

This comment has been minimized.

Copy link
Contributor

matks commented Oct 9, 2019

@matks matks merged commit a90b77c into PrestaShop:dev Oct 9, 2019
@matks matks referenced this pull request Oct 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.