Fixed :: limit listening to AJAX actions to set `Prefers reduced moti…#5536
Fixed :: limit listening to AJAX actions to set `Prefers reduced moti…#5536bplv112 wants to merge 5 commits intoWordPress:trunkfrom
Conversation
src/js/_enqueues/admin/common.js
Outdated
| $( 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.
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.
@afercia Thank you for the suggestion. I've updated it to window.pagenow
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.
| 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.
@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.
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.