Opened 5 months ago
Last modified 1 day ago
(In [12461]) Refs #10695:
(In [12463]) Refs #10657, #10695:
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.
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.
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.
Index: adminenumlistplugin/htdocs/adminenumlist.js =================================================================== --- adminenumlistplugin/htdocs/adminenumlist.js (revision 12491) +++ adminenumlistplugin/htdocs/adminenumlist.js (working copy) @@ -41,7 +41,7 @@ }); // 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);
Replying to jun66j5:
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.
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.
(In [12509]) refs #10695:
Thanks, Ryan. I've committed my patch with your suggestions in r12509.
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.
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.
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.