[wp-trac] [WordPress Trac] #47048: Nav Menus Admin: Missing Deselect All text when toggling Select All

[WordPress Trac]

#47048: Nav Menus Admin: Missing Deselect All text when toggling Select All
-------------------------------------+-------------------------------------
 Reporter:  birgire                  |       Owner:  audrasjb
     Type:  defect (bug)             |      Status:  accepted
 Priority:  normal                   |   Milestone:  5.3
Component:  Menus                    |     Version:  3.0
 Severity:  normal                   |  Resolution:
 Keywords:  has-screenshots has-     |     Focuses:  ui, accessibility,
  patch                              |  administration
-------------------------------------+-------------------------------------

Comment (by afercia):

 Reviewing a bit [attachment:"47048-5.diff"], noticed a few things:

 - Within `attachTabsPanelListeners` the variable `selectAll` is
 `undefined`: it needs to be declared at the top of the function. Please
 run `grunt jshint` to lint the JS files
 - While it is a partially pre-existing thing, could anyone please refresh
 my mind about the usage of `removeAttr( 'checked' )` vs. `prop( 'checked',
 false )` and `prop( 'checked', true )`? Genuinely asking.
 - I'd rename `data-area` to something along the lines of "data-items-type"
 or something like that to better clarify what it is.
 - Originally, the various "Select all" were links. It's a pattern that was
 commonly used in WordPress for no-js compatibility. Progressive
 enhancement: the basic functionality was available also whit JS off (see
 it was using a `&selectall=1` param). Now that the "Select All" are
 checkboxes and their functionality is based on JavaScript, when JS support
 is off they're hidden with the CSS class `hide-if-no-js`. Personally I
 don't have objections to make this feature only available when JS is on
 but I guess the logic to check for `$_REQUEST['selectall']` that's still
 in place should be removed?

 @birgire thanks for taking care of this issue. Do you have the bandwidth
 to address these points in the next days? Thanks!

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/47048#comment:15>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform