WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#37973 closed defect (bug) (fixed)

When searching installed plugins "select all" checkbox fails

Reported by: corradomatt Owned by: swissspidy
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.6
Component: Plugins Keywords: has-patch commit
Focuses: javascript Cc:

Description

When on the plugin list page (installed plugins) and searching for a set of plugins the "select all" checkboxes above and below the list of plugins do not work to select all plugins in the search results.

Attachments (3)

Untitled.png (87.6 KB) - added by corradomatt 3 years ago.
37973.diff (630 bytes) - added by afercia 3 years ago.
37973.2.diff (1.7 KB) - added by afercia 3 years ago.

Download all attachments as: .zip

Change History (17)

@corradomatt
3 years ago

#1 @afercia
3 years ago

  • Focuses javascript added
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.7
  • Owner set to afercia
  • Status changed from new to assigned
  • Version 4.6.1 deleted

@corradomatt thanks for your report and welcome to Trac! Yep, confirmed. Looks like after refreshing the search results page, then it works.

#2 @afercia
3 years ago

  • Keywords shiny-updates added
  • Owner changed from afercia to swissspidy
  • Version set to 4.6

Oh well, after the introduction of Shiny Updates v2 in 4.6 the plugins search is now via AJAX so I guess the events need to be delegated.

This ticket was mentioned in Slack in #core by afercia. View the logs.


3 years ago

@afercia
3 years ago

#4 @afercia
3 years ago

  • Keywords has-patch added; needs-patch removed

The attached patch just delegates the click event on the body.

#5 @corradomatt
3 years ago

Ah, yeah..I noticed that the refresh fixed it too. Thanks for the quick reply @afercia

#6 @swissspidy
3 years ago

  • Keywords commit added

Patch works as expected.

I was going to say that this needs to be added to the themes list table in the network admin as well, but it looks like we didn't implement Ajax search there. Something for a new ticket (any volunteers?).

#7 @Nathan Miller
3 years ago

Hi all,

Thanks for the patch. Tested it and it is working. Great work.

Will let you know if there is anything else that is needing work on :)

Cheers. N

#8 follow-up: @ocean90
3 years ago

  • Keywords needs-patch added; shiny-updates has-patch commit removed

Looks like $('tbody').children().children('.check-column').find(':checkbox').click( function(e) {} needs also to be delegated.

#9 in reply to: ↑ 8 ; follow-up: @afercia
3 years ago

Replying to ocean90:

Looks like $('tbody').children().children('.check-column').find(':checkbox').click( function(e) {} needs also to be delegated.

Hm if I get it correctly that block of JS is meant to invert the selection, right? I was unaware this functionality even existed and it seems it's not communicated in the UI interface in any way. Unless I'm missing something, I'd be in favour of removing it entirely.

#10 in reply to: ↑ 9 ; follow-up: @swissspidy
3 years ago

Replying to afercia:

Replying to ocean90:

Looks like $('tbody').children().children('.check-column').find(':checkbox').click( function(e) {} needs also to be delegated.

Hm if I get it correctly that block of JS is meant to invert the selection, right? I was unaware this functionality even existed and it seems it's not communicated in the UI interface in any way. Unless I'm missing something, I'd be in favour of removing it entirely.

That code is used for checking the boxes in combination with pressing the shift key, e.g. when clicking the 1st and shift-clicking the 5th item afterwards, checkboxes 2-4 will be checked as well.

#11 in reply to: ↑ 10 @afercia
3 years ago

Replying to swissspidy:

That code is used for checking the boxes in combination with pressing the shift key, e.g. when clicking the 1st and shift-clicking the 5th item afterwards, checkboxes 2-4 will be checked as well.

Ah that too, right! It also inverts the current selection when shift-clicking on the "Select All" checkboxes (no ;) )
Wondering how many users are aware of this

Last edited 3 years ago by afercia (previous) (diff)

@afercia
3 years ago

#12 @afercia
3 years ago

  • Keywords has-patch commit added; needs-patch removed

Refreshed patch to delegate also the other event.

#13 @swissspidy
3 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 38703:

Plugins: Fix checkbox selection when searching for installed plugins.

Since the plugin search works via Ajax, the JavaScript events need to delegated in order for the checkboxes to work properly.

Props afercia.
Fixes #37973.

#14 @swissspidy
3 years ago

In 38706:

Plugins: Fix odd typo introduced in [38703].

even should of course be event.

See #37973.

Note: See TracTickets for help on using tickets.