WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#20261 closed enhancement (fixed)

The 'check all' checkboxes don't get checked/unchecked when appropriate

Reported by: johnbillion Owned by: markjaquith
Milestone: 3.5 Priority: normal
Severity: minor Version: 3.3.1
Component: Administration Keywords: has-patch ux-feedback
Focuses: Cc:

Description

Steps to reproduce:

  1. Click the check box at the top of the checkbox column on any list screen (posts, pages, users, plugins, etc).
  2. Uncheck one of the individual row checkboxes.
  3. The check box at the top of the checkbox column doesn't get unchecked as you might expect.

Attachments (9)

20261-1.diff (1.1 KB) - added by jkudish 6 years ago.
checks/unchecks the "all" checkbox upon checking of any other checkbox (according to the state of all the checkboxes)
20261-2.diff (1.0 KB) - added by jkudish 6 years ago.
adjusted the comment above the function so it's more fitting to what it actually does
20261-3.diff (1.0 KB) - added by jkudish 6 years ago.
revised patch for readability and efficiency
20261-4.diff (398 bytes) - added by SergeyBiryukov 6 years ago.
20261-5.diff (1.2 KB) - added by jkudish 6 years ago.
20261-6.diff (1.1 KB) - added by jkudish 6 years ago.
remove left-over console.log()
20261-7.diff (1.1 KB) - added by jkudish 6 years ago.
use $current_checkbox for $current_table selector
20261-8.diff (1.0 KB) - added by jkudish 6 years ago.
remove svn:ignore that was accidentally included in the previous patch
20261-9.diff (560 bytes) - added by SergeyBiryukov 6 years ago.

Download all attachments as: .zip

Change History (26)

#1 @scribu
6 years ago

  • Keywords needs-patch added

#2 @johnbillion
6 years ago

Personally I don't think the indeterminate state is appropriate for a 'check all' checkbox, but it'd be interesting to see what the consensus is among other apps.

@jkudish
6 years ago

checks/unchecks the "all" checkbox upon checking of any other checkbox (according to the state of all the checkboxes)

#3 @jkudish
6 years ago

  • Cc joachim.kudish@… added
  • Keywords has-patch dev-feedback needs-testing ux-feedback added; needs-patch removed

@jkudish
6 years ago

adjusted the comment above the function so it's more fitting to what it actually does

#4 @jkudish
6 years ago

Any reason this couldn't make it into 3.4?

#5 follow-up: @johnbillion
6 years ago

It's low priority, it needs UX feedback, it needs testing, and 3.4 is past feature freeze.

#6 in reply to: ↑ 5 @jkudish
6 years ago

Replying to johnbillion:

It's low priority, it needs UX feedback, it needs testing, and 3.4 is past feature freeze.

Good point. Early 3.5 then :)

#7 @jkudish
6 years ago

  • Keywords dev-feedback removed
  • Severity changed from minor to normal

Reviewed/revised my patch against the latest trunk and revised it for readability and efficiency. Would love to see it in 3.5-early :)

@jkudish
6 years ago

revised patch for readability and efficiency

#8 @SergeyBiryukov
6 years ago

  • Severity changed from normal to minor

Seems we don't need to iterate through all the checkboxes. Getting a status of the current clicked one is enough (20261-4.diff).

#9 @SergeyBiryukov
6 years ago

Ah, I only tested unchecking, as per the description of the ticket.

If we want to check them automatically as well, would still need to iterate.

#10 @SergeyBiryukov
6 years ago

  • Keywords 2nd-opinion added; needs-testing removed

#11 follow-up: @dd32
6 years ago

A combination could be used.

If the action is uncheck, skip iteration, we know not all are checked.

We can also shift the iteration into jQuery itself, by using the :checkbox :not(:checked) selector to select any checkboxes that are NOT checked, If empty, they're all selected.

@jkudish
6 years ago

#12 in reply to: ↑ 11 @jkudish
6 years ago

Replying to dd32:

A combination could be used.

That's exactly what I am doing in 20261-5.diff

@jkudish
6 years ago

remove left-over console.log()

@jkudish
6 years ago

use $current_checkbox for $current_table selector

@jkudish
6 years ago

remove svn:ignore that was accidentally included in the previous patch

#13 @jkudish
6 years ago

would love to see this in 3.5-early :)

#14 @jkudish
6 years ago

  • Keywords 3.5-early added; 2nd-opinion removed

#15 follow-up: @SergeyBiryukov
6 years ago

  • Keywords 3.5-early removed
  • Milestone changed from Awaiting Review to 3.5

20261-9.diff is an attempt to simplify the patch.

Only visible checkboxes are taken into account (related: #8355, #8520).

#16 in reply to: ↑ 15 @jkudish
6 years ago

20261-9.diff looks good to me

#17 @markjaquith
6 years ago

  • Owner set to markjaquith
  • Resolution set to fixed
  • Status changed from new to closed

In [21209]:

Uncheck the "select all" checkbox when one of the child checkboxes is manually unchecked. props SergeyBiryukov. fixes #20261

Note: See TracTickets for help on using tickets.