-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Fixed :: limit listening to AJAX actions to set `Prefers reduced moti… #5536
Fixed :: limit listening to AJAX actions to set `Prefers reduced moti… #5536
Conversation
src/js/_enqueues/admin/common.js
Outdated
@@ -2220,8 +2220,14 @@ $( function( $ ) { | |||
// Listen for jQuery AJAX events. | |||
( function( $ ) { | |||
$( document ).ajaxComplete( function( event, xhr, settings ) { | |||
|
|||
// Return early if this is not the plugin-install.php page. | |||
if ( ! $( document.body ).hasClass( 'plugin-install-php' ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe there are more standard ways in WordPress to check what a page is, see the followiing JS 'global' variables:
window.adminpage
will return 'plugin-install-php' which is the current page admin body CSS class
window.pagenow
will return 'plugin-install' which is che current screen ID
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@afercia Thank you for the suggestion. I've updated it to window.pagenow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bplv112 you're welcome and thanks for the PR!
src/js/_enqueues/admin/common.js
Outdated
// Check if this is the 'search-install-plugins' request. | ||
if ( settings.data && settings.data.includes( 'action=search-install-plugins' ) ) { | ||
if ( settings.data && typeof settings.data === "string" && settings.data.includes( 'action=search-install-plugins' ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if ( settings.data && typeof settings.data === "string" && settings.data.includes( 'action=search-install-plugins' ) ) { | |
if ( settings.data && typeof settings.data === 'string' && settings.data.includes( 'action=search-install-plugins' ) ) { |
Just to keep consistency, I think you can use 'string' instead of "string" because the single quote is a more strict sting and everywhere uses a single quote of this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rudlinkon Thank you for reviewing the PR.
I've updated it as per your suggestion
if ( window.pagenow !== 'plugin-install' ) { | ||
return; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced this change fixes the problem. The ajaxComplete is still listened to; it just exits under different conditions. Wouldn't it make more sense to move the page check as a wrapper before the listener?
if ( window.pagenow === 'plugin-install' ) {
$( document ).ajaxComplete( function( event, xhr, settings ) {
// etc.
});
}
The newly added jQuery function on 6.4 RC 1 listens to all completed Ajax requests. ef54f4e#diff-b9e9194f24d2a3548ae0b16b6bde5e0c7756a2b7668bed4d1040093e00550bf6R2201
The function expects string but if the data is an object it throws an error for other requests as well. ef54f4e#diff-b9e9194f24d2a3548ae0b16b6bde5e0c7756a2b7668bed4d1040093e00550bf6R2203
Trac ticket: https://core.trac.wordpress.org/ticket/59689
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.