Enable the 'Remove selected items' button only when a checkbox has been selected, and add a 'select all' checkbox in the table header

[Ryan J Ollos]

• Enable the Remove selected items button only when a checkbox has been selected.
• Add a select all checkbox in the table header for the column that contains the checkboxes.

[Ryan J Ollos]

(In [12461]) Refs #10695:

• The Remove selected items button is enabled only when a checkbox has been selected for at least one of the items.
• Added a tri-state Select all checkbox to the table header.
• The Revert changes button is now enabled when a radio button has been selected.
• Refactoring:
  • Extracted some selectors into variables.
  • Renamed some variables.

[Ryan J Ollos]

(In [12463]) Refs #10657, #10695:

• closest doesn't exist until jQuery 1.3, so its use was replaced with parents.
• prop is recommended for setting the DOM element statesdisabled and checked in jQuery 1.6+. prop is aliased to attr in version of jQuery that it doesn't exist.

In Trac 0.11.0 (jQuery 1.2.3) the indeterminate state of the checkbox is not seen. It may not be supported in earlier versions of jQuery.

[Ryan J Ollos]

I've committed all the work that I have planned for this plugin. I'll wait a little while to see if I get any testing feedback, then change the version from 2.0dev to 2.0 and create a tag.

[Jun Omae]

I tried the "select all" checkbox feature. But I got TracError: severity all does not exist. if submitting with checked "select all" checkbox on Firefox 17.

The following patch avoids wrongly sending sel=all in the request data.

adminenumlistplugin/htdocs/adminenumlist.js

@@ -41 +41 @@
     });
 
     // Add a checkbox for toggling the entire column of checkboxes
-    var $group_checkbox = $('#enumtable thead th.sel').html('<input type="checkbox" name="sel" value="all"/>').children();
+    var $group_checkbox = $('#enumtable thead th.sel').html('<input type="checkbox" value="all"/>').children();
     $group_checkbox.click(function() {
         $remove_checkboxes.prop('checked', this.checked);
         $remove_button.prop('disabled', !this.checked);

[Ryan J Ollos]

Replying to jun66j5:

I tried the "select all" checkbox feature. But I got TracError: severity all does not exist. if submitting with checked "select all" checkbox on Firefox 17.

I remember being concerned about having the name attribute on that element, but I can't remember or see now why I thought it was necessary. It looks like maybe we don't need the value attribute either.

Thanks for the patch. I granted you commit access in case you'd like to push it directly.

[Ryan J Ollos]

I'll get this committed later this evening, and also spotted a few other minor issues that I'll fix. Recently, I've been working on ​t:#9609, ​t:#10992 and #10745, which have a lot of overlap with this ticket.

Jun: consider my invitation to commit to be open-ended, meaning, if you find any issues in the future, please feel free to commit directly.

[Jun Omae]

(In [12509]) refs #10695:

• Avoids sending the value of "select all" checkbox in the request data if checked

[Jun Omae]

Thanks, Ryan. I've committed my patch with your suggestions in r12509.

[Ryan J Ollos]

Just a side note - I've frequently found myself having to add and then immediately select a bit of HTML. As you'll see from the code, I've been using this pattern .html('<input type="checkbox" />').children(), but thinking there might be a better, more clear way to do this.

[Ryan J Ollos]

With regard to [12461] and [12463], where we used,

if(!$.isFunction($.fn.prop))

and

if(!$.isFunction($.curCSS))

to check for the existence of a function bound to $, I learned today that the following also work,

if (!('prop' in $))

if (!('curCss' in $))

The two lines are untested, but I'm just extrapolating on what I found to work in [13192]. Using the in keyword seems like it might lead to slightly clearer code (see also ​SO:7174748).

I'm not suggesting to change the code for AdminEnumListPlugin, just discussing some various ways of doing things.

[Jun Omae]

It seems that the two lines work.

However, I normally use $.fn.name to check for the existences of jQuery object methods.

    if ($.fn.prop) { /* 1.6+ */ }

If the existence of the utility methods, e.g. $.contains, use $.name.

    if ($.contains) { /* 1.4+ */ }

ref. ​http://learn.jquery.com/plugins/basic-plugin-creation/.

I don't know that whether $.name work to check for the object methods for any jQuery versions.